godot: Debugger stops when loading scripts with invalid syntax

@YuriSizov you can write addons where users specify custom scripts. These scripts can be broken, though. In case they are, the plugin should not crash etc. but handle that gracefully. For this, e.g. I have written tests in my addon and the setup worked fine until 4.2 - I found a workaround though, which is to move the broken scripts outside the folder scanning of the unit test execution scope. While this is fine, other addon devs may stumble upon the same issue.

Having a mechanism to at least toggle off the ‘error throwing’ of load() somehow would improve this.

Well in beta 3 it does sound very likely to be https://github.com/godotengine/godot/pull/83540 as @dalexeev pointed out earlier. This was presumably cherry-picked in 4.1.3, but apparently doesn’t introduce the intended behavior, maybe it relied on earlier changes which weren’t cherry-picked.

Discussing of this issue with the GDScript team in our weekly meeting.

A bisection of the issue would be appreciated, to know if the change that introduced this issue was intentional or not.

The load is now checking in addition for script errors, this is a major regression!

This is not a regression, this is a fix for incorrect behavior. See #78540 and #83540.

Please roll this change back or provide a flag to enable/disable script syntax checking on load.

• See #64065.

@adamscott I think it actually might be worth to have a dedicated ‘future’ branch building against specific godot nightly builds to verify regressions dynamically in addon repos. That is obviously more effort but worth it in the long run. I will discuss this privately with others. Offtopic over.

There a no issues when using v4.1.3.stable.official [f06b6836a] the load works without the debugger stoped. This is behavior change in Godot 4.2.

As much I don’t want to add salt to the wound, I would highly encourage plugin developers to test their plugins against upcoming beta releases (or at minimum against release candidate releases) in order to prevent such situations.

Nobody benefits from incompatibilities like this one.

Well, the problem here is that the debug mode is specifically to launch your scripts with a debugger attached. So with the bugs fixed in 4.2 this is now operating as expected and as designed.

What you really need is some form of https://github.com/godotengine/godot-proposals/issues/8548 with a way to retrieve errors after the fact. Something that https://github.com/godotengine/godot/pull/85544 is trying to accomplish. You made it work with the debug mode and the old behavior, but your workflow is based on something that wasn’t working properly, so now that it is fixed, it doesn’t work anymore.

It’s unfortunate, but it’s always risky to commit to an implementation based on something unreliable. I would suggest opening a proposal for a proper solution in this situation. Not sure much can be done as a workaround for now without compromising on error reporting where it’s supposed to work, i.e. under most other circumstances.

I’m not entirely sure why you would load syntactically broken scripts. Wouldn’t just providing an empty GDScript.new() work for mocking?

But as a possible workaround, you could load the source code via the FileAccess API and then create the script object from that:

var script := GDScript.new()
script.source_code = code_from_file
var err := script.reload()

Godot_v4.2-stable_win64.exe -d res://TestScene.tscn

D:\develop\workspace\issues\load_issue>Godot Engine v4.2.stable.official.46dc27791 - https://godotengine.org
Vulkan API 1.3.242 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 980


Debugger Break, Reason: 'Parser Error: Expected indented block after function declaration.'
*Frame 0 - res://invalidScript.gd:10 in function ''
Enter "help" for assistance.
debug>

This breaks now the workflow, on Godot 4.2 the program is on hold via debugger break. As @bitbrain already mentioned, if you’re running a test in a CI workflow, this will now break it and the WF will run out of time. Before the script was loading without a debugger break.

Just to be clear, the -d option that you’re using stands for:

  -d, --debug                       Debug (local stdout debugger).

So this seems to me like it behaves as intended. There’s an error in the project, you asked for a stdout debugger, and you get that.

It seems to me that you don’t actually want a stdout debugger to interrupt the CI flow, so you maybe you shouldn’t use -d?

I tested and without -d in 4.2 there’s no interruption.

$ Godot_v4.2-stable_linux.x86_64 res://TestScene.tscn
Godot Engine v4.2.stable.official.46dc27791 - https://godotengine.org
Vulkan API 1.3.246 - Forward+ - Using Vulkan Device #1: AMD - AMD Radeon RX Vega M GL Graphics (RADV VEGAM)
 
SCRIPT ERROR: Parse Error: Expected indented block after function declaration.
          at: GDScript::reload (res://invalidScript.gd:10)
ERROR: Failed to load script "res://invalidScript.gd" with error "Parse error".
   at: load (modules/gdscript/gdscript.cpp:2775)
script: <GDScript#-9223372010195778363> true

I don’t quite understand why you would need to load broken scripts on CI. Unit tests for your project should be testing for issues in your project’s logic and behavior. Broken scripts are not that, they won’t work because Godot itself won’t be able to do anything with them. They will always trigger the debugger at runtime during test runs.

What exactly is it that you’re trying to test here?

To bring more light into the issue here, an example project to show the problem. load_issue.zip

Godot_v4.1.3-stable_win64.exe -d res://TestScene.tscn

D:\develop\workspace\issues\load_issue>Godot Engine v4.1.3.stable.official.f06b6836a - https://godotengine.org
Vulkan API 1.3.242 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 980

ERROR: Failed to load script "res://invalidScript.gd" with error "Parse error".
   at: load (modules/gdscript/gdscript.cpp:2705)
script: <GDScript#-9223372010078337895> true

vs

Godot_v4.2-stable_win64.exe -d res://TestScene.tscn

D:\develop\workspace\issues\load_issue>Godot Engine v4.2.stable.official.46dc27791 - https://godotengine.org
Vulkan API 1.3.242 - Forward+ - Using Vulkan Device #0: NVIDIA - NVIDIA GeForce GTX 980


Debugger Break, Reason: 'Parser Error: Expected indented block after function declaration.'
*Frame 0 - res://invalidScript.gd:10 in function ''
Enter "help" for assistance.
debug>


Debugger Break, Reason: 'Parser Error: Expected indented block after function declaration.'
*Frame 0 - res://invalidScript.gd:10 in function ''
Enter "help" for assistance.
debug>

This breaks now the workflow, on Godot 4.2 the program is on hold via debugger break. As @bitbrain already mentioned, if you’re running a test in a CI workflow, this will now break it and the WF will run out of time. Before the script was loading without a debugger break.

Note that this is in 4.1 as well, if those PRs are indeed the source of those changes (this also shows the importance of declaring what versions you are talking about when suggesting something is a regression, since unless this is caused by some other PR this behavior is in 4.1.3)

We addon developers rely on the old behaviour, as we want to e.g. test via unit tests that loading a broken script does not break anything. However, currently, this just fails the CI build (this did not happen in 4.1 and prior). If it is not possible any longer to revert back to the old behaviour due to architectural reasons, a suggested workaround would be greatly appreciated.