Skip to content

I suggest moving this &str #79

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

greenfox1505
Copy link
Contributor

IMHO, for portability, it should be suggested you put your name in your struct, instead of placing the string inside the register/de-register method.

IMHO, for portability, it should be suggested you put your name in your struct, instead of placing the string inside the register/deregister method.
@Bromeon
Copy link
Member

Bromeon commented Feb 28, 2025

If we already do that, then probably class_name() is better than a user-maintained constant?

@greenfox1505
Copy link
Contributor Author

I thought about suggesting something like: std::any::type_name::<Self>().into() but then if you're gunna do that, you might as well put it in the trait. Which turns into scope creep. I just wanted a standard place to look from names, but if y'all wanna turn it into something better, please do.

@Yarwin
Copy link
Contributor

Yarwin commented Feb 28, 2025

While we are at it, I usually use something like that for user-defined singletons:

pub trait UserSingleton:
    GodotClass + Bounds<Declarer = bounds::DeclUser> + NewAlloc + Inherits<Object>
{
    const NAME: &'static str;

    fn singleton() -> Gd<Self> {
        Engine::singleton()
            .get_singleton(Self::NAME)
            .unwrap()
            .cast::<Self>()
    }

    fn initialize() -> Gd<Self> {
        let game_system = Self::new_alloc();
        Engine::singleton().register_singleton(Self::NAME, &game_system);
        game_system
    }

    fn exit(&mut self) {
        Engine::singleton().unregister_singleton(Self::NAME);
    }

    #[allow(unused_variables)]
    fn physics_process(&mut self, delta: f64) {}

    #[allow(unused_variables)]
    fn process(&mut self, delta: f64) {}
}

It allows to ensure that user-defined singletons can be used in the same fashion as engine-declared/code generated ones

@Bromeon
Copy link
Member

Bromeon commented Feb 28, 2025

Imo these suggestions, while good ones, miss the point of that page: how to declare a singleton in the easiest way.

Fancy abstractions -- and singleton registration itself -- should eventually be provided in the library.

So what about this? It already exists:

If we already do that, then probably class_name() is better than a user-maintained constant?

@Yarwin
Copy link
Contributor

Yarwin commented Feb 28, 2025

If we already do that, then probably class_name() is better than a user-maintained constant?

Note: I tested it briefly with gdscript and seems to work fine 👍 – I haven't noticed any clashes nor issues with namespace.

Example code
struct MyExtension;


#[gdextension]
unsafe impl ExtensionLibrary for MyExtension {
    fn on_level_init(level: InitLevel) {
        if level == InitLevel::Scene {
            MyEditorSingleton::initialize();
        }
    }

    fn on_level_deinit(level: InitLevel) {
        if level == InitLevel::Scene {
            let mut my_singleton = MyEditorSingleton::singleton();
            my_singleton.bind_mut().exit();
            my_singleton.free();
        }
    }
}

#[derive(GodotClass)]
#[class(init, base=Object)]
struct MyEditorSingleton {
    base: Base<Object>,
}

#[godot_api]
impl MyEditorSingleton {
    #[func]
    fn foo(&mut self) {
        godot_print!("foo");
    }
}

impl UserSingleton for MyEditorSingleton {} // everything is provided by default impl - no macro magic needed 

pub trait UserSingleton:
GodotClass + Bounds<Declarer = bounds::DeclUser> + NewAlloc + Inherits<Object>
{

    fn singleton() -> Gd<Self> {
        Engine::singleton()
            .get_singleton(&Self::class_name().to_string_name())
            .unwrap()
            .cast::<Self>()
    }

    fn initialize() -> Gd<Self> {
        let game_system = Self::new_alloc();
        Engine::singleton().register_singleton(&Self::class_name().to_string_name(), &game_system);
        game_system
    }

    fn exit(&mut self) {
        Engine::singleton().unregister_singleton(&Self::class_name().to_string_name());
    }

    #[allow(unused_variables)]
    fn physics_process(&mut self, delta: f64) {} // in real world implementation should be handled inside the class; docs should provide ways to call it (via the GameLoop class)

    #[allow(unused_variables)]
    fn process(&mut self, delta: f64) {} // as above
}

image

@Bromeon
Copy link
Member

Bromeon commented Feb 28, 2025

Thanks!

(Maybe we should at some point add impl AsArg for ClassName...)

@Bromeon
Copy link
Member

Bromeon commented Mar 3, 2025

@greenfox1505 could you change this to use ::class_name(), which avoids defining a new constant?

I'll then consider impl AsArg<StringName> for ClassName in parallel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants