godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.86k stars 21k forks source link

PackedScene behaving unexpectedly #45886

Open TimTheToad opened 3 years ago

TimTheToad commented 3 years ago

Godot version: 3.2.3 stable

OS/device including version: Windows 10 64-bits

Issue description: I might be missing something but when saving a scene in a PackedScene, I need to set the owner of instanced scenes to be the same as the scene I'm packing, right? I also have to set the owner of all the children of the instanced scene otherwise these will be discarded and not packed, right?

If this is the correct way to pack a scene, then there might be a bug or I'm missing something(probably the case)

What I'm doing: I have set up the main scene with a Node2D as "root" carrying a Kinematic2D with a sprite and a collisionShape2D as children. so Main Scene: -MainScene ----Player -------CollisionShape2D -------Sprite

I have also set up a secondary scene, the scene I'm instancing during _ready() the Secondary Scene Tree is identical to the MainScene. So Secondary Scene: -SecondScene -----FakePlayer --------CollisionShape2D --------Sprite

As I just mentioned, I instance the SecondScene in MainScene _ready() function. I add this scene as a child on Player. I set the owner to "self.get_tree().current_scene", as this is the scene I'm packing. I do this for all the children in the instanced scene, as said in the docs. I pack my Scene with OK results. I change the scene to the PackedScene with a "change_scene_to(packed_scene)"

Save function: func SaveState(): saved = true packed_scene = PackedScene.new() if packed_scene.pack(get_tree().current_scene) == 0: if ResourceSaver.save("res://saved.tscn", packed_scene) == 0: print("Saved Scene") else: print("Error saving Scene") else: print("Error packing Scene") return image

Load function func LoadState(): if get_tree().change_scene("res://saved.tscn") == 0: print("Loaded Scene") else: print("Error loading Scene") return image

I expect the loaded scene to look like this: -MainScene ---Player -----CollisionShape2D -----Sprite -----SecondScene -------FakePlayer(correct values) ----------CollisionShape2D ----------Sprite

What I get:

-MainScene ---Player -----CollisionShape2D -----Sprite -----SecondScene --------FakePlayer(Incorrect values) ----------CollisionShape2D ----------Sprite --------FakePlayer(Correct values) ----------CollisionShape2D ----------Sprite

I end up with two copies of the first child of the instanced scene. I didn't mention that I change a variable in FakePlayer before saving to make sure the values are carried with the save. That's what I mean with "Correct/incorrect Values). Incorrect Values are the default values.

The culprit and the issue lie within the .tscn file. specifically this line here: [node name="SecondScene" type="Node2D" parent="Player" instance=ExtResource( 2 )] If I remove "instance=ExtResource( 2 )" before loading the scene, it does not create a copy of "FakePlayer" and it's children with default values.

Steps to reproduce: Start Project Save (press "S") Load (press "L") Look at the remote tree.

Minimal reproduction project: TestProject.zip

Edit: Added images of code as the code got squished.

rsubtil commented 3 years ago

I think it's happening because in the script you are instantiating a SecondScene. When you load the saved scene, that script gets called again, instantiating another SecondScene, thus the "duplicated" one appearing.

Calinou commented 3 years ago

@TimTheToad Did you try Ev1lbl0w's solution above?

nikomartn commented 2 years ago

Hi, I'm currently studying this, and it seems to me that it's the documentation what is wrong.

I add this scene as a child on Player. I set the owner to "self.get_tree().current_scene", as this is the scene I'm packing. I do this for all the children in the instanced scene, as said in the docs.

I do the same that the OP does in my own project, and indeed the nodes inside the instantiated scenes become duplicated.

If you inspect the owner (from code, sadly it doesn't appear at the inspector), of internal nodes of a scene, it shows the parent node of that instantiated scene.

This is what I think really happens, a PackedScene stores as a remote resource any node that you have instantiated from another PackedScene, and just writes that all of that resource should be instantiated with [this local resources of that node]. For example "This is a remote PackedScene, instantiate it with {transform.origin : xxyyzz}". And so, the "responsibility" of instantiate any children is of that node.

Just like in the scene editor, when you load a scene from another place in the project, you can't see the children of it, and thus, only change the exports of the root node.

Then, since you are manually crawling, as the docs says, to make all nodes of that scene owned by the to-be packed node, they get written, along with the blueprint that the imported scene is. "Instantiate this scene, and all of this other nodes"

The docs should explain that you should change the owner of the Nodes you create [with new()] and the Instantiated scenes nodes [you load with add_child(load())] and mimic what you see at the editor tree.

This is all speculative from the behaviour I could observe, but if a dev could correct or assert me, I could edit the docs so they don't bring to confusion.

KoBeWi commented 4 months ago

I might be missing something but when saving a scene in a PackedScene, I need to set the owner of instanced scenes to be the same as the scene I'm packing, right? I also have to set the owner of all the children of the instanced scene otherwise these will be discarded and not packed, right?

No. Instance's child nodes use their root as owner. By setting the owner to parent scene, you are saving them as part of the parent scene, thus the nodes are loaded twice - from their instance and from the parent scene.

We could document that, but not sure where.