godotengine / godot

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

Reloading a scene won't reset exported custom resource properties #57268

Open Gnumaru opened 2 years ago

Gnumaru commented 2 years ago

Godot version

3.4.2 and 4.0alpha1

System information

Windows 10 x64

Issue description

When you reset a scene (Eg.: get_tree().reload_current_scene() or get_tree().change_scene(get_tree().current_scene.filename)), even if you wait some time between unloading and reloading the scene (Eg.:, switching to an empty scene, waiting some time, then switching back to the previous scene) the properties for custom exported resources will retain the values they had right before the scene was unloaded (supposing those resource instances are embedded in the scene file, not created at runtime).

Let me describe the scenario. You have some scene, wich contains a single node with a script. This script exports a resource wich you instance in the inspector using a custom resource you created in another script. This custom resource has some property of any type (the exported type doesnt matter). If you load this scene, change the resource property, and then reload the scene, the resource property value will not be reset, it will be like it was right before the scene was reloaded.

Steps to reproduce

  1. create a new project
  2. create a resource script
  3. export a property of any value
  4. create a new scene
  5. create and attach a new node script onto the scene root node
  6. export a resource property
  7. in the inspector, instantiate a new instance of the previously created resource script
  8. set some value for the resource instance property
  9. in the node script, in the ready function: wait a frame, change the resource property value, wait a frame, reload the scene with get_tree().reload_current_scene() or get_tree().change_scene(get_tree().current_scene.filename)
  10. when the scene reloads and the ready function is called again, you may see that the initial value for the resource's property is not the one provided in the inspector, but the one set in the code prior to reloading the scene. This can be seen either by printing the value or debugging.
  11. beware that if you are not using the attached project and you did not set any breakpoint, you will need to implement something to prevent an infinite loop of scene reloading.

Minimal reproduction project

godot 3 test.zip

godot 4 ExportedEmbededResourcePropertyWontResetOnSceneReload.zip

Remarks

On godot 3 (not on 4), after reloading the scene, the newly created objects have the same instance ids they had prior to reloading the scene. Is this the expected behavior? Is godot deterministic and always produces the same ids given the same initial state upon scene loading?

It should be noted that setting resource_local_to_scene to true on the resources prevents the unwanted behavior. This should be obvious for resources saved to disk since loaded resources are cached if I'm not mistaken, so a modified object will obviously retain its value across scene reloads. But this is counter intuitive for embedded resources because, since it is embedded to the scene, the user would expect its properties to be reset when the scene is reset in the same way it happens with any other non resource export like strings, dictionaries etc.

Calinou commented 2 years ago

Related to https://github.com/godotengine/godot/issues/24646 (possible duplicate)?

Gnumaru commented 2 years ago

Although #24646 is dealing with tool scripts, it does seem to be related, if not the same problem. But note that my report deals only with runtime, not the editor at all, or not even with saving resources to disk and loading them again. I tried creating a desktop export of the attached project, and the bug is still reproducible there.

What's more: even though I previously said that the problem would occur "even if you wait some time between unloading and reloading the scene", I have just circumvented the problem (in the attached project) by doing just that: Load an empty scene, wait a single frame, then load back the previous scene. At least on the attached project it works, but on the project I'm working the exact same workaround doesn't work =(, so I'm not able to use this. This workaround also works in the attached project when exporting a desktop build.

Gnumaru commented 2 years ago

I was able to pin out my specific problem. This does not solve the bug, but at least now I'm able to use the 'change to intermediate scene then wait one frame then load the other scene' workaround.

My specific problem was somehow related to setting the node owner of a resource as a property of the resource. In several resource scripts I'm creating a var parent:Object property that I then initialize to the node or resource that 'owns' that resource as a property in it's script. By doing this I'm able to use get_node and other things from a resource instance and I'm also able to consecutively get resource parents until I reach the node that owns a 'resource tree'.

For some reason, when I stopped doing this I was able to, in my main project, use the intermediate scene change workaround as I did with the attached 'bug demo' project.

Maybe this could interfere somehow in object disposal? Maybe the resource object does not get freed for some reason?

I would also like to notice that the instance id thing does not seem to be related to the problem at hand. With or without putting node references inside resource scripts, both the instance ids between consecutive scene loads are different AND they are the same between consecutive RUNS (play the scene, close the game instance, play the scene again). edit: please ignore.

