GDScript: Enhance handling of cyclic dependencies

[RandomShaper]

Since I wasn't very successful in getting my ideas across in #92326, due to the complex nature of the matter, I decided to get my own hands dirty with this to illustrate what I meant. It turned out that the resulting code is much more complicated than what I was trying to convey in my comments. Therefore, I'm submitting my complete idea in a new PR.

Fixes #70985.
Fixes #90362.
Fixes #90954.
Fixes #92780 (according to #92780 (comment)).

Disclaimers:

• I haven't tested it with binary resources.
• The risk of regressions is obvious.

For reviewers, disable whitespace diffing because the changes are not a big as they seem.

[adamscott]

I thought that your PR could solve what I stumbled upon, that I hoped to solve with my previous PR. Could you try to check if it could solve it?

I got this crash when I tried to load this peculiar MRP (cyclic-scenes.zip):

Crash log

❯ godot --editor
Godot Engine v4.3.beta.custom_build.18499487d (2024-06-19 12:48:01 UTC) - https://godotengine.org
Vulkan 1.2.280 - Forward+ - Using Device #0: Apple - Apple M2

2024-06-19 09:30:23.673 godot[5269:63129] WARNING: AVCaptureDeviceTypeExternal is deprecated for Continuity Cameras. Please use AVCaptureDeviceTypeContinuityCamera and add NSCameraUseContinuityCameraDeviceType to your Info.plist.
ERROR: Parse Error: Busy. [Resource file res://b.tscn:8]
   at: _parse_node_tag (scene/resources/resource_format_text.cpp:294)
ERROR: Failed loading resource: res://b.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (core/io/resource_loader.cpp:282)
ERROR: Parse Error: Busy. [Resource file res://a.tscn:8]
   at: _parse_node_tag (scene/resources/resource_format_text.cpp:294)
ERROR: Failed loading resource: res://a.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (core/io/resource_loader.cpp:282)
ERROR: Attempting to parent and popup a dialog that already has a parent.
   at: _try_parent_dialog (scene/main/window.cpp:1899)
ERROR: Parse Error: Busy. [Resource file res://b.tscn:8]
   at: _parse_node_tag (scene/resources/resource_format_text.cpp:294)
ERROR: Failed loading resource: res://b.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (core/io/resource_loader.cpp:282)
ERROR: Parse Error: Busy. [Resource file res://a.tscn:8]
   at: _parse_node_tag (scene/resources/resource_format_text.cpp:294)
ERROR: Failed loading resource: res://a.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (core/io/resource_loader.cpp:282)
ERROR: Parse Error: Busy. [Resource file res://b.tscn:8]
   at: _parse_node_tag (scene/resources/resource_format_text.cpp:294)
ERROR: Failed loading resource: res://b.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (core/io/resource_loader.cpp:282)

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.3.beta.custom_build (18499487dbd22d53c16ec841bf056365988d47bf)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] 1   libsystem_platform.dylib            0x000000018d76f584 _sigtramp + 56
[2] 2   libc++abi.dylib                     0x000000018d6f67b0 __dynamic_cast + 56
[3] _property_translation_fr_compressed (in godot.macos.editor.dev.arm64) + 39489
[4] _property_translation_fr_compressed (in godot.macos.editor.dev.arm64) + 31877
[5] _property_translation_fr_compressed (in godot.macos.editor.dev.arm64) + 52389
[6] _property_translation_id_compressed (in godot.macos.editor.dev.arm64) + 11372
[7] HashMap<String, ResourceLoader::LoadToken*, HashMapHasherDefault, HashMapComparatorDefault<String>, DefaultTypedAllocator<HashMapElement<String, ResourceLoader::LoadToken*>>>::erase(String const&)
[8] HashMap<String, ResourceLoader::ThreadLoadTask, HashMapHasherDefault, HashMapComparatorDefault<String>, DefaultTypedAllocator<HashMapElement<String, ResourceLoader::ThreadLoadTask>>>::Iterator::operator->() const
[9] HashMap<String, ResourceLoader::LoadToken*, HashMapHasherDefault, HashMapComparatorDefault<String>, DefaultTypedAllocator<HashMapElement<String, ResourceLoader::LoadToken*>>>::operator[](String const&)
[10] Ref<ResourceLoader::LoadToken>::operator->() const
[11] VectorWriteProxy<RenderingServer::SurfaceData>::operator[](long long) (in godot.macos.editor.dev.arm64) (vector.h:52)
[12] RendererCanvasRenderRD::RendererCanvasRenderRD() (in godot.macos.editor.dev.arm64) (renderer_canvas_render_rd.cpp:2772)
[13] RID_Alloc<RendererDummy::MeshStorage::DummyMultiMesh, false>::~RID_Alloc() (in godot.macos.editor.dev.arm64) (rid_owner.h:346)
[14] RendererDummy::MeshStorage::multimesh_instance_get_color(RID, int) const (in godot.macos.editor.dev.arm64) (mesh_storage.h:170)
[15] fsr2Dispatch(FfxFsr2Context_Private*, FfxFsr2DispatchDescription const*) (in godot.macos.editor.dev.arm64) (ffx_fsr2.cpp:803)
[16] fsr2Dispatch(FfxFsr2Context_Private*, FfxFsr2DispatchDescription const*) (in godot.macos.editor.dev.arm64) (ffx_fsr2.cpp:788)
[17] ffxFsr2ContextDispatch (in godot.macos.editor.dev.arm64) (ffx_fsr2.cpp:1134)
[18] SearchArray<Variant, CallableComparator>::bisect(Variant const*, long long, Variant const&, bool) const
[19] HashMap<StringName, Object::SignalData, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, Object::SignalData>>>::erase(StringName const&)
[20] MethodBindTRC<StringName, StringName const&, StringName const&>::call(Object*, Variant const**, int, Callable::CallError&) const (in godot.macos.editor.dev.arm64) (method_bind.h:612)
[21] CowData<SceneState::ConnectionData>::_get_alloc_size(unsigned long long) const (in godot.macos.editor.dev.arm64) (cowdata.h:135)
[22] void call_get_argument_type_info<RID, unsigned int, unsigned int, Vector<unsigned char> const&>(int, PropertyInfo&) (in godot.macos.editor.dev.arm64) (binder_common.h:720)
[23] void call_with_validated_variant_args_helper<__UnexistingClass, long long, Color const&, 0ul, 1ul>(__UnexistingClass*, void (__UnexistingClass::*)(long long, Color const&), Variant const**, IndexSequence<0ul, 1ul>) (in godot.macos.editor.dev.arm64) (binder_common.h:364)
[24] MethodBind* ClassDB::bind_compatibility_method<MethodDefinition, String (Object::*)(StringName const&, StringName const&, int, StringName const&) const, char const*>(MethodDefinition, String (Object::*)(StringName const&, StringName const&, int, StringName const&) const, char const*)
[25] ProjectSettings::ProjectSettings() (in godot.macos.editor.dev.arm64) (project_settings.cpp:1420)
[26] HashMap<StringName, ProjectSettings::AutoloadInfo, HashMapHasherDefault, HashMapComparatorDefault<StringName>, DefaultTypedAllocator<HashMapElement<StringName, ProjectSettings::AutoloadInfo>>>::erase(StringName const&) (in godot.macos.editor.dev.arm64) (hash_map.h:353)
[27] ProjectSettings::get_argument_options(StringName const&, int, List<String, DefaultAllocator>*) const (in godot.macos.editor.dev.arm64) (project_settings.cpp:1356)
[28] Gizmo3DHelper::initialize_handle_action(Variant const&, Transform3D const&) (in godot.macos.editor.dev.arm64) (gizmo_3d_helper.cpp:39)
[29] VisualShaderEditor::VisualShaderEditor() (in godot.macos.editor.dev.arm64) (visual_shader_editor_plugin.cpp:6645)
[30] HashMap<int, VisualShaderGraphPlugin::Port, HashMapHasherDefault, HashMapComparatorDefault<int>, DefaultTypedAllocator<HashMapElement<int, VisualShaderGraphPlugin::Port>>>::_insert(int const&, VisualShaderGraphPlugin::Port const&, bool) (in godot.macos.editor.dev.arm64) (hash_map.h:214)
[31] 31  dyld                                0x000000018d3b60e0 start + 2360
-- END OF BACKTRACE --
================================================================
fish: Job 1, 'godot --editor' terminated by signal SIGABRT (Abort)

In that MRP, I have a.tscn that has an exported PackedScene to b.tscn, and vice-versa. I'll try to see if I can load partially a scene as we do for GDScript.

[RandomShaper]

@adamscott, is that something we even want to support? I guess we can modify the loading machinery so that's possible, but, then, the two PackedScenes would be referencing each other, making them impossible to be freed from memory.

How was that scene setup made in the first place? Can one create that with normal usage of the editor?

We're getting maybe a bit off-topic, though...

[akien-mga]

Tested on a few projects (GDQuest TPS demo, W4 Planet Crashers), I confirm that it fixes the linked issues, and I didn't spot any obvious regression. That confirms #90362 fixed.

I also confirm that the MRP in #92780 seems fixed by this PR.

---

For #70985, I tested the MRP and it seems to solve the main issue (couldn't load the script at all), but the first load of the project still produces these errors:

ERROR: Parse Error: Busy. [Resource file res://src/swapPad/cell.tscn:9]
   at: _parse_node_tag (./scene/resources/resource_format_text.cpp:289)
ERROR: Failed loading resource: res://src/swapPad/cell.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (./core/io/resource_loader.cpp:282)

This sounds related to #92303 so I tested with this PR on top of #93346, but that doesn't solve it either.

---

For #90954, with a dev build I'm actually triggering the DEV_ASSERT added in this PR:

ERROR: Parse Error: Busy. [Resource file res://example_menu.tscn:12]
   at: _parse_node_tag (./scene/resources/resource_format_text.cpp:289)
ERROR: Failed loading resource: res://example_menu.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (./core/io/resource_loader.cpp:282)
ERROR: Parse Error: Busy. [Resource file res://example_menu.tscn:12]
   at: _parse_node_tag (./scene/resources/resource_format_text.cpp:289)
ERROR: Failed loading resource: res://example_menu.tscn. Make sure resources have been imported by opening the project in the editor at least once.
   at: _load (./core/io/resource_loader.cpp:282)
ERROR: FATAL: DEV_ASSERT failed  "!res_ref_overrides.has(load_nesting - 1) || !res_ref_overrides[load_nesting - 1].has(local_path)" is false.
   at: set_resource_ref_override_for_outer_load (./core/io/resource_loader.cpp:737)

Thread 1 "godot-git" received signal SIGILL, Illegal instruction.
0x000000000a72096e in ResourceLoader::set_resource_ref_override_for_outer_load (p_path=..., p_res_override=...) at ./core/io/resource_loader.cpp:737
737             DEV_ASSERT(!res_ref_overrides.has(load_nesting - 1) || !res_ref_overrides[load_nesting - 1].has(local_path));

(gdb) bt
#0  0x000000000a72096e in ResourceLoader::set_resource_ref_override_for_outer_load (p_path=..., p_res_override=...) at ./core/io/resource_loader.cpp:737
#1  0x00000000066985a9 in GDScriptAnalyzer::reduce_preload (this=0x7fffffff97b0, p_preload=0x259ca250) at ./modules/gdscript/gdscript_analyzer.cpp:4318
#2  0x000000000668c4a0 in GDScriptAnalyzer::reduce_expression (this=0x7fffffff97b0, p_expression=0x259ca250, p_is_root=false) at ./modules/gdscript/gdscript_analyzer.cpp:2488
#3  0x00000000066933c1 in GDScriptAnalyzer::reduce_dictionary (this=0x7fffffff97b0, p_dictionary=0x259c9f50) at ./modules/gdscript/gdscript_analyzer.cpp:3536
#4  0x000000000668c405 in GDScriptAnalyzer::reduce_expression (this=0x7fffffff97b0, p_expression=0x259c9f50, p_is_root=false) at ./modules/gdscript/gdscript_analyzer.cpp:2473
#5  0x0000000006688964 in GDScriptAnalyzer::resolve_assignable (this=0x7fffffff97b0, p_assignable=0x259c98c0, p_kind=0x3f52cf3 "variable") at ./modules/gdscript/gdscript_analyzer.cpp:1891
#6  0x0000000006689644 in GDScriptAnalyzer::resolve_variable (this=0x7fffffff97b0, p_variable=0x259c98c0, p_is_local=false) at ./modules/gdscript/gdscript_analyzer.cpp:2005
#7  0x00000000066832e7 in GDScriptAnalyzer::resolve_class_member (this=0x7fffffff97b0, p_class=0x1adcdc30, p_index=0, p_source=0x259c98c0) at ./modules/gdscript/gdscript_analyzer.cpp:958
#8  0x0000000006684b7b in GDScriptAnalyzer::resolve_class_interface (this=0x7fffffff97b0, p_class=0x1adcdc30, p_source=0x1adcdc30) at ./modules/gdscript/gdscript_analyzer.cpp:1219
#9  0x0000000006684d68 in GDScriptAnalyzer::resolve_class_interface (this=0x7fffffff97b0, p_class=0x1adcdc30, p_recursive=true) at ./modules/gdscript/gdscript_analyzer.cpp:1247
#10 0x00000000066a041d in GDScriptAnalyzer::resolve_interface (this=0x7fffffff97b0) at ./modules/gdscript/gdscript_analyzer.cpp:5731
#11 0x00000000066a05bb in GDScriptAnalyzer::analyze (this=0x7fffffff97b0) at ./modules/gdscript/gdscript_analyzer.cpp:5765
#12 0x000000000666f3ab in GDScript::reload (this=0x259c4c80, p_keep_state=true) at ./modules/gdscript/gdscript.cpp:816
#13 0x00000000066aa9a6 in GDScriptCache::get_full_script (p_path=..., r_error=@0x7fffffffa2ec: OK, p_owner=..., p_update_from_disk=false) at ./modules/gdscript/gdscript_cache.cpp:347
#14 0x00000000066aac50 in GDScriptCache::finish_compiling (p_owner=...) at ./modules/gdscript/gdscript_cache.cpp:386
#15 0x00000000066bddd5 in GDScriptCompiler::compile (this=0x7fffffffa5f0, p_parser=0x7fffffffa6b0, p_script=0x1063efb0, p_keep_state=true) at ./modules/gdscript/gdscript_compiler.cpp:3231
#16 0x000000000666f633 in GDScript::reload (this=0x1063efb0, p_keep_state=true) at ./modules/gdscript/gdscript.cpp:835
#17 0x00000000066aa9a6 in GDScriptCache::get_full_script (p_path=..., r_error=@0x7fffffffb1bc: OK, p_owner=..., p_update_from_disk=false) at ./modules/gdscript/gdscript_cache.cpp:347
#18 0x00000000066aac50 in GDScriptCache::finish_compiling (p_owner=...) at ./modules/gdscript/gdscript_cache.cpp:386
#19 0x00000000066bddd5 in GDScriptCompiler::compile (this=0x7fffffffb4c0, p_parser=0x7fffffffb580, p_script=0x259b6ec0, p_keep_state=true) at ./modules/gdscript/gdscript_compiler.cpp:3231
#20 0x000000000666f633 in GDScript::reload (this=0x259b6ec0, p_keep_state=true) at ./modules/gdscript/gdscript.cpp:835
#21 0x00000000066aa9a6 in GDScriptCache::get_full_script (p_path=..., r_error=@0x7fffffffc0b4: OK, p_owner=..., p_update_from_disk=false) at ./modules/gdscript/gdscript_cache.cpp:347
#22 0x000000000667c13a in ResourceFormatLoaderGDScript::load (this=0xba6af40, p_path=..., p_original_path=..., r_error=0x259b7fc0, p_use_sub_threads=false, r_progress=0x259b7fb0, 
    p_cache_mode=ResourceFormatLoader::CACHE_MODE_REUSE) at ./modules/gdscript/gdscript.cpp:2947
#23 0x000000000a71e5c7 in ResourceLoader::_load (p_path=..., p_original_path=..., p_type_hint=..., p_cache_mode=ResourceFormatLoader::CACHE_MODE_REUSE, r_error=0x259b7fc0, p_use_sub_threads=false, 
    r_progress=0x259b7fb0) at ./core/io/resource_loader.cpp:268
#24 0x000000000a71ed2c in ResourceLoader::_thread_load_function (p_userdata=0x259b7f68) at ./core/io/resource_loader.cpp:332
#25 0x000000000a71fc0b in ResourceLoader::_load_start (p_path=..., p_type_hint=..., p_thread_mode=ResourceLoader::LOAD_THREAD_FROM_CURRENT, p_cache_mode=ResourceFormatLoader::CACHE_MODE_REUSE)
    at ./core/io/resource_loader.cpp:533
#26 0x000000000a71f6b9 in ResourceLoader::load (p_path=..., p_type_hint=..., p_cache_mode=ResourceFormatLoader::CACHE_MODE_REUSE, r_error=0x0) at ./core/io/resource_loader.cpp:452
#27 0x0000000007735b87 in EditorFileSystem::_update_script_classes (this=0xd8f8a50) at ./editor/editor_file_system.cpp:1646
#28 0x0000000007735d94 in EditorFileSystem::_update_pending_script_classes (this=0xd8f8a50) at ./editor/editor_file_system.cpp:1676
#29 0x000000000773e84f in EditorFileSystem::reimport_files (this=0xd8f8a50, p_files=...) at ./editor/editor_file_system.cpp:2545
#30 0x0000000007730461 in EditorFileSystem::_update_scan_actions (this=0xd8f8a50) at ./editor/editor_file_system.cpp:701
#31 0x0000000007733e06 in EditorFileSystem::_notification (this=0xd8f8a50, p_what=17) at ./editor/editor_file_system.cpp:1304
#32 0x00000000077d09ea in EditorFileSystem::_notificationv (this=0xd8f8a50, p_notification=17, p_reversed=false) at ./editor/editor_file_system.h:140
#33 0x000000000ab9c44a in Object::notification (this=0xd8f8a50, p_notification=17, p_reversed=false) at ./core/object/object.cpp:870
#34 0x00000000086b46c8 in SceneTree::_process_group (this=0xd822bf0, p_group=0xd822e48, p_physics=false) at ./scene/main/scene_tree.cpp:962
#35 0x00000000086b4c41 in SceneTree::_process (this=0xd822bf0, p_physics=false) at ./scene/main/scene_tree.cpp:1039
#36 0x00000000086b2c63 in SceneTree::process (this=0xd822bf0, p_time=0.11886100000000033) at ./scene/main/scene_tree.cpp:526
#37 0x0000000005be29bb in Main::iteration () at main/main.cpp:4099
#38 0x0000000005b2a5a4 in OS_LinuxBSD::run (this=0x7fffffffd0a0) at platform/linuxbsd/os_linuxbsd.cpp:962
#39 0x0000000005b23331 in main (argc=2, argv=0x7fffffffd6f8) at platform/linuxbsd/godot_linuxbsd.cpp:85

[RandomShaper]

Pushed a fix for #90954. There's still an issue there, but I think unrelated to the scope of this PR: in my testing, the global classname cache isn't updated at once; I have first to remove the use of some classes from a script that refers the other. Once the editor has been able to parse the scripts thanks to that manual aid, the project loads and works. Thoughts?

[akien-mga]

Pushed a fix for #90954. There's still an issue there, but I think unrelated to the scope of this PR: in my testing, the global classname cache isn't updated at once; I have first to remove the use of some classes from a script that refers the other. Once the editor has been able to parse the scripts thanks to that manual aid, the project loads and works. Thoughts?

This sounds like what #92303 aims at solving. This morning I've actually been testing both PRs together for best results (but also separately to make sure I'm not attributing a problem to a PR that's caused by the other).

[akien-mga]

Tested again, I confirm this now fixes #90954 and the dev assert is solved.

[akien-mga]

Thanks! Awesome work to everyone involved here and on #92326 🎉