Replace temporary .gdshader syntax with more extensive support

[DaelonSuzuka]

Thanks to a generous contribution from @AlfishSoftware, we have a brand new, custom built gdshader syntax.

@Calinou Are you aware of any changes in the gdshader language from Godot3 to Godot4 that might cause a problem here? I have to admit I've barely touched shaders in Godot3, and I have absolutely no idea what's been added or removed in 4.

The following screenshots show the before/after comparison, as well as a shader embedded in a resource file:

[Calinou]

@Calinou Are you aware of any changes in the gdshader language from Godot3 to Godot4 that might cause a problem here? I have to admit I've barely touched shaders in Godot3, and I have absolutely no idea what's been added or removed in 4.

The shader language in 4.0 is mostly the same as 3.x, except there are a few new keywords such as global and instance. Some hints will be renamed too. There are also a few more built-ins available such as FOG.

[DaelonSuzuka]

The shader language in 4.0 is mostly the same as 3.x, except there are a few new keywords such as global and instance. Some hints will be renamed too. There are also a few more built-ins available such as FOG.

Awesome, I'll have this stuff double-checked sometime today.

[AlfishSoftware]

I'm no expert in any of this, but according to this article, there is a struct keyword as well? If so, then it should be included as a new type of keyword (none of the current categories fit well).

It also has implications on how it should recognize these identifier tokens. The ideal would be semantic highlighting, but without that implemented, the best it can do is to assume PascalCase refers to structs. This is similar to what I'm currently doing, assuming any ALL_UPPERCASE identifier (such as TIME, ALBEDO, PI, etc) refers to a language variable (in the latter case they could be listed too, I admit I didn't do it out of laziness). I think it's reasonable to assume this until actual semantic parsing is implemented/enabled. These should be colored similar to language types like int, except not as keywords.

It's also possible that I don't have to make such assumptions; so maybe a struct identifier can be recognized depending on where it appears. This would be the ideal way to detect them without needing semantic highlighting.

It's important to categorize these tokens right so that they look right regardless of theme, so maybe I'll take a look at this today.
If I update my repo, I'll comment here.

[Calinou]

4.0 indeed supports structs in its shader language. I'd highlight the keyword anywhere, even in places where it doesn't make sense. This is the approach the built-in script editor follows too. It is a reserved word after all, so it can't be used as a variable or function name.

[DaelonSuzuka]

I'm no expert in any of this, but according to this article, there is a struct keyword as well? If so, then it should be included as a new type of keyword (none of the current categories fit well).

Thanks for the link, I added some snippets from the article to the example file.

It also has implications on how it should recognize these identifier tokens. The ideal would be semantic highlighting, but without that implemented, the best it can do is to assume PascalCase refers to structs.

This is what I would do, with the caveat that I have no idea if there's some kind of normal convention for shaders that uses PascalCase for something other than a struct "type".

[DaelonSuzuka]

4.0 indeed supports structs in its shader language. I'd highlight the keyword anywhere, even in places where it doesn't make sense. This is the approach the built-in script editor follows too. It is a reserved word after all, so it can't be used as a variable or function name.

I think the dilemma is more about assuming all PascalCase symbols are struct "type" names, and if that has a conflict with another shader feature or style convention.

[AlfishSoftware]

Taking a superficial look at the example, it seems it should be absolutely possible to mark struct identifiers properly in most places without assuming naming conventions.
struct myStruct <- this can be easily detected with positive look-ahead
myStruct myField <- pretty sure this can be detected as well, though I'll have to make proper tests with the syntax to be sure
This can appear at least in fields, variables, parameters and method return types.
I don't know if there's other places where a struct type could appear (type hints?).

The only place where it's an issue is constructors, which, without a new keyword like in C#, are indistinguishable from methods.
myStruct(MyMethod(), 0)
In this place, I think it's OK to assume that constructors use PascalCase convention. In any case, marking a constructor as a method is not unbearably bad (it IS technically a method), but assuming just here is a lot better IMO.

[AlfishSoftware]

