GDScript: Fix conflict between property and group names

[dalexeev]

• Closes Singletons inaccessible when an exported group has the same name #67865.
• Closes @tool and @export_group("", "") after prefixed @export_group do crash editor #68030.
• Closes Invalid call. Nonexistent function 'new' in base 'Nil'. if you use @export_group with same class name #73843.
• Closes Using @export_category using the same name as a variable will overwrite the first variable in the category. #78252.
• Closes Using the same export_group name in an inherited class and re-saving that class while the project is running crashes the project and sometimes the editor #79804.
• Closes @export_category creates a variable that causes issues if another one has the same name #79861.

[dalexeev]

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

[YuriSizov]

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.

[dalexeev]

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.

[YuriSizov]

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.

[adamscott]

Will postpone this issue to 4.2, as there seem to be a discussion on if this is an issue by itself.

[dalexeev]

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.

[adamscott]

Discussed in the GDScript weekly meeting. We agree with @dalexeev, the group names should not count as properties for the check of name collision. So we approve this PR.

[YuriSizov]

Thanks!

[dalexeev]

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.

[YuriSizov]

Cherry-picked for 4.1.2. This seems to be isolated and simple enough indeed.