Dylan-George commented 2 years ago

I think I've encountered this same issue but a slightly different manifestation. (Godot 3.4.2 stable)

I have my main level and a player inside of it. That player has an AnimationPlayer with tracks that I customize at runtime. If I call get_tree().reload_current_scene(), the AnimationPlayer tracks are still modified and do not reset. If I call get_tree().change_scene("res://new_scene.tscn") to a different scene with an instance of the same player class, once again the AnimationPlayer track values do not reset. However, if I use the method you described above of changing to an empty scene, waiting a frame, and then changing back, then the AnimationPlayer does reset as expected.

I'm relatively new to Godot, but this seems like a pretty big deal unless I'm doing something wrong.

Gnumaru commented 2 years ago

still replicates on 3.5 and 4 beta 6

dmaz commented 1 year ago

I can confirm odd behavior when using change_to_scene_file or packed. adding a wait for the next process frame does fix many of them but not all of them depending on the complexity of the scenes.

I was adding a loader, menu, game; loop using change_to_scene but had to move on to just adding / deleting child scenes from a controller node which I think is better anyway.

TimCoraxAudio commented 11 months ago

Any update on this? It's causing some big issues in my game

jaimejaime19 commented 10 months ago

I can confirm this issue persists in 4.2 stable. I have a node which exports nested resources (3-4 layers deep if that matters). If the node is left in the scene when reloading, the resources will maintain the previous values they had. One solution that worked for me was to queue_free() the node when it was done with its actions. Then when the scene reloaded, the node is recreated and the resources have new values (at least in my testing).

Looking at the remote tree when running the project, the remote ID for the node is always the same before reloading the scene, and always different after reloading the scene. This is the case even if my queue_free() solution is applied - no idea why it fixes the problem, but it does.

ydeltastar commented 7 months ago

The problem is how SceneTree's scene-changing works internally. (reload_current_scene() is just a shortcut for change_scene_to_file(current_scene.filename))

The SceneTree first instantiates the new scene in memory while the current one is just removed from the tree. It remains in memory and will only be deleted in the next process step.

Resources are reference counted and cached. A Resource will not be deleted as long as a scene holds a reference to it and trying to load it from disk returns the in-memory version instead.

In other words, when the new scene is instantiated, it will get a reference to the Resources in memory instead of reloading them from the disk since the previous scene still exists. So get_tree().reload_current_scene() resets the node tree but the Resources objects are the same.

I'm not sure if it's a design decision or a bug, but this behavior is counterintuitive. One would expect Resources to be flushed as well if there are no other references to the Resources while reloading a scene, whereas the current behavior should be an optional parameter or done manually.

get_tree().unload_current_scene(), for example, has the expected behavior of immediately deleting the current scene. It can be used as a workaround for full reloading.

var scene_path = get_tree().current_scene.scene_file_path
get_tree().unload_current_scene()
get_tree().change_scene_to_file(scene_path)
brno32 commented 5 months ago
var scene_path = get_tree().current_scene.scene_file_path
get_tree().unload_current_scene()
get_tree().change_scene_to_file(scene_path)

The C# equivalent of this code crashes my game.

I got it working with this nastiness, where CustomResource[] stats;

// after loading the scene and spawning my characters
enemy.stats[0] = enemy.stats[0].Duplicate();

With this, when I change scenes my resources are properly reset. Without it, any modifications to the resource's properties persist across scene changes, despite having having "local to scene" checked in the editor for the resource.

roa-nyx commented 5 months ago

I'm having the same issue in 4.2 and 4.3beta2. I reworked the solution from https://github.com/godotengine/godot/issues/57268#issuecomment-2067805358

    var tree = get_tree()
    var scene_path = tree.current_scene.scene_file_path
    tree.unload_current_scene()
    tree.change_scene_to_file(scene_path)

However this STILL doesn't work for me. It seems like subscenes are somehow kept and not reset?

Still playing around to try to create a minimally reproducible project.

BlackBarman commented 3 weeks ago

any updates on this?

jerrywzy commented 1 week ago

I am still facing a similar issue on Godot 4.3, with an Array of Objective Custom Resources not resetting when going Load Game for Level A > Increment some Objectives > Quit to Main Menu and start new Level A with change_to_packed_scene > the Objectives array progress does not reset, remaining same as the Loaded Game's. I ended up just creating a separate StartingObjectives array and doing Objectives.assign(StartingObjectives) every time I load my level.