Here's some specific behavior I found when making tests with gdshader (Godot 4.0.alpha8).
These don't necessarily need to be considered in syntax, but I'll post my test file here just for the sake of registering it somewhere.

It's worth noting that hints are indeed reserved words (and shader types | render modes aren't).
Built-in variables, however, are not necessarily reserved (it depends on context); same for fragment | light function names. I'll think about how to better handle these later. It might not be worth having special syntax for them (at least in Godot4 shader API, until it's stable).
Also, I realized that current syntax doesn't support uint literals (e.g. 1u) yet.

shader_type spatial;
render_mode wireframe;
const uint one = 1u;
void fn() {}
struct A { int b; };
struct MyStruct { // only uninitialized fields are allowed
  lowp int x; // only precision modifiers allowed
  A[1] s; A t[one]; // and arrays
};
void vertex() {
  //err: int hint_white = 0; // hints are reserved words, just like other keywords
  int ALBEDO = 0, xx = ALBEDO; // language variables are only reserved in their function
}
// fragment|light are not reserved functions in particle shaders, so params are allowed in this case
void fragment() {
  int spatial = 0, xx1 = spatial; // shader types are not reserved words
  int wireframe = 0, xx2 = wireframe; // render modes are not reserved words
  int fragment = 0, fn = 0, xx3 = fragment; // can declare locals using existing function names
  int MyStruct = 0, xx4 = MyStruct; // can declare locals using existing struct names
  float a = 0.f, b = 0.e0; // f|e can appear right after . so weird field-like syntax is valid
  A[one] x1 = { A(0) }; // a positive const identifier is allowed as array size
  A[1u] x2 = { A(0) }; // unsigned int|uint literals are also allowed (but not expr)
  A x3[] = { A(0) }; // [] may appear in right side, with same rules
}
const MyStruct[1] x = { MyStruct(0, {A(0)}, {A(0)}) };

Lastly, weird cases like 0.f 0.e0 are supported, but I think it's bad syntax (looks like it's accessing a field) ... I'd even say 0. is bad too. Strangely, 0f, which I supported because I assumed was valid seems to be invalid (at least in v4). I think it's worth reporting this to whoever is responsible for the gdshader syntax while it's still in alpha; maybe it wasn't intended.

[Chaosus]

Also, I realized that current syntax doesn't support uint literals (e.g. 1u) yet.

This is fixed in 4.0 (see godotengine/godot#55623)

Lastly, weird cases like 0.f 0.e0 are supported

This is allowed GLSL syntax and would not be changed in order to preserve compatibility.

[AlfishSoftware]

This is allowed GLSL syntax and would not be changed in order to preserve compatibility.

Ah, I see. So the weird definition came from GLSL. Makes sense.
Though the literals syntax isn't completely compatible with GLSL, it seems.

I'm not sure what version of GLSL it's based on, but I checked the grammar in the spec for GLSL ES 3.0.
I just noticed, it seems octal literals from GLSL aren't supported (? I might be mistaken) which I actually think is good because leading 0 as octal is just a very stupid syntax. Strangely, the hexadecimal syntax is supported, but it doesn't allow the u suffix like in GLSL. Also the LF suffix for doubles is not supported. Not that I think these need to be supported or anything, it's just an observation (and I didn't check earlier versions of the spec either).

Also, I realized that current syntax doesn't support uint literals (e.g. 1u) yet.

I meant my vscode syntax didn't support it yet. I didn't know it wasn't in Godot 3, probably that's why I didn't notice it.
I also noticed another mistake now: I must make these case-insensitive in literals: 0X prefix, E notation, and F and U suffix.

I just noticed something else too which could be a bug (Godot 4.0.alpha8).
It's accepting floats like 0ef, 0e+f, 0.0ef, 0.0e-f, which aren't valid in GLSL. Which is strange because invalids like 0e, 0.0e are being rejected correctly. At least in the GLSL grammar above, the digits are always required after the e in exponent notation. But something strange in gdshader grammar is accepting an empty number when f is present.

[Chaosus]

