Regression due to GDScript cache changes for preload in Godot 4.3 dev 4+

[mhilbrunner]

Tested versions

• Reproducible in 4.3 dev 4 onwards, starting from ef5dccc
  (Stop caching packed scenes in GDScript cache (on preload) #85501)

System information

Windows 11

Issue description

It looks like the change in #85501, included in Godot 4.3 dev 4 may have some side effects.
With this change included, starting W4 Game's Planet Crashers demo fails with a compilation error in weapon_drop.gd:

SCRIPT ERROR: Parse Error: Could not preload resource file "res://items/weapon_drop.tscn".
          at: GDScript::reload (res://characters/character.gd:420)
SCRIPT ERROR: Compile Error:
          at: GDScript::reload (res://items/weapon_drop.gd:-1)
ERROR: Failed to load script "res://items/weapon_drop.gd" with error "Parse error".
   at: load (modules\gdscript\gdscript.cpp:2769)

Note that the source line for the error is -1, and that this script is attached to a scene that gets preload()ed here.
Changing that preload() to load() fixes the error. Even when moved to the top of the file, the scene can't be preload()ed: const drop_scene := preload("res://items/weapon_drop.tscn") at the top fails with the same error.

gdscript.cpp:2769 seems to point to the script attached to the preload()ed scene not being found in the GDScript cache. Reverting the above PR locally seems to fix this. Also see #85081.

Steps to reproduce

Load the above mentioned project with the linked change included. The error is output to console.
Starting the project from the editor results in the same error.
Note that later commits of the Planet Crasher's demo work around this error, so use 3f07e50ed09 to test.

Minimal reproduction project (MRP)

MRP by Jordyfel

Not really minimal:
Planet Crashers repository
Direct link to ZIP file

Note that this is a multiplayer project and may open network connections to W4 Games servers.

[Jordyfel]

There is a cyclic dependency of GDScriptCache::get_full_script() happening between the preloading script and the script of the preloaded scene.

A breakpoint on the error path in reload() shows:

godot.windows.editor.dev.x86_64.exe!GDScript::reload(bool p_keep_state) Line 779 (c:\dev\Godot4\godot\modules\gdscript\gdscript.cpp:779)
godot.windows.editor.dev.x86_64.exe!GDScriptCache::get_full_script(const String & p_path, Error & r_error, const String & p_owner, bool p_update_from_disk) Line 325 (c:\dev\Godot4\godot\modules\gdscript\gdscript_cache.cpp:325)
godot.windows.editor.dev.x86_64.exe!GDScriptCache::finish_compiling(const String & p_owner) Line 364 (c:\dev\Godot4\godot\modules\gdscript\gdscript_cache.cpp:364)
godot.windows.editor.dev.x86_64.exe!GDScriptCompiler::compile(const GDScriptParser * p_parser, GDScript * p_script, bool p_keep_state) Line 3213 (c:\dev\Godot4\godot\modules\gdscript\gdscript_compiler.cpp:3213)
godot.windows.editor.dev.x86_64.exe!GDScript::reload(bool p_keep_state) Line 795 (c:\dev\Godot4\godot\modules\gdscript\gdscript.cpp:795)
godot.windows.editor.dev.x86_64.exe!GDScriptCache::get_full_script(const String & p_path, Error & r_error, const String & p_owner, bool p_update_from_disk) Line 325 (c:\dev\Godot4\godot\modules\gdscript\gdscript_cache.cpp:325)
godot.windows.editor.dev.x86_64.exe!ResourceFormatLoaderGDScript::load(const String & p_path, const String & p_original_path, Error * r_error, bool p_use_sub_threads, float * r_progress, ResourceFormatLoader::CacheMode p_cache_mode) Line 2855 (c:\dev\Godot4\godot\modules\gdscript\gdscript.cpp:2855)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_load(const String & p_path, const String & p_original_path, const String & p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error * r_error, bool p_use_sub_threads, float * r_progress) Line 264 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:264)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_thread_load_function(void * p_userdata) Line 322 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:322)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_load_start(const String & p_path, const String & p_type_hint, ResourceLoader::LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode) Line 525 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:525)
godot.windows.editor.dev.x86_64.exe!ResourceLoaderText::load() Line 474 (c:\dev\Godot4\godot\scene\resources\resource_format_text.cpp:474)
godot.windows.editor.dev.x86_64.exe!ResourceFormatLoaderText::load(const String & p_path, const String & p_original_path, Error * r_error, bool p_use_sub_threads, float * r_progress, ResourceFormatLoader::CacheMode p_cache_mode) Line 1681 (c:\dev\Godot4\godot\scene\resources\resource_format_text.cpp:1681)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_load(const String & p_path, const String & p_original_path, const String & p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error * r_error, bool p_use_sub_threads, float * r_progress) Line 264 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:264)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_thread_load_function(void * p_userdata) Line 322 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:322)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::_load_start(const String & p_path, const String & p_type_hint, ResourceLoader::LoadThreadMode p_thread_mode, ResourceFormatLoader::CacheMode p_cache_mode) Line 525 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:525)
godot.windows.editor.dev.x86_64.exe!ResourceLoader::load(const String & p_path, const String & p_type_hint, ResourceFormatLoader::CacheMode p_cache_mode, Error * r_error) Line 442 (c:\dev\Godot4\godot\core\io\resource_loader.cpp:442)
godot.windows.editor.dev.x86_64.exe!GDScriptAnalyzer::reduce_preload(GDScriptParser::PreloadNode * p_preload) Line 4213 (c:\dev\Godot4\godot\modules\gdscript\gdscript_analyzer.cpp:4213)

