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

Trying to manually load new scenes to toggle back and forth between two scenes fails #83079

Open
thorbrian opened this issue Oct 10, 2023 · 10 comments

Comments

@thorbrian
Copy link

thorbrian commented Oct 10, 2023

Godot version

v4.1.1.stable.mono.official [bd6af8e]

System information

v4.1.1.stable.mono.official [bd6af8e]

Issue description

I tried to make two menu scenes, where I could go from one scene to another and back (a "MainMenu" that loads the "Level" and the "Level" can return back to the "MainMenu")
I tried to use manual scene switching, where the new packed scenes are instantiated and added, and the old scene is removed (was doing this because I was hoping to eventually make the old scene animate out while the new scene is animating in)
I tried to make the references to the packed scenes be by a packed scene reference (@export'd var of type PackedScene) editable in the editor (I did this so that I could, at some point, move around my files and have the references still be connected)
When I did that, when the "Level" scene got loaded from the "MainMenu", the instantiated level lost it's assigned reference to the "MainMenu", so it couldn't return to main menu. If I make the "Level" the main scene, then it can load "MainMenu" using it's reference, but "MainMenu" then loses it's reference

So I tried doing it instead with hardcoded paths to the PackedScenes in preloads, and then when I toggle back and forth a few times, it fails with some kind of circular reference error

Finally, if I try to reopen the project, for some reason the editor claims Level.tscn is invalid/corrupt, when it is not. I can still run everything (with the same bad behavior) it's just the editor refuses to edit Level.tscn

So the 3 problems are:

  1. script vars of "@export var target_scene : PackedScene" are lost when I "target_scene.instantiate()" a PackedScene reference which itself has one of these vars
  2. Going back and forth between scenes with scripts with have "var preloadedScene : PackedScene = preload("path")" references to each other eventually errors out with a circular reference error
  3. Having two scenes with scripts with "@export var exportedMenu : PackedScene" where assigned to each other makes one (or both) of them considered invalid by the editor, on re-opening the project

Steps to reproduce

Open the attached project. Notice how it claims Level.tscn is invalid, but the file format is fine

Run the project, which takes you to the main menu. Click the top button to go to the Level scene (which loads fine now, despite the editor saying it's invalid), and now inspect the scene remote'ly and see that it doesn't have the reference to MainMenu.tscn in it's "exportedMenu" variable, despite the packed scene file having the reference, so the top button to return to the main menu doesn't work

Run the project again, which takes you to the main menu. Use the bottom buttons to go back and forth between MainMenu and Level, and notice after a few times, it fails with an error about a circular reference

Minimal reproduction project

SceneToggleErrors.zip

@Calinou
Copy link
Member

Calinou commented Oct 10, 2023

Try using load() instead of preload() to avoid cyclic reference issues.

@thorbrian
Copy link
Author

Thanks that avoids problem 2. Why would two objects preloading each other cause an issue? Isn't the whole idea of referencing another resource so that you don't have to have your own copy though? like wouldn't a second preload call for the same object just return the same object as the first call, without having to load a new copy of it? It's very strange that it works for like 4 transitions before erroring. If it was truly a cyclic reference problem, why wouldn't it fail to work even once?

@thorbrian
Copy link
Author

Also, I still care about problem 1, which has nothing to do with the preload or load or any of that. Problem 1 is especially important to me because I don't want to count on hardcoded file paths, if I can let the editor reference things by id for me instead, but the loaded menus are losing their reference to the other menu when they are instantiated. Why is that?

@thorbrian
Copy link
Author

I really think there's a bug here - I get that you don't think I should have done what I did, but it's a completely natural thing to want to make two scenes that reference each other, there is no good reason why it should fail, and the behavior of the engine when this occurs is totally bizarre and confusing

@thorbrian
Copy link
Author

I saw this bug tracker for 4.2: #80877

It looks like there is some support for circular dependencies in 4.2?

@thorbrian
Copy link
Author

I tried opening my test project in 4.2 dev, and it fails to open either of the two scenes with circular references, with this set of errors:
scene/resources/resource_format_text.cpp:284 - res://Level.tscn:14 - Parse Error:
Failed loading resource: res://Level.tscn. Make sure resources have been imported by opening the project in the editor at least once.
scene/resources/resource_format_text.cpp:284 - res://MainMenu.tscn:14 - Parse Error:
Failed loading resource: res://MainMenu.tscn. Make sure resources have been imported by opening the project in the editor at least once.
editor/editor_data.cpp:626 - Index p_idx = 1 is out of bounds (edited_scene.size() = 1).

note that the first scene that it outputs the error for is the one referenced by the scene I tried to load, the second scene it outputs the error for is the one I tried to load

So weird confusing behavior with circular references between packed scenes is still happening in 4.2 dev

@YuriSizov
Copy link
Contributor

YuriSizov commented Nov 20, 2023

It looks like there is some support for circular dependencies in 4.2?

For scripts, not for other types of resources like scenes. Circular dependencies between resources are not allowed.

@thorbrian
Copy link
Author

The line 14 in the tscn files are the ones that have the assignment of the exported packed scene reference to the other scene

so for example, in the "Level.tscn" scene file, line 14 is:
exportedMenu = ExtResource("2_e8wkl")

which refers to the main menu:
[ext_resource type="PackedScene" uid="uid://bvocpsanus0yb" path="res://MainMenu.tscn" id="2_e8wkl"]

where the variable that has that assignment is declared and exported like this:
@export var exportedLevel : PackedScene

where I had assigned that exported var in the editor to the Main Menu

To repeat again why I think this should be considered a bug, something to fix, is that it's natural for users to want to make it so the UI flow can be set through the editor, and the Godot docs themselves recommend setting up scene transition and references the way I did in order to accomplish that, with an exported PackedScene var. Also, once you are assigning resources in the editor, it's again totally natural to end up making circular references, the example of returning to main menu is an obvious one, but there are lots of others. Then once somebody gets in that state, the scene behaves weird and the resource start failing to load. So just natural behavior, trying to use the mechanisms the engine provides as intended leads to things breaking without the user knowing why

As to why I think it makes sense to fix, there really shouldn't be any fundamental reason why with resource references in exported vars, circular references should be a problem at all. I get why with the code being written to immediately load any referenced resource while it's loading the resource with the reference, it would become an circular dependency problem. However there are lots of other ways to do the loading process that handle circular references fine. For example, if when the loading process for a resource was initiated, it was constructed enough such that it could be used in a reference, then there'd be no circular loop (so loading MainMenu would first build it enough to make it referenceable, then it would load all it's references, which would start loading Level, which would first make Level referenceable then load all of level, which would be able to happily reference MainMenu and return, allowing MainMenu Load to complete, and everything would be good)

Why exactly is it that circular references between resources in exported vars would be a problem?

@thorbrian
Copy link
Author

For scripts, not for other types of resources like scenes. Circular dependencies between resources are not allowed.

Got it, thanks! I still feel the same way about this bug

@joelmortel
Copy link

Try using load() instead of preload() to avoid cyclic reference issues.

Thank you so much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants