Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shaders: Allow trailing commas in function calls #86991

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

akx
Copy link
Contributor

@akx akx commented Jan 9, 2024

This PR implements godotengine/godot-proposals#8840: allowing trailing commas in shader call function call argument lists like

vec3 fadeInTex = clamp(
  mix(-1.0, 2.0, fadeIn) + texture(fade_in_bias, UV).rgb, 
  0.0, 
  1.0,
);

@akx akx requested review from a team as code owners January 9, 2024 11:08
@akx akx force-pushed the shader-trailing-comma branch from fc8dd82 to ccc3b34 Compare January 9, 2024 11:10
@akien-mga akien-mga added this to the 4.x milestone Jan 9, 2024
@akien-mga akien-mga requested review from a team and removed request for a team January 9, 2024 11:37
@akx akx force-pushed the shader-trailing-comma branch from ccc3b34 to 0bf26c8 Compare January 9, 2024 11:59
@AThousandShips
Copy link
Member

@AThousandShips
Copy link
Member

AThousandShips commented Jan 11, 2024

Haven't tested but doesn't look like this fixes:

MyStruct my_struct = MyStruct(foo, bar, );

Or

uniform float my_uniform : hint_range(0, 1, );

If we allow it explicitly in function calls these should also be allowed IMO, or it makes things more inconsistent

Further gdshaders don't allow trailing commas even in array definition, unsure if this matches glsl or not

Edit: seems glsl allows trailing comma in arrays, will look at a fix for that here as it's a parity matter

@akx
Copy link
Contributor Author

akx commented Jan 12, 2024

Haven't tested but doesn't look like this fixes [struct initializers or uniform hint syntax]

No, it doesn't. Those are parsed by an entirely different part of code.

If we allow it explicitly in function calls these should also be allowed IMO, or it makes things more inconsistent

Sure, I can do that. I'm just waiting for an actual decision on godotengine/godot-proposals#8840 before doing more work that might just end up being thrown away if someone decides trailing commas should not be allowed.

Co-authored-by: A Thousand Ships <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shaders: Allow trailing commas in function calls
3 participants