Add get_singleton() function in the prelude for easy access to autoloads

[ttencate]

Something like:

fn get_singleton<T>(name: impl Into<GodotString>) -> Result<Gd<T>, Error>

Right now you have to do something like:

self.get_node_as::<MySingleton>("/root/GlobalMySingleton")

That's not very intuitive.

If we implement get_singleton like this, it would need access to the scene tree. I'm not sure if there is a better way.

[lilizoey]

You can actually just use Engine::singleton().get_singleton(name) to get a singleton by name

[lilizoey]

Looking at this a bit more. My suggestion for a "proper" solution like this would be something like:

1. add attribute #[singleton(name="SomeName")] for GodotClass, similar to #[class(base=Base)]
2. add the name from this to the plugin system
3. make auto_register_classes call Engine::singleton().register_singleton() on each singleton
4. make the function get_singleton() call into and properly cast a call to Engine::singleton().get_singleton()

A simpler implementation could also be doable, which is a simple wrapper around Engine::singleton().get_singleton().

As a first issue, this is mostly gonna involve copying a lot of pre-existing functionality, as every bit of steps 1-4 is already done for registering the class itself. Concretely:

For 1: parse_class_attr, this is where the #[class(base=Base)] is parsed, and would likely look very similar for singleton.

For 2: transform, this is where the information about the class is actually added to the plugin system

For 3: auto_register_classes, this is where classes and their info is registered with godot.

For 4: There are many places that this function could be created to be included in the prelude, but im not entirely sure which place is the best one.

[Bromeon]

There are two problems:

1. Getting a singleton dynamically by name (suggestion in initial post)
2. Register user-defined singletons (what you describe in your latest response)

If you want to discuss 2., I would suggest to open a separate issue, as 1. could already be solved today for existing engine classes and is labeled good first issue.

I would however postpone "user-defined singletons" until after a threading model has been worked out. There is significant design needed if we want to provide an API beyond just "here's your mutex". Interaction with GDScript is also a concern. In other words, this is not on top of my priority list 😉

[lilizoey]

I'm not entirely sure what about this might require a more worked out threading model. Either the basic impl or the more complicated impl would both just call into Engine::get_singleton, and using Engine is thread safe according to the godot docs. And they'd return Gd<T> objects.

The more complicated version would just allow you to get the singleton by type instead of by name only. I.e

get_singleton::<MySingleton>()

instead of

get_singleton::<MySingleton>("MySingletonName")

If this function is gonna be specific to autoloads, then we shouldn't name it get_singleton, because Engine::get_singleton cannot access autoloads, only classes registered with Engine::register_singleton and so would likely cause confusion about why get_singleton and Engine::get_singleton work differently.

So we should maybe split this into several issues because there are several things we'd want:

1. An easy way to use Engine::register_singleton and Engine::get_singleton, preferably with Engine::register_singleton happening automatically
2. An easy way to access autoloads
3. A unified way of creating singletons with conveniences like mutexes and other things

[Bromeon]

True, the name "singleton" is overloaded -- thanks for pointing it out. I agree we should separate the "autoload" terminology.

Either the basic impl or the more complicated impl would both just call into Engine::get_singleton, and using Engine is thread safe according to the godot docs.

I just realized this opens another door of issues... Engine::get_singleton can be used to easily circumvent thread safety. Sure, the method is thread-safe itself, but that doesn't propagate to the class being returned. So I can set a singleton from one thread and access it from another, without synchronization (just a plain Gd<T>). This is not limited to get_singleton but can also apply to GDScript globals or the node tree. But it means that we need to add runtime checks if we don't want to cause race conditions.

It's just one example why this whole topic needs a holistic design, we cannot provide singletons as an isolated feature -- the fact that they're already available through engine APIs at best makes the status quo a bug.

[lilizoey]

True, since there's a lot of discussion here already we should probably make a new issue for making a convenient way of accessing an autoload.

[lilizoey]

Also probably just mark either register_singleton or get_singleton as unsafe for now (or both?)

[StatisMike]

It should be noted that the OP message is mixing the two Godot concepts (which Godot mixes itself very often, though).

Engine::singleton().get_singleton(name) refers to the engine singleton, which is stored as an Object. Mostly GodotClasses which have purely scripting logic (GDScript / Rust class).

For engine singletons there are many things to consider yet, mainly ownership, thread-safety and register/unregister mechanism.

self.get_node_as::<MySingleton>("/root/GlobalMySingleton") refers to autoloads, which can be registered either by EditorPlugin or by Godot Editor. It supports both GDscript classes (I don't think pure Rust class can be registered as autoload, only the GDScript class extending Rust class) and premade scenes (with some logic-containing GodotClass as its root). It is recommended to use if access to the current scene tree is required.

So, instead of suggested function fn get_singleton<T>(name: impl Into<GodotString>) -> Result<Gd<T>, Error> it should be actually fn get_autoload (though some things should be figured out first, eg. #566 ), possibly a method on Gd<T: Inherits<Node>>, unless some global workaround could be find.