C# scripts let you shadow engine properties

paulloz commented 3 months ago

Tested versions

Reproducible in
- v4.2.2.stable.mono.official [15073afe3]
- v4.3.beta.mono.custom_build [b97110cd3]

System information

Godot v4.2.2.stable.mono - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 4060 (NVIDIA; 32.0.15.5599) - Intel(R) Core(TM) i5-10400F CPU @ 2.90GHz (12 Threads)

Issue description

It is possible to shadow engine properties on nodes with C# scripts. For instance, we can see in the linked capture that I shadow the visible property of CanvasItem. Not only the two properties (mine, and the engine's) appear to be linked in the inspector (checking one box checks the other), but the C# property seems to take precedence on the engine one (the node visibility doesn't change any more when the value change).

https://github.com/godotengine/godot/assets/437025/914f59be-b514-4492-9e46-c8e0dbbba108

Of course, the issue is not limited to the inspector. If I try to set the visible property on that node from GDScript, it won't actually set CanvasItem.visible.

Steps to reproduce

Attach the following script to your node.

using Godot;

public partial class Foo : Sprite2D
{
  [Export]
  private bool visible;
}

Enjoy.

Minimal reproduction project (MRP)

MRP.zip

AThousandShips commented 3 months ago

paulloz commented 3 months ago

Arf, thanks. I quickly searched for the issue but apparently didn't find the right keywords 😅

paulloz commented 3 months ago

@raulsntos we can probably define an analyzer for that?

raulsntos commented 3 months ago

That's what I was thinking back then but I'm not sure if it's actually feasible. How would the analyzer know which names are reserved? I guess we'd have to hardcode a list of reserved names by type. I'm also a bit worried about the cost of such analyzer, because this would slow down the build even for users that follow the usual C# naming conventions where you are unlikely to run into conflicts.

paulloz commented 3 months ago

Yeah, I was thinking of only property marked with [Export]. But we expose way more than that to the engine, so I suppose we'd need to analyze every single field and property in objects extending Godot.Object 🙈 It feels a bit much.

godotengine / godot