Add `Engine::is_editor_hint()` equivalent for GDExtensions initialized before `INITIALIZATION_LEVEL_SCENE`

Describe the project you are working on

An extension that wraps the Steamworks APIs in higher-level functions and classes (e.g., a MultiplayerPeer that uses ISteamNetworkingSockets).

Describe the problem or limitation you are having in your project

(Background) Due to how the Steam overlay integrates with Vulkan, the Steamworks API need to be initialized before the window is created at INITIALIZATION_LEVEL_CORE or INITIALIZATION_LEVEL_SERVERS. However, initializing the API in the editor causes issues when running the game from the editor since there will be two processes simultaneous using the API for the same game. Additionally, the API is generally not useful in the editor and conflicts with hotkeys.

(Problem) Since the Steamworks API needs to be initialized before INITIALIZATION_LEVEL_SCENES, Engine::is_editor_hint() is unavailable because the Engine singleton has not yet been registered to ClassDB and exposed to the GDExtension API. This makes it impossible (as far as I am aware) to tell if the extension has been loaded by the editor or an instance of the game.

EDIT: Typos

Describe the feature / enhancement and how it helps to overcome the problem or limitation

The feature would expose a variable or method that tells GDExtensions if they were initialized from the editor. By checking this, GDExtensions could skip pre-INITIALIZATION_LEVEL_SCENE setup when running the Editor.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I have only skimmed the source code for the GDExtension interface, so there is probably much more to it than I know. However, with the little I do know, these are two possible implementations:

(1) Register the Engine singleton earlier The Engine singleton is created and Engine::editor_hint is set before INITIALIZATION_LEVEL_CORE, so it can be registered before then in Main::setup. I have been using this solution in a local build of Godot and have not encountered any issues.

The benefit of this solution are that (1) it is essentially a one-line change (move Engine::get_singleton()->add_singleton(Engine::Singleton("Engine", core_bind::Engine::get_singleton())) out of register_core_singletons() in Main::setup2 and place before initialize_modules(MODULE_INITIALIZATION_LEVEL_CORE) in Main::setup, (2) GDExtensions would use the existing Engine::is_editor_hint API to check if they are running in the editor, and (3) it does not break older GDExtensions. The drawback of this approach is that there is no reason to register the Engine singleton this earlier, aside from Engine::is_editor_hint.

(2) Add a member variable during initialization An is_editor_hint equivalent could be a member variable on GDExtensionInitialization or an argument for GDExtensionInitializationFunction. The benefit of this solution is that it decouples singleton registration from the proposed GDExtension is_editor_hint flag. The downsides are that (1) this would be another breaking change for GDExtension, (2) the API would be awkward (developers would have to store the flag and/or know to look in GDExtensionInitialization), and (3), after INITIALIZATION_LEVEL_CORE, the flag is redundant since Engine::is_editor_hint() is accessible.

My personal preference is for (1) since it is the simplest, most intuitive, and is not a breaking change. However, as I said before, my knowledge of GDExtension's and Godot's inner workings is limited, so there could be a much better solution I am missing.

If this enhancement will not be used often, can it be worked around with a few lines of script?

No. Engine::editor_hint is set based on a combination of compiler flags and command line arguments, so it cannot be reverse-engineered in the GDExtension and must be accessed through Engine::is_editor_hint. Exposing Engine::is_editor_hint or passing its value to GDExtension requires the engine to be modified.

Is there a reason why this should be core and not an add-on in the asset library?

This cannot be done through an add-on and is generally useful to GDExtensions that need to exclude the editor during setup before INITIALIZATION_LEVEL_SCENE.

Thanks!

This is related to a number of issues over the years around singletons not being registered early enough. While we haven't yet found a solution to that that's acceptable to everyone, I'd personally prefer to make the Engine singleton available earlier, as opposed to adding something new to the API for this specific case.

Here's some links to earlier issues/PRs (more probably exist):

We most recently discussed this at the GDExtension meeting on November 15th, 2023 (see notes), but we've discussed it numerous other times as well.

Thanks for sharing some of the history! I looked into directly calling get_singleton() instead of going through ClassDB as @BastiaanOlij proposed, and I have an implementation working:

static GDExtensionObjectPtr gdextension_global_get_singleton(GDExtensionConstStringNamePtr p_name) {
    const StringName name = *reinterpret_cast<const StringName *>(p_name);
    Object *o = Engine::get_singleton()->get_singleton_object(name);
    if (unlikely(o == nullptr && name == "Engine")) {
        return (GDExtensionObjectPtr)core_bind::Engine::get_singleton();
    } else {
        return (GDExtensionObjectPtr)o;
    }
}

The implementation modifies GDExtension's get_singleton interface to fall back to the core_bind wrapper around Engine iff it cannot get the singleton from ClassDB. Once the singleton is registered with ClassDB, it will go back to the existing logic.

If the solution were to be expanded to cover core singletons in general, I would rewrite the branch as:

if (likely(o != nullptr) {
    return (GDExtensionObjectPtr)o;
} else {
    return _gdextension_get_core_bind_singleton(name);
}

where _gdextension_get_core_bind_singleton is a new method that maps the singleton name to the direct get_singleton method (instead of using ClassDB).

I like this solution even more than (1). The benefits are that (1) it also a simple change, (2) it is isolated to GDExtension code, (3) it can be extended to other core singletons people need earlier, (4) it does not break the existing API, and (5) it avoids concerns about modifying the ClassDB registration order. The downsides I see are (1) performance concerns around adding branching to a hotpath and (2) philosophical objections to bypassing ClassDB. If there are no objections to this approach, I can write up the PR to get some more eyes and opinions on it.

Thanks for sharing some of the history! I looked into directly calling get_singleton() instead of going through ClassDB as @BastiaanOlij proposed, and I have an implementation working:

Hm, I don't think that implementation is what @BastiaanOlij had in mind. I think he was imagining that we'd register function pointers to the ::get_singleton() functions, and then have a way for GDExtension to call those. I think calling Engine::get_singleton()->get_singleton_object(name) as shown in your code snippet is what we already have.

We also discussed this for a little bit at the GDExtension meeting today, and myself and @vnen felt that simply registering the individual singletons at the point that they were ready to be used would be an acceptable solution. Some singletons (like Engine) are ready to be used very, very early, and can be registered early. Whereas others (like the physics or rendering servers) aren't ready to be used until much later, and so will need to be registered later.

However, not everyone has agreed with this viewpoint in the past. :-)

I think we need to discuss the possible solutions some more (perhaps at the next GDExtenion meeting - stop over to the #gdextension channel on chat.godotengine.org for details on that) and come together on some proposals. And then have another discussion with Juan and other core folks to find which of those proposals would be acceptable to them.

godotengine / godot-proposals