godotengine
GDScript: Fix conflict between property and group names
See also #73870, #73905 and #73933.
I think #73870 can be safely reintroduced after this PR (but it requires testing). Although this is optional (since no valid identifier starts with @group_), but without this check, the following curious thing is possible:
extends Node
@export_group("Resource")
func _ready() -> void:
set("@group_0_Resource", 123)
print(get("@group_0_Resource")) # 123
set("lalala", 123)
print(get("lalala")) # <null>
СС @vnen
Groups and categories are also properties, they just have special flags that make them behave differently. So the error that the user is facing is not a bug but a design limitation. While we can work around it somewhat with this PR, I think we should instead err and report name collisions in the parser. That would be more correct in terms of what the engine expects from the user input.
But you can achieve this with _get_property_list() and the Inspector handles this correctly (see test case output). @group_* is the key of the internal HashMap (which, unlike the property list, does not support repetitions), not the real name of the property. I don't think it makes sense to not allow it.
{ "name": "prop_1", "class_name": &"", "type": 2, "hint": 0, "hint_string": "int", "usage": 4102 }
{ "name": "prop_1", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 128 }
{ "name": "prop_2", "class_name": &"", "type": 2, "hint": 0, "hint_string": "int", "usage": 4102 }
Also, #73843 is a bug anyway.
Well, the inspector is pretty dumb, it just displays things :) But this structure of properties is still an error, because it implies you have 2 properties with the same name and different configurations. If anything, we should validate this more strictly in ClassDB as well.
Will postpone this issue to 4.2, as there seem to be a discussion on if this is an issue by itself.
I would suggest simply not counting entries with group usage flags as properties. So they can be repeated, unlike regular properties. Even the structure matches this: an array of dictionaries, not a dictionary of dictionaries.
1. We already have such entries in native classes.
{ "name": "RefCounted", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 128 }
{ "name": "Resource", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 128 }
{ "name": "Resource", "class_name": &"", "type": 0, "hint": 0, "hint_string": "resource_", "usage": 64 }
{ "name": "resource_local_to_scene", "class_name": &"", "type": 1, "hint": 0, "hint_string": "", "usage": 6 }
{ "name": "resource_path", "class_name": &"", "type": 4, "hint": 0, "hint_string": "", "usage": 4 }
{ "name": "resource_name", "class_name": &"", "type": 4, "hint": 0, "hint_string": "", "usage": 6 }
2. Group ending entries use the empty string, which of course will be repeated if you have multiple group endings.
@export_group("A")
@export_subgroup("1")
@export var a1: int
@export_subgroup("2")
@export var a2: int
@export_group("B")
@export_subgroup("1")
@export var b1: int
@export_subgroup("2")
@export var b2: int
3. I think it is logical that subgroups in several groups can be repeated. At the same time, the property list is flat.
{ "name": "A", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 64 }
{ "name": "1", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 256 }
{ "name": "a1", "class_name": &"", "type": 2, "hint": 0, "hint_string": "int", "usage": 4102 }
{ "name": "2", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 256 }
{ "name": "a2", "class_name": &"", "type": 2, "hint": 0, "hint_string": "int", "usage": 4102 }
{ "name": "B", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 64 }
{ "name": "1", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 256 }
{ "name": "b1", "class_name": &"", "type": 2, "hint": 0, "hint_string": "int", "usage": 4102 }
{ "name": "2", "class_name": &"", "type": 0, "hint": 0, "hint_string": "", "usage": 256 }
{ "name": "b2", "class_name": &"", "type": 2, "hint": 0, "hint_string": "int", "usage": 4102 }
4. As I mentioned above, otherwise we would need to disallow category/group/subgroup names in GDScript that match Variant types, native classes, and user-defined classes (otherwise groups shadow them, in the current implementation). I think it doesn't make sense, for users the group names are just text.
This PR is relatively small and fixes many issues. So in my opinion it's worth considering for cherry-pick. I've added the label beforehand, but note that the production team makes the final decisions on cherry-picks.
Cherry-picked for 4.1.2. This seems to be isolated and simple enough indeed.