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

GDShader incorrectly expects int cases in switch on uint expressions #95453

Closed
geekley opened this issue Aug 12, 2024 · 0 comments · Fixed by #95473
Closed

GDShader incorrectly expects int cases in switch on uint expressions #95453

geekley opened this issue Aug 12, 2024 · 0 comments · Fixed by #95473
Assignees
Milestone

Comments

@geekley
Copy link

geekley commented Aug 12, 2024

Tested versions

  • Reproducible in v4.2.2.stable.flathub [15073af]

System information

Godot v4.2.2.stable (15073af) - 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 only allows switch on integer expressions. Since it's also accepting uint values on switch, it can be assumed this rule is meant to also include uint. However, the implementation is wrong. When doing a switch on a uint value, it still expects int values on every case label.

I was told to report the bug by @Chaosus on devel chat:

case type must be equal to the switch(type)
on Godot it seems bugged and not compile uints in cases under any circumstances
then its another bug

So you're saying switch should work for both int and uint if you use compatible type?

yes

Steps to reproduce

shader_type canvas_item;

const int iOne = 1;
const uint uOne = 1u;
void fragment() {
	int i; uint u;
	switch (i) {
		case iOne: break; // correct VALID
		//case uOne: break; // correct ERROR
	}
	switch (u) {
		//case iOne: break; // incorrect VALID
		case uOne: break; // incorrect ERROR
	}
}

Minimal reproduction project (MRP)

N/A

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants