Godot: export var PackedScene crash when having circular references

[ebuchmann]

Godot version:

v3.1.alpha.caliou.8dd00ed
8dd00ed1762c4956b3231d709ce0d01ee9b306c8

OS/device including version:

MacOS, 2017 Macbook Pro 15 inch

Issue description:

I have an object called stairs and when going to those stairs it should teleport the player to a new scene. I exported a var and labeled it as a PackedScene. It works fine to reference it and create an instance inside the script, but if the scene that should be loaded also has an instance of the stairs and those stairs link back to the first scene I get an unknown crash. No console errors or anything, just Godot crashes and I have to re-open it.

Steps to reproduce:
I've managed to reproduce it in a new project: https://github.com/ebuchmann/PackedSceneCrashTest

The SceneSwitch scene is in both Scene1 and Scene2 and points to each other scene.

Minimal reproduction project:

PackedSceneCrashTest-master.zip

[LikeLakers2]

(I'm using v3.1.alpha.custom_build. 6f9aa87 on Windows 10 here.)

I'm able to replicate this, but I'd be inclined to say this isn't something I'd consider to be Godot's fault. What it's trying to do is load Scene1, which needs Scene2, so it tries to load Scene2, but then Scene2 needs Scene1, and it tries to load Scene1 (since it isn't already loaded), but Scene1 needs Scene2... to make a infinite story short, you've basically given Godot an infinite loop. (edit: I apologize if this seems a little Captain Obvious to you -- I didn't notice before commenting that you already referred to this as a "circular reference" in the issue title!)

If you really need the two to link to the other, I would suggest asking for a file path (as a String) instead of a PackedScene, so that Godot doesn't try to load the PackedScenes upon loading one of the scenes and thus get stuck in an infinite loop. It's not exactly an elegant solution, but it solves your issue and lets you get back to making your game.

That said, I still think this issue has some merit -- Godot should error when it finds itself loading things in an infinite loop, not crash!

[akien-mga]

Indeed, the editor should attempt to prevent such circular references by returning an error and stopped the circular reference.

We have similar issues in GDScript when using preload.

I guess the editor could keep track of already parsed scenes when preloading new scenes inside them, so that it can compare if the new scene to load matches one further up the dependency tree, and then abort.
Such a check would be costly so it would only be done in debug mode/the editor, so if you don't fix it your released game would still crash - but at this point there's only so much we can do to make it fail proof.

[bojidar-bg]

I think we should try to fix the crash rather than making the editor guard against it. An error in the Debugger tab when running is going to be nearly as helpful as a warning dialog when selecting.

[rzuf79]

I doesn't crash for me in alpha 4 and 5 but the editor shows a tscn loading error dialog ("Load failed due to missing dependencies") which is accompanied by an error being written a couple dozen times in the terminal console:
ERROR: set_path: Another resource is loaded from path: res://Scene1.tscn (possible cyclic resource inclusion) At: core/resource.cpp:79.

[Bombfuse]

I'm also having an issue with two scenes pointing at each other in an exported PackedScene reference. I'm confused as to why this wouldn't be allowed?

[bojidar-bg]

@Bombfuse Circular references are generally not allowed in Godot, as they mess up the reference counting used to reclaim unused Resource-s.

[nhold]

I don't think the intended behaviour should be a crash or even mess up any references. Referencing the game from the menu (To load to) and the menu from the game (To load to) should be a perfectly reasonable thing to expect to work.

When parsing the resources why can't we handle ensuring only one reference to the resource is loaded and skip loading it again if it has been loaded in the current sequence?

I.e: Psuedocode:

• Parse the resource and referenced resources for cyclic references
• Consider any resources that are cyclicly referenced partially loaded, load any variables that aren't apart of a cyclic reference
• Keep what variables rely on the cyclic reference in each resource and skip loading anything twice

• After all are loaded and\or partially loaded, go through each partially loaded resource and give eachother their resource reference

  

[tom-jk]

For reference (state of issue in 3.2.2.stable):

Opening the minimal reproduction project in 3.2.2.stable pops up a "Load Errors" dialog:

Scene 'res://Scene2.tscn' has broken dependencies:
res://Scene1.tscn::PackedScene

[OK]

And shows this in the terminal (running with --verbose):

Loading resource: res://Main.tscn
Loading resource: res://Scene1.tscn
Loading resource: res://SwitchScene.tscn
Loading resource: res://Scene2.tscn
ERROR: load: Resource: 'res://Scene1.tscn' is already being loaded. Cyclic reference?
   At: core/io/resource_loader.cpp:360.
ERROR: load: Resource: 'res://Scene1.tscn' is already being loaded. Cyclic reference?
   At: core/io/resource_loader.cpp:360.
WARNING: _parse_ext_resource: Couldn't load external resource: res://Scene1.tscn
   At: scene/resources/resource_format_text.cpp:175.

Running the project opens and closes the game immediately, with 11 errors:

ERROR: load: Resource: 'res://Scene1.tscn' is already being loaded. Cyclic reference?
   At: core/io/resource_loader.cpp:360.
ERROR: poll: res://Scene2.tscn:4 - Parse Error: [ext_resource] referenced nonexistent resource at: res://Scene1.tscn
   At: scene/resources/resource_format_text.cpp:440.
ERROR: load: Failed to load resource 'res://Scene2.tscn'.
   At: core/io/resource_loader.cpp:208.
ERROR: _load: Failed loading resource: res://Scene2.tscn.
   At: core/io/resource_loader.cpp:278.
ERROR: poll: res://Scene1.tscn:4 - Parse Error: [ext_resource] referenced nonexistent resource at: res://Scene2.tscn
   At: scene/resources/resource_format_text.cpp:440.
ERROR: load: Failed to load resource 'res://Scene1.tscn'.
   At: core/io/resource_loader.cpp:208.
ERROR: _load: Failed loading resource: res://Scene1.tscn.
   At: core/io/resource_loader.cpp:278.
ERROR: poll: res://Main.tscn:3 - Parse Error: [ext_resource] referenced nonexistent resource at: res://Scene1.tscn
   At: scene/resources/resource_format_text.cpp:440.
ERROR: load: Failed to load resource 'res://Main.tscn'.
   At: core/io/resource_loader.cpp:208.
ERROR: _load: Failed loading resource: res://Main.tscn.
   At: core/io/resource_loader.cpp:278.
ERROR: start: Failed loading scene: res://Main.tscn
   At: main/main.cpp:1936.

errors in editor, expanded