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

[4.3dev] Exported node path in child scene are reflected in the parent scene file #91591

Open
Cronos87 opened this issue May 5, 2024 · 9 comments · May be fixed by #92095
Open

[4.3dev] Exported node path in child scene are reflected in the parent scene file #91591

Cronos87 opened this issue May 5, 2024 · 9 comments · May be fixed by #92095

Comments

@Cronos87
Copy link

Cronos87 commented May 5, 2024

Tested versions

  • Reproductible in any 4.3 dev

System information

macOS Monterey 12.6.5

Issue description

Exported nodes in a child scene will be reflected in the parent scene file when saving a scene, BUT only if the parent scene is opened in the editor.

Steps to reproduce

  • Create an empty scene called MyLevel
  • Create a scene called Menu. Create a node with a script that exports some other nodes from the same scene (example: @export var nodes: Array[Label])
  • Instantiate the Menu scene in MyLevel as a child scene
  • Keep both scenes open in the editor and save
  • Using an external tool (VSCode, vim, cat, etc...), inspect the MyLevel scene and observe that exported nodes from Menu are reflected in the MyLevel file

image

Minimal reproduction project (MRP)

exported-node-issue.zip

@Cronos87 Cronos87 changed the title Exported node path in child scene are reflected in the parent scene file [4.3dev] Exported node path in child scene are reflected in the parent scene file May 6, 2024
@caimantilla
Copy link

If I understand correctly, what you've reported is about properties of the instance being copied to properties of the base scene.
It also applies in reverse, though. It's turning out to be quite the headache. I tried removing a node export on the base scene, but since, for some reason, the NodePath is also saved for the instance despite the two paths matching, I can't just do this like I used to be able to...

@Cronos87
Copy link
Author

Cronos87 commented May 7, 2024

If I understand correctly, what you've reported is about properties of the instance being copied to properties of the base scene.

That's it! That's the issue I wanted to describe. This wasn't the case in Godot 4.2.

@kleonc
Copy link
Member

kleonc commented May 7, 2024

This wasn't the case in Godot 4.2.

Indeed, can't reproduce in v4.2.2.stable.official [15073af] and earlier.

Can reproduce since v4.3.dev1.official [9d1cbab].

That's a regression, hence marking it as such (should be addressed before 4.3.stable).


menu.tscn my_level.tscn
Godot_v4 3-dev6_win64_wyfX0MgmYU uDFAUVuFal

menu.tscn contents (same in v4.2.2.stable.official [15073af] / v4.3.dev6.official [89850d5]):

[gd_scene load_steps=2 format=3 uid="uid://bdp4gvtih2bec"]

[ext_resource type="Script" path="res://menu.gd" id="1_vb7hi"]

[node name="Menu" type="Node2D" node_paths=PackedStringArray("nodes")]
script = ExtResource("1_vb7hi")
nodes = [NodePath("VBoxContainer/Label"), NodePath("VBoxContainer/Label2"), NodePath("VBoxContainer/Label3")]

[node name="VBoxContainer" type="VBoxContainer" parent="."]
offset_right = 40.0
offset_bottom = 40.0

[node name="Label" type="Label" parent="VBoxContainer"]
layout_mode = 2
text = "Label 1"

[node name="Label2" type="Label" parent="VBoxContainer"]
layout_mode = 2
text = "Label 2"

[node name="Label3" type="Label" parent="VBoxContainer"]
layout_mode = 2
text = "Label 3"

my_level.tscn contents:

  • v4.2.2.stable.official [15073af]:
[gd_scene load_steps=2 format=3 uid="uid://dakw7h8ta0aa4"]

[ext_resource type="PackedScene" uid="uid://bdp4gvtih2bec" path="res://menu.tscn" id="1_tkt5r"]

[node name="MyLevel" type="Node2D"]

[node name="Menu" parent="." instance=ExtResource("1_tkt5r")]
[gd_scene load_steps=2 format=3 uid="uid://dakw7h8ta0aa4"]

[ext_resource type="PackedScene" uid="uid://bdp4gvtih2bec" path="res://menu.tscn" id="1_tkt5r"]

[node name="MyLevel" type="Node2D"]

[node name="Menu" parent="." node_paths=PackedStringArray("nodes") instance=ExtResource("1_tkt5r")]
nodes = [NodePath("VBoxContainer/Label"), NodePath("VBoxContainer/Label2"), NodePath("VBoxContainer/Label3")]

This leads to potential desyncing of the properties, e.g. after adding a new entry to nodes within the menu.tscn and saving it, the nodes saved in my_level.tscn remains unchanged.


Exported nodes in a child scene will be reflected in the parent scene file when saving a scene, BUT only if the parent scene is opened in the editor.

"BUT only if the parent scene is opened in the editor"
I can reproduce with no other scenes opened (v4.3.dev6.official [89850d5]):

  1. Open the MRP, no scenes are auto opened.
  2. Create new scene (root node), instantiate menu.tscn as child.
  3. Save the scene.
  4. Inspect created <new_scene>.tscn file, the node paths from menu.tscn are duplicated in there.

@Sauermann Unlabelling topic:export as it's not for @export annotation related issues, it's for the export system and templates (exporting the project to executable etc.). 🙂

@kleonc kleonc added this to the 4.3 milestone May 7, 2024
@Cronos87
Copy link
Author

Cronos87 commented May 7, 2024

Thank you for digging into it! I wasn't able to reproduce this when the scenes are closed, but maybe I missed something. That's good to know.

@akien-mga
Copy link
Member

If someone has time to bisect, this should be done between 4.2-stable (good) and 9d1cbab (bad). Interestingly, 4.3-dev1 was a very conservative snapshot with mostly the same contents as 4.2.1-rc1, so it must be one of the few commits which weren't cherry-picked (we did merge a handful of "riskier" ones in the first snapshot to get a headstart on regression testing).

@matheusmdx
Copy link
Contributor

Bisecting points to #83343 as the culprit

image

@akien-mga
Copy link
Member

CC @warriormaster12 @KoBeWi

@krdluzni
Copy link

krdluzni commented May 8, 2024

Minor note from my own run-in and experimentation with this issue: It does not seem to affect @exports of NodePaths, only @exports of Nodes themselves.

@lostminds
Copy link

I've run into this as well and my interpretation of the issue is that when instancing a scene (child.tscn in parent.tscn) with exported nodepaths on child.tscn it now always saves the nodepaths in parent.tscn as well, as if they had set overrides in parent.tscn. For example, even without overriding the my_node_path export var parent.tscn will contain:

[node name="Child" parent="." node_paths=PackedStringArray("my_node_path") instance=ExtResource("1_v8w3q")]
my_node_path = NodePath("Node1")

This is problematic since if I then change the my_node_path export var in child.tscn the instance in parent.tscn in will not update as it will use the nodepath set in parent.tscn as an override. However, if I open parent.tscn in the editor and resave it, it will update the nodepath to match child.tscn. So somehow the editor is keeping track of that this nodepath should not be an override even if it's saved as one and applied as one when the scene is run. But the real issue to me seems to be that it's saved there at all when there's no override set.

If instead I edit parent.tscn in a text editor and just remove the my_node_path = NodePath("Node1") line it will behave as expected, using the nodepath set in child.tscn as expected with no overrides without needing to open or re-save parent.tscn on changes. This is also consistent with how overrides of other export var types work, and how I'd expect it to work for nodepaths as well.

I think this could also be the root issue behind the node duplication issues #83343 above was originally meant to fix. Since if the nodepaths are seen as overridden in instances when they are not this could I think cause exactly these types of issues when duplicating these instances.

@KoBeWi KoBeWi linked a pull request May 18, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants