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

Hint fallback property as node when it is a node #89175

Conversation

paulloz
Copy link
Member

@paulloz paulloz commented Mar 5, 2024

Fixes #85492

A comprehensive (I hope) step by step of what is happening.

  • Let's make a new scene with a Node at the top.
  • Attach the following script to the root of the scene.
    using Godot;
    public partial class MyNode : Node
    {
        [Export]
        public Sprite2D MySprite { get; private set; }
     
        public override void _Ready()
        {
     	   GD.Print($"{nameof(MySprite)}: {MySprite?.GetPath()}");
        }
    }
  • Add a Sprite2D in the scene, build, and assign that Sprite2D to our MySprite property.
  • Run the scene, and witness the following output.
    MySprite: /root/MyNode/MySprite2D
    
  • At that point, if we look at the .tscn, it should look like this.
    [gd_scene load_steps=2 format=3 uid="uid://c7ybg78eykspy"]
    
    [ext_resource type="Script" path="res://MyNode.cs" id="1_u8846"]
    
    [node name="MyNode" type="Node" node_paths=PackedStringArray("MySprite")]
    script = ExtResource("1_u8846")
    MySprite = NodePath("MySprite2D")
    
    [node name="MySprite2D" type="Sprite2D" parent="."]
  • Close all the open scenes, scripts, etc.
  • Clean the C# solution.
  • Close and reopen Godot, reopen the scene.
  • Do not rebuild the C# solution, run the game, and witness the output.
    MySprite: 
    
    E 0:00:03:0953   NativeCalls.cs:1026 @ Godot.NodePath Godot.NativeCalls.godot_icall_0_112(nint, nint): Cannot get path of node as it is not in a scene tree.
    
  • At that point, if we look at the .tscn, it will look like this.
    [gd_scene load_steps=2 format=3 uid="uid://c7ybg78eykspy"]
    
    [ext_resource type="Script" path="res://MyNode.cs" id="1_u8846"]
    
    [node name="MyNode" type="Node"]
    script = ExtResource("1_u8846")
    MySprite = Object(Sprite2D,"_import_path":NodePath(""),"unique_name_in_owner":false,"process_mode":0,"process_priority":0,"process_physics_priority":0,"process_thread_group":0,"auto_translate_mode":0,"editor_description":"","visible":true,"modulate":Color(1, 1, 1, 1),"self_modulate":Color(1, 1, 1, 1),"show_behind_parent":false,"top_level":false,"clip_children":0,"light_mask":1,"visibility_layer":1,"z_index":0,"z_as_relative":true,"y_sort_enabled":false,"texture_filter":0,"texture_repeat":0,"material":null,"use_parent_material":false,"position":Vector2(0, 0),"rotation":0.0,"scale":Vector2(1, 1),"skew":0.0,"texture":null,"centered":true,"offset":Vector2(0, 0),"flip_h":false,"flip_v":false,"hframes":1,"vframes":1,"frame":0,"region_enabled":false,"region_rect":Rect2(0, 0, 0, 0),"region_filter_clip_enabled":false,"script":null)
    
    
    [node name="MySprite2D" type="Sprite2D" parent="."]
  • We can see the Sprite2D has been serialized inline, instead of via its path. You can fiddle around by, e.g., setting metadata on the Sprite2D and see it is indeed the right node being serialized there, only not in the right way.

My understanding of what is happening

When running a project, the process goes vaguely like this:

  1. Save (and so, serialize) all the open scenes.
  2. Call all the _build callbacks for all plugins (and so, the one from the .NET module).
  3. Run the project.

In C#, we rely on the built assembly to retrieve information about exported properties. If the assembly is not built (e.g. the user manually cleaned it, the user just cloned their repo on another computer, etc.). We do not have information about exported properties available. When this is the case, Godot relies on fallbacks that are set here. This explains how we actually serialize the right node still. But this process always sets the exported property's hint to PROPERTY_HINT_NONE, which explains why it is serialized inline (PROPERTY_HINT_NODE_TYPE is what triggers the serialization as path).

The fix assumes that setting a fallback to a value of type OBJECT that is a Node means we want to hint it as PROPERTY_HINT_NODE_TYPE.

@paulloz paulloz force-pushed the dotnet/fix-exported-nodes-reset-without-build-artifacts branch from 6199e4a to e7c90e0 Compare March 5, 2024 11:48
@paulloz paulloz marked this pull request as ready for review March 5, 2024 12:27
@paulloz paulloz requested a review from a team as a code owner March 5, 2024 12:27
@akien-mga akien-mga added this to the 4.3 milestone Mar 5, 2024
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the detailed description. It makes a lot of sense to me.

I think this solution is good, we do want to keep hinting this as a Node if we can get that info.

As discussed on chat, it might be worth evaluating whether compiling assemblies before saving scenes would also be good to do. I think a risk there is that some unsaved scene changes may impact which assemblies should be compiled?

@akien-mga akien-mga added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 5, 2024
@paulloz
Copy link
Member Author

paulloz commented Mar 5, 2024

Yes, although I don't think issues about not being sync between the assembly and the scenes are that common, since if you purposefully change something that'd affect the scene (add an export for instance), you'd need to rebuild to be able to assign a value.

The bigger issue we fix here is the current process being destructive for the content of the scene. Once you click run, the only option too retrieve data would be your VCS.

@akien-mga akien-mga merged commit 9b94c80 into godotengine:master Mar 6, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 4.2.2.

@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 11, 2024
@paulloz paulloz deleted the dotnet/fix-exported-nodes-reset-without-build-artifacts branch April 19, 2024 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

C# Variable Exports get lost when the ".godot" folder is deleted
3 participants