godotengine / godotengine/godot

GDShader allows commas in `for` conditions (middle part) incorrectly

Open
#95,451 3 comments 0 reactions 0 assignees View on GitHub
enhancement topic:shaders
Dominant language
C++
Stars
117k
Forks
26.8k
PR merge metrics
PR metrics pending

Description

### Tested versions

- Reproducible in v4.2.2.stable.flathub [15073afe3]

### System information

Godot v4.2.2.stable (15073afe3) - Freedesktop SDK 23.08 (Flatpak runtime) - X11 - Vulkan (Forward+) - integrated Intel(R) HD Graphics 5500 (BDW GT2) () - Intel(R) Core(TM) i5-5300U CPU @ 2.30GHz (4 Threads)

### Issue description

GDShader is allowing commas in `for` middle part (condition), requiring every comma-separated part to be a boolean operator. Since GDShader (thankfully!) doesn't have a "comma operator" like in GLSL, this has to be a bug.

The compiler should either:
- Preferably, ***not allow a comma in the middle (condition) part of `for`***, while still allowing it on 1st part (to declare multiple variables) and on 3rd part (to run multiple instructions, e.g. to do `i++, j++`). This is consistent with `while`, which does NOT allow commas.
- The worse option: allow `,` PROPERLY with comma operator semantics, so only the last comma-separated part has to be a boolean in a condition area. But I'm strongly against adding a comma operator that works everywhere, as it's a very confusing syntax. It's only actually useful in the 3rd part of `for`.

Additionally, I don't understand why the `for` condition part requires a boolean *operator*. It makes more sense to allow any expression returning a boolean, e.g. some bool function, some bool variable, literals `true` and `false`, etc.

### Steps to reproduce

`buggy.gdshader`
```glsl
shader_type canvas_item;

void fragment() {
for (int i = 0, j = 0; 1 == 2, j <= 10; i++, j++)
COLOR = vec4(float(i)/10.0, float(j)/10.0, 0, 1);
// yellow texture shows the comma operator is being accepted on the `for` condition (2nd part) too
// but the editor incorrectly expects every part of the comma to be a boolean operator expression
}

```

### Minimal reproduction project (MRP)

N/A

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.