-
-
Notifications
You must be signed in to change notification settings - Fork 168
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
Fix errors in grammar syntax #416
Merged
Merged
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
e5cacae
Replace invalid lookbehind assertion
DaelonSuzuka a8104e8
Fix regression caused by missing double escapes
DaelonSuzuka 56147ef
Replace invalid lookbehind assertions
DaelonSuzuka d912895
Remove unused fields from GDResource grammar
DaelonSuzuka 385bc98
Apply feedback from code review
DaelonSuzuka 9f2a0f4
Add additional gdshader example, provided by @alfishsoftware
DaelonSuzuka b712eff
Prevent "identifierField" rule from consuming function calls
DaelonSuzuka 8f318f9
Apply suggestions from code review
DaelonSuzuka 7166143
Add more example cases
DaelonSuzuka e5ab256
Replace broken rule for "in" keyword
DaelonSuzuka fcad85d
SImplify rules for identifier names
DaelonSuzuka 87feab1
Standardized regex rule for identifier names
DaelonSuzuka File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,193 @@ | ||
shader_type spatial; | ||
render_mode wireframe; | ||
|
||
const lowp vec3 v[1] = lowp vec3[1] ( vec3(0, 0, 1) ); | ||
|
||
void fn() { | ||
// The required amount of scalars | ||
vec4 a0 = vec4(0.0, 1.0, 2.0, 3.0); | ||
// Complementary vectors and/or scalars | ||
vec4 a1 = vec4(vec2(0.0, 1.0), vec2(2.0, 3.0)); | ||
vec4 a2 = vec4(vec3(0.0, 1.0, 2.0), 3.0); | ||
// A single scalar for the whole vector | ||
vec4 a3 = vec4(0.0); | ||
|
||
mat2 m2 = mat2(vec2(1.0, 0.0), vec2(0.0, 1.0)); | ||
mat3 m3 = mat3(vec3(1.0, 0.0, 0.0), vec3(0.0, 1.0, 0.0), vec3(0.0, 0.0, 1.0)); | ||
mat4 identity = mat4(1.0); | ||
|
||
mat3 basis = mat3(identity); | ||
mat4 m4 = mat4(basis); | ||
mat2 m2a = mat2(m4); | ||
|
||
vec4 a = vec4(0.0, 1.0, 2.0, 3.0); | ||
vec3 b = a.rgb; // Creates a vec3 with vec4 components. | ||
vec3 b1 = a.ggg; // Also valid; creates a vec3 and fills it with a single vec4 component. | ||
vec3 b2 = a.bgr; // "b" will be vec3(2.0, 1.0, 0.0). | ||
vec3 b3 = a.xyz; // Also rgba, xyzw are equivalent. | ||
vec3 b4 = a.stp; // And stpq (for texture coordinates). | ||
b.bgr = a.rgb; // Valid assignment. "b"'s "blue" component will be "a"'s "red" and vice versa. | ||
|
||
lowp vec4 v0 = vec4(0.0, 1.0, 2.0, 3.0); // low precision, usually 8 bits per component mapped to 0-1 | ||
mediump vec4 v1 = vec4(0.0, 1.0, 2.0, 3.0); // medium precision, usually 16 bits or half float | ||
highp vec4 v2 = vec4(0.0, 1.0, 2.0, 3.0); // high precision, uses full float or integer range (default) | ||
|
||
const vec2 aa = vec2(0.0, 1.0); | ||
vec2 bb; | ||
bb = aa; // valid | ||
|
||
const vec2 V1 = vec2(1, 1), V2 = vec2(2, 2); | ||
|
||
float fa = 1.0; | ||
float fb = 1.0f; | ||
float fc = 1e-1; | ||
|
||
uint ua = 1u; | ||
uint ub = uint(1); | ||
|
||
bool cond = false; | ||
// `if` and `else`. | ||
if (cond) { | ||
} else { | ||
} | ||
// Ternary operator. | ||
// This is an expression that behaves like `if`/`else` and returns the value. | ||
// If `cond` evaluates to `true`, `result` will be `9`. | ||
// Otherwise, `result` will be `5`. | ||
int i, result = cond ? 9 : 5; | ||
// `switch`. | ||
switch (i) { // `i` should be a signed integer expression. | ||
case -1: | ||
break; | ||
case 0: | ||
return; // `break` or `return` to avoid running the next `case`. | ||
case 1: // Fallthrough (no `break` or `return`): will run the next `case`. | ||
case 2: | ||
break; | ||
//... | ||
default: // Only run if no `case` above matches. Optional. | ||
break; | ||
} | ||
// `for` loop. Best used when the number of elements to iterate on | ||
// is known in advance. | ||
for (int i = 0; i < 10; i++) { | ||
} | ||
// `while` loop. Best used when the number of elements to iterate on | ||
// is not known in advance. | ||
while (cond) { | ||
} | ||
// `do while`. Like `while`, but always runs at least once even if `cond` | ||
// never evaluates to `true`. | ||
do { | ||
} while (cond); | ||
} | ||
|
||
const float PI_ = 3.14159265358979323846; | ||
|
||
struct PointLight { | ||
vec3 position; | ||
vec3 color; | ||
float intensity; | ||
}; | ||
|
||
struct Scene { | ||
PointLight lights[2]; | ||
}; | ||
|
||
const Scene scene = Scene(PointLight[2]( | ||
PointLight(vec3(0.0, 0.0, 0.0), vec3(1.0, 0.0, 0.0), 1.0), | ||
PointLight(vec3(0.0, 0.0, 0.0), vec3(1.0, 0.0, 0.0), 1.0) | ||
)); | ||
|
||
Scene construct_scene(PointLight light1, PointLight light2) { | ||
return Scene({light1, light2}); | ||
} | ||
|
||
varying flat vec3 some_color; | ||
|
||
varying float var_arr[3]; | ||
|
||
varying smooth vec3 some_light; | ||
|
||
uniform float some_value; | ||
|
||
uniform vec4 color : hint_color; | ||
uniform float amount : hint_range(0, 1); | ||
uniform vec4 other_color : hint_color = vec4(1.0); | ||
|
||
uniform vec4 some_vector = vec4(0.0); | ||
uniform vec4 some_color2 : hint_color = vec4(1.0); | ||
|
||
void vertex() { | ||
const float arr[] = { 1.0, 0.5, 0.0 }; | ||
COLOR.r = arr[0]; // valid | ||
|
||
float arr2[3]; | ||
arr2[0] = 1.0; // setter | ||
COLOR.r = arr2[0]; // getter | ||
|
||
PointLight light; | ||
light.position = vec3(0.0); | ||
light.color = vec3(1.0, 0.0, 0.0); | ||
light.intensity = 0.5; | ||
|
||
COLOR.rgb = construct_scene( | ||
PointLight(vec3(0.0, 0.0, 0.0), vec3(1.0, 0.0, 0.0), 1.0), | ||
PointLight(vec3(0.0, 0.0, 0.0), vec3(1.0, 0.0, 1.0), 1.0) | ||
).lights[0].color; | ||
|
||
some_color = NORMAL; // Make the normal the color. | ||
|
||
var_arr[0] = 1.0; | ||
var_arr[1] = 0.0; | ||
} | ||
|
||
void fragment() { | ||
float arr[3]; | ||
|
||
float float_arr[3] = float[3] (1.0, 0.5, 0.0); // first constructor | ||
int int_arr[3] = int[] (2, 1, 0); // second constructor | ||
vec2 vec2_arr[3] = { vec2(1.0, 1.0), vec2(0.5, 0.5), vec2(0.0, 0.0) }; // third constructor | ||
bool bool_arr[] = { true, true, false }; // fourth constructor - size is defined automatically from the element count | ||
|
||
float a[3] = float[3] (1.0, 0.5, 0.0), | ||
b[2] = { 1.0, 0.5 }, | ||
c[] = { 0.7 }, | ||
d = 0.0, | ||
e[5]; | ||
|
||
float arr2[] = { 0.0, 1.0, 0.5, -1.0 }; | ||
for (int i = 0; i < arr2.length(); i++) { | ||
} | ||
|
||
ALBEDO = v[0]; | ||
|
||
PointLight light = PointLight(vec3(0.0), vec3(1.0, 0.0, 0.0), 0.5); | ||
|
||
ALBEDO = scene.lights[0].color; | ||
|
||
const float EPSILON = 0.0001, value = 0.f; | ||
if (value >= 0.3 - EPSILON && value <= 0.3 + EPSILON) { | ||
discard; | ||
} | ||
|
||
ALBEDO = vec3(var_arr[0], var_arr[1], var_arr[2]); // red color | ||
|
||
some_light = ALBEDO * 100.0; // Make a shining light. | ||
} | ||
|
||
void sum2(int a, in int b, inout int result) { | ||
result = a + b; | ||
} | ||
void sub2(const int a, const in int b, out int result) { | ||
result = a - b; | ||
} | ||
|
||
global uniform sampler2D global1; | ||
instance uniform int un = 0; | ||
|
||
void light() { | ||
DIFFUSE_LIGHT = some_color * 100.; // optionally | ||
|
||
DIFFUSE_LIGHT = some_light; | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't test this, but I suspect that it will now consider the whole thing as a keyword.
E.g.
for i in array
thenfor i in
is a keyword. It shouldn't mark the loop variable (i
in this case) as a keyword.The correct thing to do here is to join this with the
for
keyword so that one rule recognizes both keywords. You don't need to removefor
fromcontrol_flow
(for
should still be a keyword even beforein
is typed) as long as this is before so it has higher priority; but change this rule to something along the lines of:I changed
\w
(which matches[a-zA-Z0-9_]
including numbers) since I believe identifiers can't start with a number (correct me if I'm wrong). It's probably worth checking this on other places in the code too. I'm assuming identifiers allow ASCII only.I also allowed more than one space in the syntax.
Btw, using a tool like https://regex101.com/ while you write it really helps with understanding nuances in regexes and checking that it will really do what is expected, and prevent against corner cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I cannot believe that I missed this. You were correct,
i
was purple.Your rule worked great. I removed the middle capture group and it's name block, because we don't actually use the
variable.name
tag for variables. Python doesn't tag variable names, they're just white, so that's the behavior I want to imitate.Many other places used the full form
[a-zA-Z_][a-zA-Z_0-9]*
, but I just replaced them with[a-zA-Z_]\\w*
. I wish we could give a block like this a custom name so we don't have to copy it everywhere.I started off doing this, but having to add all the extra escapes to move a rule over to vscode was too annoying so I stopped. I've thought about switching to yaml files, since the regex strings won't need escapes, but there was some problem with that too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually referring to places capturing identifiers with just
\w+
, which would allow names starting with numbers, e.g.1whatever
,0e2f
,0xFACADE
. I took a look and saw it in 2 places.So, unless this is on purpose for some reason, this seems like it makes more sense:
Line 360:
Line 375:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another good catch, thanks. I found some other places where identifier names were being matched with
[[:alpha:]_]\\w*
, so I standardized all of them to[a-zA-Z_]\\w*
.