godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
89.83k stars 20.96k forks source link

`load()` returns `GDScript` even with `Parse error`, but `new()` method can't be called and error is undetectable #96065

Open luckyabsoluter opened 1 month ago

luckyabsoluter commented 1 month ago

Tested versions

v4.2.1.stable.official [b09f793f5] v4.3.stable.official [77dcf97d8]

System information

Godot v4.2.1.stable

Issue description

When loading a .gd script using the load() function in Godot, the function returns a GDScript object even if there are parsing errors in the script. This is unexpected because:

This issue is problematic because there's no clear way to detect or handle this scenario programmatically. Even with parse errors, the script loads and appears valid according to the checks. The only way to detect that the script has issues is by using loaded.reload(), which returns a non-zero value if errors are present. However, relying on this method for error detection is not intuitive or well-documented.

Expected behavior:

Steps to reproduce

Here’s an example of the code and its output in both normal and error scenarios:

Code:

# res://main.gd
var loaded = load("res://error.gd")
print("loaded: ", loaded)
print("loaded == null: ", loaded == null)
print("loaded is GDScript: ", loaded is GDScript)
print("loaded.has_method('new'): ", loaded.has_method("new"))
print("loaded.new: ", loaded.new)
print("loaded.has_source_code(): ", loaded.has_source_code())
print("loaded.reload(): ", loaded.reload())
loaded.new()
# res://error.gd
error

Output with a valid script:

loaded: <GDScript#-9223371840326459786>
loaded == null: false
loaded is GDScript: true
loaded.has_method('new'): true
loaded.new: GDScript::new
loaded.has_source_code(): true
loaded.reload(): 0

Output with a script containing parse errors:

loaded: <GDScript#-9223371833011593533>
loaded == null: false
loaded is GDScript: true
loaded.has_method('new'): true
loaded.new: GDScript::new
loaded.has_source_code(): true
loaded.reload(): 43

Error encountered:

Invalid call. Nonexistent function 'new' in base 'GDScript'.

Steps to reproduce:

  1. Use the code provided above to load a script and check its validity.
  2. Create a .gd script with syntax errors and attempt to load it.
  3. Observe that loaded.has_method("new") returns true, but calling loaded.new() raises an error.
  4. Note that loaded.reload() returns a non-zero value, indicating an error that isn't detectable by other checks.

Minimal reproduction project (MRP)

gdscript load issue project.zip

AThousandShips commented 1 month ago

Please test in 4.2.2, or 4.3, 4.2.1 is no longer supported and this might have been fixed.

AThousandShips commented 1 month ago

Sounds related to:

Which was fixed in 4.3 by:

luckyabsoluter commented 1 month ago

Reproduced on v4.3.stable.official [77dcf97d8]

dalexeev commented 1 month ago
luckyabsoluter commented 1 month ago

I believe it would be more intuitive if, instead of the current error message stating that the new() method doesn't exist, the error could explicitly indicate that the script cannot be instantiated due to errors in the script. This would make it clearer that the issue lies with the script's validity rather than suggesting that the new() method is missing from a seemingly valid GDScript object.

Such an error message would better guide users toward understanding the root cause of the issue and encourage the use of can_instantiate() to validate scripts before attempting to instantiate them.

Thank you for your input and the reference to the can_instantiate() method.

This response was generated with the assistance of GPT.