It's actually not cyclic, 2 scripts need to preload the same scene for this to occur

[Jordyfel]

MRP:
test_proj.zip

The full set of conditions seem to be that a scene is loaded (the main scene in the mrp), whose script preloads another scene, which has a reference to another scene that preloads it, different from the first

[Jordyfel]

preload() can make the script compilation depend on loading a resource, and that resource could need to fully load a script. This breaks the assumption that a script can be loaded without calling "get_full_script" on another script, which breaks cyclic reference support.

[akien-mga]

I can reproduce the issue in latest master (78cce19) with the GDQuest TPS demo: https://github.com/gdquest-demos/godot-4-3d-third-person-controller

SCRIPT ERROR: Parse Error: Could not preload resource file "res://Player/Coin/Coin.tscn".
          at: GDScript::reload (res://Player/Player.gd:7)
SCRIPT ERROR: Parse Error: Cannot infer the type of "COIN_SCENE" constant because the value doesn't have a set type.
          at: GDScript::reload (res://Player/Player.gd:7)
SCRIPT ERROR: Parse Error: Cannot infer the type of "coin" variable because the value doesn't have a set type.
          at: GDScript::reload (res://Player/Player.gd:212)
SCRIPT ERROR: Compile Error: 
          at: GDScript::reload (res://Player/Coin/Coin.gd:-1)
ERROR: Failed to load script "res://Player/Coin/Coin.gd" with error "Parse error".
   at: load (modules/gdscript/gdscript.cpp:2824)

[vnen]

The GDScript analyzer should probably avoid the preload and leave loading to a later step. The problem is that then it does not know the type of the resource. It cannot tell the base class nor if it has any script attached, so it cannot infer the types.

I wonder how to solve this. If there was a way to get the type and script attached without having to actually load the resource, we could solve this case. I don't think there's any facility for that yet. It could be done only if all resource loaders had a function to return this information.

[vnen]

I realize even that won't be enough.

Imagine a scenario like this: scene_a -> script_a -> script_b -> scene_a.

When loading scene_a it will eventually ask to load scene_a again, because of the dependency graph. But since scene_a is already being loaded, it will give an error.

Even if we delay loading scene_a from script_b until the absolute end of the process, it will still be loaded before the first scene_a is loaded, so it'll give an error anyway. We could only solve this if we postpone the preload until after the first scene_a is loaded, but I can't think of any mechanism that could this. At least not without adding more stuff to the resource loading process.

This issue is actually expected behavior considering we don't support dependency cycles between resources. So the work around is using load instead of preload.