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

must_have_an_init_method is too strict #617

Closed
MatrixDev opened this issue Feb 20, 2024 · 5 comments
Closed

must_have_an_init_method is too strict #617

MatrixDev opened this issue Feb 20, 2024 · 5 comments
Labels
question Not a problem with the library, but a question regarding usage.

Comments

@MatrixDev
Copy link

Lets just say introduction of must_have_an_init_method::__type_check just broke half of my project...

I'm not using #[class(init, ...)] nor defining init in #[godot_api] because those don't allow me to pass custom arguments. So I just used a simple pattern for most of my objects:

impl MyControl {
    pub fn new(dependency: Gd<Node3d>) -> Gd<Self> {
        let child = Node3d::new_alloc();

        let mut this = Gd::from_init_fn(|base| Self {
            base,
            child: child.clone(),       // <-- no need for Option here, which is huge for me !!
            dependency,                 // <-- same here for non-trivial / default types
        });
        this.add_child(child.upcast());
        this
    }
}

Is there any way to construct objects without the need of Option now?

@StatisMike
Copy link
Contributor

Godot itself expects to construct and destroy all declared classes with the init function while opening editor - for full rationale, you should look into the upstream. You could test declaring #[class(no_init, ...)] - I think it should work, though you are then opting out of the potential benefits of this behavior. Additionally, I think that classes without init function declared can't be ever constructed or declared within the Godot Editor itself (eg. if you wanted to #[export] Node in the field to assign it in editor, which can be very useful).

If your objects are descendants of Node, you can add the fields as OnReady<T> (though then you can't #[export] them.

However, if you are creating the objects through from_init_fn you can also add the blanket init in it, as long as the fields have some default value - just for the Godot to run its init -> destroy cycle and give you an option to assign other nodes in editor. You don't need to call the init function in your code anywhere.

@Bromeon
Copy link
Member

Bromeon commented Feb 20, 2024

@MatrixDev check out #593, the new API is explained there. Everything you did before is still possible 🙂

@Bromeon
Copy link
Member

Bromeon commented Feb 20, 2024

@StatisMike

However, if you are creating the objects through from_init_fn you can also add the blanket init in it, as long as the fields have some default value - just for the Godot to run its init -> destroy cycle and give you an option to assign other nodes in editor. You don't need to call the init function in your code anywhere.

I strongly recommend against this pattern, unless you actually need default-constructibility. It weakens invariants, introduces "zombie states" and requires Option<T> or other workarounds, instead of enforcing concrete values in the constructor.

Some Rust classes are never directly needed in the editor, but just passed around in GDScript code. It's valid to write MyClass.custom_constructor(args) instead of MyClass.new(), or to never construct the instance outside Rust.

@Bromeon Bromeon added the question Not a problem with the library, but a question regarding usage. label Feb 20, 2024
@StatisMike
Copy link
Contributor

@Bromeon Yeah, thanks for the clarification. If GodotClass isn't going to be a part of the scene tree at all, it should be safe to just include no_init. I'm not entirely sure about the init -> destruct behavior of Godot - if it's purely for making sure that the class can be constructed within the editor you can replace the should be with is in the above sentence (I cannot find the rationale for that right now).

@MatrixDev
Copy link
Author

@MatrixDev check out #593, the new API is explained there. Everything you did before is still possible 🙂

Thanks, now everything work. I should've read updated docs more closely, my bad...

Now I also undestand why hot-reloading stopped working for me when I've started populating project with classes (#539).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Not a problem with the library, but a question regarding usage.
Projects
None yet
Development

No branches or pull requests

3 participants