Fix GDScriptCache::get_full_script eating parsing errors because of early exit

[mattbork]

Historically and up through Godot 4.0-beta4, parse errors in a malformed GDScript will be printed to output and reported to the debugger at runtime to interrupt execution. In 4.0-beta5 and later, parse errors for most scripts are no longer reported, and the debugger does not stop execution. (I say most because scripts used as named classes or preloads, e.g., still fail to resolve or load as a resource). Up through 4.1.1-stable, there is no indication at all in the editor that a script has failed to be run because of a parsing error, which is potentially quite confusing. In 4.1.2-stable, the addition of #78540 at least prints an error when ResourceFormatLoaderGDScript::load fails, which a user can see in the debug panel. However, it is easily missed as execution is still not interrupted, and it does not show the actual parsing error anyway.

The root cause is a change in the responsiblity and logic of GDScriptCache::get_shallow_script introduced in #68374. Prior to that, get_shallow_script only loaded and cached the source code of a script. GDScriptCache::get_full_script would exit early if get_shallow_script failed because there would have been no script to call GDScript::reload on (which does the actual parsing and error reporting). Since the changes in the above PR, however, get_shallow_script now also parses the source code and passes along any errors. But get_full_script still exits early if get_shallow_script fails for any reason. This means that a script that loads sucessfully but hits an error during parsing will not make it to the reload call. Thus, the errors are never printed and never reported to the debugger.

The simplest fix is to exit from get_full_script early only when get_shallow_script returns a null Ref, indicating that the script simply failed to load. Actual parsing errors are ignored at that point so that reload will always be called on a script that loads. Then parsing errors will not be silently lost. Note that get_shallow_script will not have called GDScriptCompiler::make_scripts in the case of a parsing error, but this is fine because get_shallow_script is only called here for scripts that are not in the full_gdscript_cache and therefore were never compiled, and reload will exit after finding the same error during its own call of GDScriptParser::parse, well before compilation is attempted.

Other callers of get_shallow_script of course still retain the ability to error out if that initial parsing fails.

Fixes #75545.

[adamscott]

Hi @mattbork. Thanks for your interest in Godot and your first PR!

Unfortunately, I fail to see any difference between your fix and the master branch. Can you join a MRP where I can easily test the difference? I have trouble running the MRP from #75545.

[mattbork]

Sorry, my mistake. I should have included the MRP I was testing against. I was more thinking of #79041 when fixing this, but that was marked as a duplicate of #75545. Still, with the MRP in #75545, no syntax error for the un-indented a is printed on master but is printed with these changes. A simpler example is to run the following script from the command line with --script:

extends SceneTree

func _initialize()
        print("hello")

On master, the output is

ERROR: Failed to load script "res://cmd-mrp.gd" with error "Parse error".
   at: ResourceFormatLoaderGDScript::load (modules\gdscript\gdscript.cpp:2663)

Before #78540, there would have been no error reported at all. With the change here, the output now properly includes the actual parse error:

SCRIPT ERROR: Parse Error: Expected ":" after function declaration.
          at: GDScript::reload (res://cmd-mrp.gd:3)
ERROR: Failed to load script "res://cmd-mrp.gd" with error "Parse error".
   at: ResourceFormatLoaderGDScript::load (modules\gdscript\gdscript.cpp:2663)

As for in the editor, here's a project with a single node with a script attached.

mrp.zip

The script has a simple syntax error:

extends Node

func _ready()
	print("hello")

On master, the editor notes the syntax error and the separate error from ResourceFormatLoaderGDScript::load is printed to the debugger's Error panel but the scene still runs:

With these changes, the scene is interrupted by the debugger on line 3 and the actual syntax error is shown in the Stack Trace panel:

[adamscott]

I tested the new MRP and it works as expected! Thanks for your contribution, it should be merged for the 4.2 release, as well as cherry-picked for other 4.0 and 4.1 releases.

Thanks! And don't hesitate @mattbork to join the developer chat. We're always welcoming new contributors.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉

[YuriSizov]

Cherry-picked for 4.1.3.