Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
luckyabsoluter opened this issue Aug 25, 2024 · 5 comments

Comments

@luckyabsoluter
Copy link

luckyabsoluter commented Aug 25, 2024

Tested versions

v4.2.1.stable.official [b09f793]
v4.3.stable.official [77dcf97]

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:

  • A valid GDScript object should always have a new() method.
  • Despite loaded.has_method("new") returning true, calling loaded.new() results in the error:
Invalid call. Nonexistent function 'new' in base 'GDScript'.

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:

  • If there are parsing errors, load() should return null or provide a clear and intuitive way to detect errors.
  • A valid GDScript should always have a new() method that can be called without errors.

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

@luckyabsoluter luckyabsoluter changed the title load() returns GDScript even with Parse error, but new() method is missing and error is undetectable load() returns GDScript even with Parse error, but new() method can't be called and error is undetectable Aug 25, 2024
@AThousandShips
Copy link
Member

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

@AThousandShips
Copy link
Member

@luckyabsoluter
Copy link
Author

luckyabsoluter commented Aug 26, 2024

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

@dalexeev
Copy link
Member

@luckyabsoluter
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants