godotengine / godot

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

Loss of changes on branches with multiple nested scenes (when duplicating or exporting as new scenes) #89824

Open L30n4rd0-4rg3nt1n0 opened 7 months ago

L30n4rd0-4rg3nt1n0 commented 7 months ago

Tested versions

-Reproducible in: Godot v4.3.dev (c9c17d6ca)

System information

Godot v4.3.dev (c9c17d6ca) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3070 (nvidia) - Intel(R) Core(TM) i7-4790K CPU @ 4.00GHz (8 Threads)

Issue description

Duplicating a branch with multiple nested (and edited) scenes results in changes to the second level of nesting being lost. This also happens when the branch is exported as a scene.

Steps to reproduce

To illustrate what I'm saying, I'm sharing a project with several scenes (I also show some photos).

-cube.tscn: A rigid body with BOX mesh. -Platform.tscn: A rigid body with a PLANE mesh. -cube_on_platform.tscn: The cube (cube.tscn) is placed on the platform (Platform.tscn). 00

-scene.tscn: I use an instance of "cube_on_platform.tscn", but I rotate it and change the color of the cube. For that I recursively set the "editable children" option twice. 01

-scene_2.tscn: Same as "scene.tscn" but duplicating the modified branch. In principle, the changes are applied to the duplicate. But if I save, close and reopen, it only survives the rotation but the color edition is lost (which corresponds to the second level of nesting), it back to original setting . 02

-scene_3.tscn: Same as "scene_2.tscn", but after creating the duplicate I apply the "editable children" option again (since I realized that the children are closed although the changes are still visible). If I save, close and reopen, the changes now survive (unlike "scene_2.tscn"). 03

-scene_4.tscn: Like "scene_3.tscn" but I made a second duplicate (with the same procedure). I placed the two duplicates inside a node3d (renamed "cube_on_plantform_X2")... Then I duplicated cube_on_plantform_X2. I was forced to reapply the "editable children" option within that duplicate branch. Only then if I save, close and reopen, I don't lose the changes. 04

-scene_5.tscn: Like "scene_4.tscn" but I exported the duplicate of "cube_on_plantform_X2" as a scene, creating the scene "cube_on_platform_x_2_EXPORTED.tscn". When I do this immediately, that branch is replaced by a node that is an instance of that scene. In that scene the color change is lost (second level of nesting). 05

Of course, if you want you could edit "cube_on_platform_x_2_EXPORTED.tscn" to apply a color change to the cubes. But it is clear that there is a problem when doing the export, that even an already saved change is lost.

Minimal reproduction project (MRP)

bug_nestig.zip

eswartz commented 7 months ago

I am seeing a variant of this too, but with simpler steps, perhaps. I don't think it even needs editable children. (I hope this is the same issue as the OP's! I can open a new issue if needed, but I try to avoid adding duplicates if possible...)

This issue is new after 4.3dev5 and still reproducible in master as of 7d151c83811f8ac8873439826c16d88c83aba12f.

In this example project with a MeshInstance3D containing a StaticBody3D and a CollisionShape3D, when using "Save Branch as Scene" on the StaticBody3D, the "shape" from CollisionShape3D and the "physics override material" from StaticBody3D are lost -- i.e. all the sub_resource entries are missing from the new .tscn file.

Original scene: scenebug-before.zip

After using "Save Branch as Scene": scenebug-after.zip

It's easy to spot in practice because in the new scene, CollisionShape3D shows a warning where it didn't before.

eswartz commented 7 months ago

According to git bisect, maybe 9851c1bdd893a6194fdbf1820d2fa07449d4f9fb is the root cause?

akien-mga commented 7 months ago

CC @warriormaster12 @KoBeWi

warriormaster12 commented 7 months ago

Dammit 😔

warriormaster12 commented 7 months ago

@eswartz was it just a guess or did you do a quick test if the issue disappears when reverting the commit?

eswartz commented 7 months ago

@warriormaster12 Yes, I have the commit reverted locally and at least my case works as expected. Not sure about op's.

warriormaster12 commented 7 months ago

Aight, found the issue.

warriormaster12 commented 7 months ago

@eswartz @L30n4rd0-4rg3nt1n0

https://github.com/warriormaster12/godot/tree/regress-dup

Can you test against this commit?

eswartz commented 7 months ago

@warriormaster12 It works for me, in my original case and in another arbitrary node in one of my scenes.

Edit 1: Ack, I tested a different variant from what I built. Trying again :)~

Edit 2: OK, works fine for me again/still. Sorry for the scare!

Thanks! I'll step back and let OP provide his findings.

warriormaster12 commented 7 months ago

Awesome, waiting for OP's response before doing a PR.

L30n4rd0-4rg3nt1n0 commented 7 months ago

@eswartz @L30n4rd0-4rg3nt1n0

https://github.com/warriormaster12/godot/tree/regress-dup

Can you test against this commit?

@warriormaster12 I downloaded the branch indicated by the link... It does not solve the bug. The problem I pointed out in "scene_2.tscn" continues to happen. That is, I can duplicate a modified instance of "cube_on_platform.tscn" (in which I have rotated and changed the color of the cube) but after saving, closing and reopening I confirm that I have lost the color modification. The change in the second level of nesting is lost.

THE BUG IS NOT FIXED

warriormaster12 commented 7 months ago

@L30n4rd0-4rg3nt1n0 Does this bug happen with a version of godot before my pr was merged. The one that had a regression?

warriormaster12 commented 7 months ago