I just noticed something else too which could be a bug (Godot 4.0.alpha8).
It's accepting floats like 0ef, 0e+f, 0.0ef, 0.0e-f, which aren't valid in GLSL. Which is strange because invalids like 0e, 0.0e are being rejected correctly. At least in the GLSL grammar above, the digits are always required after the e in exponent notation. But something strange in gdshader grammar is accepting an empty number when f is present.

This is a bug, indeed, could you post it to bug tracker?

[AlfishSoftware]

@Chaosus I posted in godot repo, don't know if it's the right one.

[AlfishSoftware]

I also updated the .tmLanguage in my repo, fixing some issues and adding support for Godot 4 shader. There were more type hints missing than those added here. I reorganized the whole thing so it's better to understand, simplified a few things, and reviewed some tokenization classes.

Array constructors myStruct[3]() are always recognized as constructors regardless of casing convention.

Most things, such as struct identifiers, functions, field access, etc are only recognized as such when context is in the same line.

MyStruct fn() { a.b // ok
// not:
MyStruct
fn
() { a.
b

I might maybe try to improve this eventually, but other than such edge cases, syntax should be functional.
Struct declaration and its fields should hopefully not have this limitation (feel free to test).

This file with various pieces taken from docs might be useful as a starting point for testing syntax: godot4docs.gdshader.txt
Here's how it looks in my custom theme:

I lost my other test file (I think Godot automatically saved it without asking when closing ...)

[DaelonSuzuka]

I also updated the .tmLanguage in my repo

@AlfishSoftware I'll pull that update over when I have time. Thanks for digging into the details here.

[AlfishSoftware]

I'll pull that update over when I have time.

Just a note: Since I've published my extension, I had to change language id and scopeName so it doesn't clash with this extension (so both can be used, otherwise the temporary grammar is taking priority). When pulling from the grammar, make sure to change scopeName source.gdshader.a back to source.gdshader here.

[DaelonSuzuka]

When pulling from the grammar, make sure to change scopeName source.gdshader.a back to source.gdshader here.

Of course. I had made a couple of changes to make the metadata fit the rest of our extension anyways. Thanks again for all your work on this!

[AlfishSoftware]

It seems Godot editor is compatible with LSP for GDScript, right?
Does that apply to GDShader as well? Would it be possible to provide completions, errors, etc by using a similar method to what is used for GDScript? Would it connect on the same host and port?

[Calinou]

It seems Godot editor is compatible with LSP for GDScript, right? Does that apply to GDShader as well? Would it be possible to provide completions, errors, etc by using a similar method to what is used for GDScript? Would it connect on the same host and port?

The LSP is currently only for GDScript, not other languages. There are no plans to add LSP support for other languages, although it may happen if a motivated contributor does the required work.

I assume the language server would still run on the same host and port for this.

[AlfishSoftware]

it may happen if a motivated contributor does the required work

OK, then. Thanks for the quick answer!
I've just opened a proposal for this as a discussion, so that I get notified if it's ever implemented.

[DaelonSuzuka]

What information would you actually get from a language server that you can't just as easily figure out in a client-only implementation? The current Godot version is the only thing I can think of.

I don't think a GDShader language server makes any sense, personally. Is it even possible for a single shader to be in more than one file?

[AlfishSoftware]

See detailed answer in the discussion linked above.

[Calinou]

Thanks a ton!

[AlfishSoftware]

I also updated the .tmLanguage in my repo

@AlfishSoftware I'll pull that update over when I have time. Thanks for digging into the details here.

Just to confirm, this hasn't been done, right?

[AlfishSoftware]

I asked because it seems tmLanguage is missing the Godot 4 keywords and type hints from my latest update, including struct syntax.

[DaelonSuzuka]

Just to confirm, this hasn't been done, right?

Correct. I got a new job last month and it's been taking up 100% of my bandwidth. Thank you for checking in on this, I appreciate the reminder.

@Calinou There are some additional changes I need to pull in from @AlfishSoftware's work that, quite frankly, I forgot about until now. I'll open another PR to pull in the rest of the features.