godotengine / godot

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

Scripts fail to load with a parse error on exported projects if it uses editor classes #91713

Open ydeltastar opened 1 month ago

ydeltastar commented 1 month ago

Tested versions

v4.3.dev.custom_build [2042420bd]

System information

Windows 11

Issue description

Tool scripts using an editor class such as EditorInterface will fail to load with a parse error only on exported projects.

@tool
extends Node3D

func _init() -> void:
    if Engine.is_editor_hint():
        var editor_filesystem = EditorInterface.get_resource_filesystem()
        if not editor_filesystem.resources_reimported.is_connected(_on_resource_reimport):
            editor_filesystem.resources_reimported.connect(_on_resource_reimport)

func _on_resource_reimport(resources:PackedStringArray):
    pass

Output of debug template console wrapper:

SCRIPT ERROR: Parse Error: Identifier "EditorInterface" not declared in the current scope.
          at: GDScript::reload (res://node_3d.gd:6)
ERROR: Failed to load script "res://node_3d.gd" with error "Parse error".
   at: ResourceFormatLoaderGDScript::load (modules\gdscript\gdscript.cpp:2894)

Well, it makes sense. Editor classes aren't included in distribution builds but there is no conditional compiling in GDScript so tool scripts with game and editor code can't be used at all in a project.

Workarounds:

Steps to reproduce

Export a project that uses the script above with a debug template and run the .console.exe version.

Minimal reproduction project (MRP)

N/A

AThousandShips commented 1 month ago

Related:

KoBeWi commented 1 month ago

Reject types, return to dynamic: var editor_interface = Engine.get_singleton("EditorInterface").

IMO this is the way. You can do this indirectly for partial safety:

func _ready():
    if Engine.is_editor_hint():
        var editor = load("res://some_script_using_editor_interface.gd")
        editor.connect_reimported(_on_resource_reimport)

where the loaded script does what you quoted above. It can provide a type-safe interface with only the script in the scene doing unsafe calls.

dalexeev commented 1 month ago

It can provide a type-safe interface with only the script in the scene doing unsafe calls.

This will not work with static typing, only with dynamic typing (if you use Engine.get_singleton() and so on for all editor classes).

As a workaround, I would suggest my plugin gdscript-preprocessor (sorry for the self-promotion).

We could try to solve this out of the box by adding an exception for editor classes when analyzing/compiling the script. However, at runtime this should still give an error if execution reaches this point. So I think that this is a rather complex solution, which also adds inconsistency.

KoBeWi commented 1 month ago

This will not work with static typing, only with dynamic typing (if you use Engine.get_singleton() and so on for all editor classes).

No I mean this (for some_script_using_editor_interface.gd from my example):

func connect_reimported(callback: Callable):
    var editor_filesystem := EditorInterface.get_resource_filesystem()
    if not editor_filesystem.resources_reimported.is_connected(callback):
        editor_filesystem.resources_reimported.connect(callback)

This code is type-safe, but it won't cause problems, because it's loaded using load(). The parent script does a dynamic call on this method. Hence I said "partially safe".

ydeltastar commented 1 month ago

Also, this isn't mentioned in the documentation and works when running from the editor leaving it to be discovery possible late in development only after exporting.

KoBeWi commented 1 month ago

Also related: #56798 (I just remembered)

ydeltastar commented 1 month ago

Related: https://github.com/godotengine/godot/issues/86013