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

Closed
Cronos87 opened this issue May 5, 2024 · 17 comments · Fixed by #92095 or #93430
Closed

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

Cronos87 opened this issue May 5, 2024 · 17 comments · Fixed by #92095 or #93430

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.). 🙂

@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 akien-mga moved this from Unassessed to Release Blocker in 4.x Release Blockers May 7, 2024
@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
Contributor

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.

@Cronos87
Copy link
Author

I'm experiencing the same issue with the 4.3.beta.2. Exported nodes are persisted in nested scenes.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 20, 2024

If you have scenes that were previously bugged you need to remove the non-default values.

@Cronos87
Copy link
Author

Cronos87 commented Jun 20, 2024

That's what I did. Here is a screencast; maybe I did something wrong.

CleanShot.2024-06-20.at.23.16.54.mp4

Thank you for the fast answer 🙏

@KoBeWi
Copy link
Member

KoBeWi commented Jun 20, 2024

Your scenes still show revert icon though. Revert them before saving.

@Cronos87
Copy link
Author

The ones in the video are exported from the root node and they are of type string; it doesn't contain the labels. Here is the correct one:

CleanShot 2024-06-20 at 23 25 41@2x

As you can see, there is no revert icon.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 20, 2024

Ok looks like the only fixed case is when a Node is exported, but not Array of nodes.

@Cronos87
Copy link
Author

Many many thanks for the quick investigation and MR @KoBeWi 🙏

@Cronos87
Copy link
Author

Cronos87 commented Jul 9, 2024

I confirm the issue has been solved for good with beta 3! Again, many thanks to everyone for this fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Release Blocker
9 participants