@akien-mga this issue should be reopened. In short the bug is that in a certain nesting level editable children setting is not applied, therefore changes that a user has done to child objects will be lost after saving.

My mistake was assuming that this bug was related to @eswartz. My bad.

warriormaster12 commented 7 months ago

When I tested in Godot 4.1.1 the behavior of activating”editable children” is different. It only applies to the first scene and doesn’t show in the ui that the nested scene is also editable. So I guess underlying logic works correctly but something in between that version and 4.3 changed that caused the editor to show nodes that aren’t suppose to be editable as editable and it only gets fixed when duplicating, saving the scene and reopening it.

L30n4rd0-4rg3nt1n0 commented 7 months ago

@L30n4rd0-4rg3nt1n0 Does this bug happen with a version of godot before my pr was merged. The one that had a regression?

@warriormaster12 I built the branch from the link you had shared. Also yesterday I cloned the main repository and did a build with it. The result was the same.

warriormaster12 commented 7 months ago

Aight. We need then figure out which prior version introduced incorrect behavior when enabling ”editable children”. As far as I understand the correct behavior is that enabling “editable children” shouldn’t allow users to edit sub scenes by default.

L30n4rd0-4rg3nt1n0 commented 7 months ago

Aight. We need then figure out which prior version introduced incorrect behavior when enabling ”editable children”. As far as I understand the correct behavior is that enabling “editable children” shouldn’t allow users to edit sub scenes by default.

@warriormaster12 I may not fully understand the philosophy behind the design of this engine, or what the correct way to use the nodes should be. But I can't imagine how I could do some things if subscenes couldn't be edited. I understand that a sub-scene is for all practical purposes a child node... and if a child node can be edited, why couldn't I edit a sub-scene?

For example, taking into account the project I shared at the beginning of this post, how could I change the color of a cube (which is over a platform) without having to create a new scene that only makes that change?

KoBeWi commented 7 months ago

When I tested in Godot 4.1.1 the behavior of activating”editable children” is different. It only applies to the first scene and doesn’t show in the ui that the nested scene is also editable. So I guess underlying logic works correctly but something in between that version and 4.3 changed that caused the editor to show nodes that aren’t suppose to be editable as editable and it only gets fixed when duplicating, saving the scene and reopening it.

I can't reproduce this. Only the instance with editable children is editable, its sub-instances are not.

I can however reproduce the issue described by the OP. In Scene_2, when you duplicate CUBE_ON_PLATFORM, it gets duplicated with all values still changed, but the changes are lost when scene is reloaded. For some reason, one of the instances doesn't carry the editable flag when duplicated:

https://github.com/godotengine/godot/assets/2223172/aa087f5a-1229-45a5-8ae1-368c26423454

In this case data loss in the last instance is expected, because it's not editable (expected i.e. in the state presented by the editor, where its children are not visible in scene tree). You can prevent losing the modifications by enabling editable children manually.

So the bug here is that editable scenes sometimes become uneditable? The follow-up on scene 5 is confusing to me, as is warriormaster's description.

But I can't imagine how I could do some things if subscenes couldn't be edited.

Enabling editable children should only allow editing the children owned by the scene, not its sub-scenes (unless you also enable this option for them).

warriormaster12 commented 7 months ago

Let me try to explain my thoughts a bit better.

My understanding based on the test that I did with 4.1.1 is that enabling editable children should only give access to the nodes that belong to that scene. In th OP’s scene2 example that would be PLATFORM-StaticBody and CUBE-Rigidbody.

The bug is that CUBE-Rigidbody scene’s children are also accessible even though editable children for that scene isn’t enabled. Therefore, when you duplicate CUBE_ON_A_PLATFORM node, those children don’t show up because they weren’t suppose to in the first place and when you reopen, the changes that you made for those nodes are lost since they are part of a scene for which you haven’t enabled editable children.

warriormaster12 commented 7 months ago

Maybe context menu should have something like ”editable children all” or something like that which would enable recursivly for all child scenes ”editable children”

KoBeWi commented 7 months ago

Ok I get what is happening. Here: image

The bug is that CUBE-Rigidbody scene’s children are also accessible even though editable children for that scene isn’t enabled.

It is though. You can see it in the context menu and inside the scene itself: [editable path="CUBE_ON_PLATFORM-Node3D/PLATFORM-StaticBody3D/CUBE-RigidBody3D"] The layout is confusing, because CUBE-RigidBody3D is not part of the platform scene. The platform scene has editable children disabled. This is the actual structure of this scene (I renamed some nodes to make them easier to distinguish): image The rigidbody is a local child of the instance, which means it belongs to Node3D not the platform. When you duplicate the CUBE_ON_PLATFORM, the engine probably also gets confused about the hierarchy and doesn't carry the editable flag.

warriormaster12 commented 7 months ago

👍 makes sense I guess.

L30n4rd0-4rg3nt1n0 commented 7 months ago

The bug is that CUBE-Rigidbody scene’s children are also accessible even though editable children for that scene isn’t enabled.

@warriormaster12 I don't know if I'm understanding what you're saying.

I first enabled "editable children" on an instance of "cube_on_platform.tscn"... One of those children is an instance of "cube.tscn"... After the previous step I enabled "editable children" for the instances of "cube.tscn". That is, I enabled "editable children" twice in succession.

When you duplicate or export as a scene, the second editing permission is lost.

akien-mga commented 2 months ago

Is this still reproducible in 4.3.rc2 or later?

NandoBroquiGames commented 1 month ago

Has anyone found a solution for this yet?

(Btw, I found my issue, it was due to the reset anim of the AnimationPlayer) :p