Gnumaru commented 1 week ago

Is been a while since I openned this issue.

Nowadays I understand that maybe this is not a bug, but an expected (but actually really undesired) behavior.

When I first oppened this issue I only described it in one way, but it was in fact not the exact way I was experiencing the bug, it was only the simplest way of explaining and reproducing it.

Nowadays, if you open up my godot 4 example in the description, you will notice that the bug is not reproducible with that specific example. So I did another minimal, more graphical, example that is able to reproduce the bug, albeit in a different manner from the way I first described it (I'll update the issue description up there), but wich is closer to the way most users will experience it

DuplicateEmbededResourcesProposal.zip

Here's also a video of it here

https://youtu.be/L1dwVBRqU7M

and here

https://github.com/user-attachments/assets/fdfa6f58-4dbe-403e-8891-c029fb05340b

edit: I haven't digged up godot's scene loading code, but I'm supposing the problem is as follow:

1) When the scene (wich is a resource of type PackedScene) changes, it get's cached, because all resource loading is cached by default. 1) In order for the scene to load, all the resources it uses (like an enemy scene instance) are also loaded, an thus also cached. 1) The enemy scene script in my example, instead of having a numeric health export, uses a resource for that. In the original enemy scene, this resource instance does not set resource_local_to_scene to true. Because of this, every instance of the enemy will share a reference to the same resource object. 1) But why the values remain changed while changing scenes? because the resource instance for the health export "lives" inside the enemy scene (wich is a resource) and gets cached along with it. So, when we change the health of the enemy, we are in fact changing it for the health export wich is cached alongside the enemy scene, and thus is shared with every enemy instance that does not duplicate it either at load time (when resource_local_to_scene is true) or explicitly by script. 1) why does the third enemy of the first stage does not share health with the others? because it was made local in the editor, so it is in fact completely unrelated to the original enemy scene, albeit presenting an identical structure to the other enemies. 1) hence, the correct way (also the intended way, the designed way) of avoiding this issue is setting resource_local_to_scene to true in the resource that is intended to be shared among scene instances.

That's why I said this is maybe not a bug. But at the same time it is a really undesired behavior. Using embedded resource instances is something extremely common, but actually wanting scene instances to share the embeded resource instance by default is actually really uncommon (I have not data on that, that's just my felling as user). When I started using godot in 2019 that was in fact one of the great things I found it had compared to unity, wich I used since 2014: In unity you cannot embbed a ScriptableObject, but in godot you can, wich makes it very convenient to setup mor complex data for scripts.

Hence, I think that a good improvement for the engine that could "fix" this "maybe-not-a-bug-but-actually-a-bad-user-experience" is that we could, by default, every time the engine changes scene, to duplicate every instanced sub-scene resource even if resource_local_to_scene is set to false. To avoid breaking compatibility, we could have a project setting like "duplicate_embedded_resources_when_instancing_scenes" wich would default to false and make everything work as it worked previously. But when it is set to true, every time we change scenes, instantiate packed scenes or load resources, every sub-resource will be duplicated as if resource_local_to_scene where set to true on it, even if it is set to false.

TimCoraxAudio commented 1 week ago

I've managed to get the workaround suggested by @ydeltastar above to work for me.

var scene_path = get_tree().current_scene.scene_file_path
get_tree().unload_current_scene()
get_tree().change_scene_to_file(scene_path)

It produces this error:

E 0:00:03:0116   level_end.gd:14 @ restart(): Object <Object#60867741445> was freed or unreferenced while a signal is being emitted from it. Try connecting to the signal using the 'CONNECT_DEFERRED' flag, or use queue_free() to free the object (if this object is a Node) to avoid this error and potential crashes.
  <C++ Source>   core/object/object.cpp:2006 @ ~Object()
  <Stack Trace>  level_end.gd:14 @ restart()

The problem was that if you try to unload the current scene while it's emitting a signal (which could be calling this very code), then the game crashes. Using object.call_deferred works though:

var tree := get_tree()
var scene_path := tree.current_scene.scene_file_path
tree.call_deferred("unload_current_scene")
tree.call_deferred("change_scene_to_file", scene_path)

This may solve the problem for those who have had problems