godot
godot copied to clipboard
GDExtension classes can easily crash the editor on startup due to always having `@tool` semantics
Godot version
4.0-dev
System information
System independent
Issue description
GDExtensions classes will crash the editor if their built in functions (_process, _ready) contain references to nodes that do not exist in the editor's scene tree.
I believe this is because they are now treated like core classes, which run _process and _ready in the editor since the editor is godot, as far as they're concerned. So if users have code in _process or _ready that reference non-existent nodes (from the editor's perspective), they will segfault and prevent the editor from opening.
Possible work arounds are using an ifdef macro to disable the offending portions of code when the editor needs to be open, or using if Engine::get_singleton()->is_editor_hint().
I realize this is not a bug, per se, since it's how the core classes work. However, GDNative did not work like this, and GDExtensions is intended to replace GDNative, so at the very least, the documentation should reflect this, in my opinion, major change. I'm sure people will be assuming this is a bug as more GDNative users migrate to GDExtensions, so it will be good to have a bug report that documents the issue.
Steps to reproduce
Create a GDExtension, and set up a reference to a node in the scene tree that doesn't exist in the editor, but will exist at runtime. It will crash.
Minimal reproduction project
No response
Why close?
I guess because it's not a bug, technically. It's expected behavior based on the design of core classes and thus GDExtensions. But it will be seen as a bug and so people searching for the bug will be able to find the solution. You can reopen if you wish.
Won't be an ifdef but we definately need access to is_editor_hint, I'm surprised we haven't got access to this.
Your classes running both in the editor and in runtime is a bonus to the system, but yes it does mean that you have to keep that in mind when writing extensions in the same way you would need to do this with modules.
Note btw that the initialiser you define with register_editor_initializer will only be called if you're in the editor, so it would be possible to set a flag so you know if you're in the editor or not. Feel workaround-ish, is_editor_hint is definitely better.
However, GDNative did not work like this (...)
How did this work with GDNative?
Is this issue about GDExtension types behaving similarly like tool scripts? (where instances are created withing the editor and callbacks are invoked).
For the record, related issues:
- https://github.com/godotengine/godot-cpp/issues/709
- https://github.com/godot-rust/gdextension/issues/70
Would it be possible to register an extension to have "run at runtime only" semantics entirely? Littering the code with lots of manual is_editor_hint checks is ugly and error prone. It also feels odd that the finally released game contains lots of logical branches that were only introduced for development purposes.
It is a bit strange to make the "tool" semantics the default even without the possibility to opt-out considering that they are clearly dangerous as emphasized by the docs:

Would it be possible to have a dedicated _process_editor for GDExtensions, separated from the regular _process? If the extension does not need to be run in the editor, the member function is simply omitted.
Would it be possible to have a dedicated _process_editor for GDExtensions, separated from the regular _process?
I don't think this is the way to go, because it is not only _process, but basically every virtual function that would need to be "doubled" into an _editor version. If the extension is supposed to be run in the editor, all of them need to forward the call to the regular version which is tedious. And this would go even more against the idea that production relevant code should not be littered with development only code (the entire virtual function interfaces would be littered now). It would be much cleaner to have a single switch on extension registration level.
As it already been considered to just reproduce the behavior of GDScripts?
- assume by default that Node-inheriting classes are all not-a-tool (not the current behavior)
- allow classes to expose a
bool is_editor_tool()that will be querried by the engine at entity init, if returning true it behaves as it does currently, like a GDSCript having@tool- otherwise it behaves like a GDScript which do not have@tool Engine::get_singleton()->is_editor_hint()still usable exactly the same way it would be usable in a GDScript which is marked@tool, therefore no need to duplicate code.
Also, what was the reasonning behind making GDExtension Node-inheriting types behave differently by default than a default GDScript?
Cross-posting from https://github.com/godotengine/godot-cpp/issues/709#issuecomment-1591995717:
GDExtensions are ideally meant to work like Godot modules. In a Godot module, nodes always run as if they are marked with
@tool, and they useEngine::get_singleton()->is_editor_hint()(as mentioned by Calinou in the first comment) to check if they are currently running in the editor. Yes, this is different than GDScript, but I believe this is by design.
Something that has been discussed on this one is maybe making a "CPPScript" (which could still be part of godot-cpp) that allows making scripts in C++, which would work closer to how GDScript works (and how GDNative worked in Godot 3).
This could potentially help by splitting up the use cases of (1) wanting to make a GDExtension that's like a Godot module (but doesn't require recompiling Godot) and (2) wanting to do game scripting in C++ (ie. something closer to GDScript)
Ok so the CPPScript solution would work to solve both this issue and the issue hot-reloading issue.
If I understand correctly, if we go that route it would mean that C++ Node inheriting classes which are not C++Script (so for example inheriting directly from Sprite2D) will act like @tool marked scripts, while other C++ CPPScript inheriting classes would act like GDScripts.
I suspect that it might be a big difference, just for checking a static condition, but I'm not against it.