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

4.0 GDExtension: global_get_singleton not returning expected singletons. #64975

Open
Splizard opened this issue Aug 28, 2022 · 4 comments
Open

Comments

@Splizard
Copy link
Contributor

Splizard commented Aug 28, 2022

Godot version

v4.0.alpha.custom_build.0c5f25495

System information

Ubuntu 22.04 Vulkan

Issue description

I'm writing a set of GDExtension/GDNative Go bindings and attempting to load all of the global singletons. I have tried loading them both at level 3 initialization time (GDNATIVE_INITIALIZATION_EDITOR) and/or when _ready is called on a extension class. A select number of them fail to load and while I expected that Javascript and JavaClassWrapper fail because they require certain environments. I did not expect the Server types to fail.

ERROR: Failed to retrieve non-existent singleton 'JavaClassWrapper'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'JavaScript'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'DisplayServer'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'RenderingServer'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'AudioServer'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'PhysicsServer2D'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'PhysicsServer3D'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'NavigationServer2D'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'NavigationServer3D'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'XRServer'.
   at: get_singleton_object (core/config/engine.cpp:251)
ERROR: Failed to retrieve non-existent singleton 'CameraServer'.

I might add that calling Engine.get_singleton("DisplayServer") for example, does work from GDScript. Is there something special about these 'Server' singletons? How is the initialization for these singletons meant to work from GDExtension?

Steps to reproduce

Call global_get_singleton on a GDNativeInterface with any of the failing cases shown in the Issue Description.

Example

# include "gdnative_interface.h"

const GDNativeInterface *api;

void deinitialize(void *userdata, GDNativeInitializationLevel p_level) {}

void initialize(void *userdata, GDNativeInitializationLevel p_level) {
    if (p_level == GDNATIVE_INITIALIZATION_EDITOR) {
        api->global_get_singleton("DisplayServer");
    }
}

GDNativeBool loadExtension(const GDNativeInterface *p_interface, const GDNativeExtensionClassLibraryPtr p_library, GDNativeInitialization *r_initialization) {
    r_initialization->minimum_initialization_level = GDNATIVE_INITIALIZATION_EDITOR;
    r_initialization->initialize = initialize;
    r_initialization->deinitialize = deinitialize;
    api = p_interface;
    return 1;
}
@Calinou
Copy link
Member

Calinou commented Aug 28, 2022

@Splizard Please upload a minimal reproduction project to make this easier to troubleshoot.

@Splizard
Copy link
Contributor Author

Minimal reproduction project
64975.zip

Only a linux.64 shared library file is included, so you will need to build the minimal main.c file and add it to example.gdextension in order to debug this on other platforms.

Splizard added a commit to Splizard/godot that referenced this issue Oct 12, 2022
Fixes godotengine#64975 by enabling server singletons to be retrieved at
initialization time.

Previously, GDExtension libraries would fail to return Server singletons
ie. 'global_get_singleton' at any defined initialization level.

This enables Server singletons to be retrieved at
GDNATIVE_INITIALIZATION_COMPLETE time so that GDExtensions can load all
singletons on startup.
@groud groud added this to the 4.0 milestone Dec 15, 2022
@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 27, 2023
@BastiaanOlij
Copy link
Contributor

I'm running into this too with XR interfaces implemented in GDExternal. I've updated my reference implementation today so it can be reproduced with that.

This line fails:

XRServer *xr_server = XRServer::get_singleton();

https://github.com/BastiaanOlij/godot_xr_reference/blob/update_godot_4_stable/src/register_type.cpp#L19

Bit of a showstopper, shame we mist it for the stable release.

@BastiaanOlij
Copy link
Contributor

Ok, looking into this some more, the problem is due to this:
https://github.com/godotengine/godot/blob/master/main/main.cpp#L2315

	MAIN_PRINT("Main: Load Physics");

	initialize_physics();
	initialize_navigation_server();
	register_server_singletons();

register_server_singletons is where the singletons are registered for access by scripts.

This is after:
https://github.com/godotengine/godot/blob/master/main/main.cpp#L2309

	initialize_modules(MODULE_INITIALIZATION_LEVEL_EDITOR);

Which is the last time we call into the initialization of GDExternals meaning that GDExternals has no way to interact with the server singletons, even though some of them have long since been created.

The solution in @Splizard PR was to introduce a new initialization level, which we definitely don't want to do.

The problem that we have in this is that there have been many discussions over time about the timing of registering our singletons, and reasons for why the timing was changed. These reasons are poorly documented so changing the timing once again may cause other problems to surface.

For my use case, I definitely need access to the XRServer singleton when we are calling into initialize_modules(MODULE_INITIALIZATION_LEVEL_SERVERS); as XRInterfaces need to potentially be registered with the XRServer before our graphics pipeline is setup.

The question here to answer is: What is the reasons all the servers need to be registered together?
This is delaying the moment of registering because we don't create, say they physics server until much later, and this is correct.
And there is an argument that only registering some servers, will lead to questions by users why the server they need, isn't available. There is something to say for consistency.

The flipside of the argument is that not registering servers as soon as possible after they have been instantiated, makes our initialization levels pretty much useless.

This will need further discussion with the results documented properly once and for all, but seeing the core team is currently preoccupied with GDC, this will likely happen afterwards.

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

6 participants