Fix incorrect handling of exported typed dictionaries containing nodes in .NET

[juanjp600]

This should fix #97850. This pull request consists of multiple commits because there are multiple distinct bugs that needed to be fixed.

As observed here, the scene is written with a copy of the node selected in the inspector instead of a path to it. The reason for this is that the property hint that the scene serializer looks for to rewrite node references as paths is PROPERTY_HINT_TYPE_STRING, but the code generation for exposing .NET properties to Godot does not account for this. That's fixed here.

With that first bug fixed, another one reveals itself: the nodes in the exported dictionary are all nulled out on the .NET side of things. Looking further with Get(nameof(Controls)), what is happening is that the node paths that are stored in the scene are not being turned back into node references. The scene reader is getting an untyped dictionary, which causes the relevant get_typed_*_builtin to not return Variant::OBJECT. The fix is to call set_typed on typed dictionaries from .NET so Godot also sees them as typed dictionaries.

These two fixes are still not enough because if no default non-null value is provided for the exported dictionary, which is the case in the MRP given, the scene reader still fails to convert node paths to the expected node type. That's fixed here.

Here's an updated MRP with a patched scene, because the first bug makes changes to the scene that are not easily reverted automatically: MRP-97850-patched-scene.zip

Do note that because of #97190, the scene will not load correctly if you open it before building the project, so make sure you do that.

[juanjp600]

This is not entirely correct if the type arguments for the dictionary are script types. However, the hint string does not include any information about scripts, so I'm not sure what the best way to address it is.

[raulsntos]

Thanks for the contribution. You are totally right about the Dictionary hint being wrong, but I don't really understand the part about set_typed. We're not calling set_typed for typed Arrays and that works fine with arrays of nodes (e.g.: Godot.Collections.Array<Label>). So I feel like there may be a way to fix the bug without set_typed.

Calling set_typed is probably something that we should do anyway, and not just on Dictionary but on Array as well. However, I don't really like the current PR's implementation, it relays heavily on reflection which may be incompatible with trimming/NativeAOT, and may add substantial cost to typed Dictionaries which may be unacceptable to some users.

I have not looked at the changes to packed_scene.cpp at all, that's an area of Godot I'm not very familiar with so I don't think I can comment on it.

[raulsntos]

This should always be PropertyHint.TypeString, no need for the containsNodeType variable.

Suggested change

	hint = containsNodeType ? PropertyHint.TypeString : PropertyHint.DictionaryType;
	hint = PropertyHint.TypeString;

[juanjp600]

I don't really understand the part about set_typed. We're not calling set_typed for typed Arrays and that works fine with arrays of nodes (e.g.: Godot.Collections.Array). So I feel like there may be a way to fix the bug without set_typed.

The reason why I think set_typed is necessary for dictionaries is because the conversion from NodePath to Node can be selective, i.e. it's possible for the conversion to apply to only keys or only values. Theoretically a dictionary of the form Dictionary[NodePath, Node] has to be converted back from an untyped dictionary containing pairs of the form [NodePath, NodePath], and as far as I'm aware there's no other mechanism to tell Godot what to convert. Arrays have only one type argument, so by simply establishing that a NodePath to Node conversion needs to happen, it's safe to infer that it applies to all elements in the array.

You are correct that, to be thorough, arrays should have a set_typed call. Would that be in scope for this pull request or should another be opened for that change?

[raulsntos]

You are correct that, to be thorough, arrays should have a set_typed call. Would that be in scope for this pull request or should another be opened for that change?

Well, what I was getting at was that I would separate the property hint changes from the set_typed changes. Because, I'm very confident that the property hint changes are correct and would be willing to approve and merge that, but I'm not so confident about the set_typed changes, these may require further discussion.

But I think whatever PR includes the set_typed changes for Dictionary should probably also make the same changes to Array if we can, since we'll want to match the behavior for both types.

[juanjp600]

Superseded by #98545 and #98546