Support using Base for initialization in init function

[lilizoey]

When initializing an object in Godot, it is a fairly common pattern in c++ to do most of the setup in the init function. But as of #497 and #501, it is now impossible to directly use the Base object to do anything (at least without using #[doc(hidden)] functions). That means that in order to call any methods on base to set up the class, you'd need to first create the Self object and then use a method like WithBaseField::to_gd, like this:

fn init(base: Base<Self::Base>) -> Self {
  let self_ = Self { .., base };
  self_.to_gd().some_method();
}

However this means that every field in Self has some sensible default, that doesn't require any calls to the base object. It would be nice to provide an official solution for this pattern.

This was initially discussed in this discord thread https://discord.com/channels/723850269347283004/1193536016968073236, i will summarize the solutions we discussed here.

Possible Solutions

1. Make Base's to_gd method public. This method is currently #[doc(hidden)], but using it solves the issue. However the reason it is hidden is because it may cause confusion in most other functions, where it may be unclear if people should use self.base.to_gd() or self.to_gd() or even self.base_mut(). As well as what the difference is. So if we do this we should likely rename it to make it more clear.
2. Have a "transport base" and a "stored base". Here we would add a new type (the transport base) that can be used to call methods on base, and it is what is passed to init. Whereas the stored base is what we currently have and is what is stored in the struct. This would however require us adding a new type that is only used in this one situation.
3. Pass in a Gd of base to the init function. This might however be confusing as both the Base and Gd point to the same thing, and people might be tempted to store the wrong object in the struct.
4. Add a post-init function (possibly replacing the original init function). This does make it easy to do initialization using the base object, however it doesn't solve the issue that every field needs a sensible default.

I am currently inclined to go with the first option, to make to_gd public. It seems the simplest to me and giving it a new name with good documentation would hopefully solve the confusion issue.

[Bromeon]

Another potential option:

5. Have Base in two possible states: transport + initialized.
   • In the transport state, one can call methods on it directly. We could allow to_gd() only in that phase, maybe with a better name, such as during_init() or whatever.
   • In the initialized phase, it would behave exactly like today, but during_init() would panic with an expressive message.

In other words, type-state without introducing yet another type (which is also an option, but really makes the API even more complex).

[mhgolkar]

1. Make Base's to_gd method public. This method is currently #[doc(hidden)], but using it solves the issue.

Yes please. At least as a quick/temporary solution.
Actually I was going to open an issue and ask for it!
Ability to manipulate a node during init stage is a must-have and right now this shy API is probably the only way to have that.

However the reason it is hidden is because it may cause confusion in most other functions, where it may be unclear if people should use self.base.to_gd() or self.to_gd() or even self.base_mut(). As well as what the difference is. So if we do this we should likely rename it to make it more clear.

May I suggest to document the situation, and why and when someone shall or shall not use this API (instead of hiding it).

[MatrixDev]

I'll just copy it here from my duplicate issue so it would not be lost.

My case was a little different, I want to have a fully constructed tree when init finishes but it has a lot in common. One more solution is to add new or change init function:

fn init() -> Gd<Self> {
    let mut node = Node3D::new_alloc();
    // do something with node

    let mut this = Gd::from_init_fn(|base|
        Self {
            node: node.clone(),
            base,
        }
    );
    this.base_mut().add_child(node.upcast());
    this
}

[mhgolkar]

My case was a little different, ...

Hi @MatrixDev

Why not implementing it this way without from_init_fn:

fn init(base: Base</*Super*/Node>) -> Self /* Extended */ {
    let mut base_gd = base.to_gd();
    let inner_node = Node3D::new_alloc();
    // ... manipulate the inner node:
    // e.g. inner_node.set("foo_property", bar_variant);
    // ...
    base_gd.add_child(inner_node.clone().upcast());
    // ...
    Self {
        base,
        inner_node
    }
}

Just a side note: Currently .to_gd is #[doc(hidden)] so your IDE may not suggest it, but it's going to be there!

• edit:
  "For the time being" I should have added (read below).
  You are already aware of this as a workaround (mentioned in your issue), but I like to promote for this to be a public API until there is a better solution.

[Bromeon]

but it's going to be there!

There's no guarantee. #[doc(hidden)] is explicitly NOT part of the public API.
So we do not take special precautions when it comes to breaking code.

[mhgolkar]

There's no guarantee. #[doc(hidden)] is explicitly NOT part of the public API.

Sure! I should have added "for the time being"!

[MatrixDev]

Sure! I should have added "for the time being"!

I'm currently doing exactly this. But my issue #585 was No recommended way to call Base<T> methods from init.

Also there is a concern that we basically have "semi-invalid" object from when Self {} is constructed and init returns. But it probably should be a separate issue.

[RedMser]

In case anyone stumbles upon this issue, it's a bit different for RefCounted classes like Resources.
I only got it to work by intentionally leaking memory, otherwise the parent reference always ended up as a nullptr 🔥

#[derive(GodotClass)]
#[class(base=Translation)]
pub struct TranslationFluent {
    base: Base<Translation>,
}

#[godot_api]
impl ITranslation for TranslationFluent {
    fn init(base: Base<Translation>) -> Self {
        std::mem::forget(base.to_gd()); // <-- this was needed to fix crashes
        base.to_gd().set_locale("".into());

        Self {
            base,
        }
    }
}

[fpdotmonkey]

Does the gdextension team have a position on what should go in init vs ready? It could be that the gdextension API was intended to be like gdscript in the sense of most godot initialization operations should happen on ready. The argument in favor of doing more initialization on init is that that's how engine classes do it, but they critically are not built with gdextension.

if manipulating base in init is what's intended for this API, I'm in favor of this change. But failing that, this just adds a second way to do the same thing when there was already a good logic for what initializes where (rust stuff in init, godot stuff in ready).

[RedMser]

to be like gdscript in the sense of most godot initialization operations should happen on ready.

That's simply not true. A primary use case is any class that does not inherit Node, does not have a ready function. Initializing it must be done within a custom constructor or init.

See my code snippet, I'd like for all created Translations to start with an empty locale, overriding the default of "en". There is no other way to do this in a way that Godot knows that this is the new default value (and won't show a revert arrow in the inspector).

[lilizoey]

I think the issue with RefCounted classes can be fixed relatively easily by just initializing the refcount earlier in the construction process. Since currently the refcount isn't initialized by the time init is called.

[lilizoey]

So that does sorta fix the issue, but there are some memory leaks and i dont think there is a way to fix those. Since if the refcount is initialized by init then if a class is created from godot then we just cant tell that the refcount shouldn't be incremented.

The fundamental problem with this is that our Gd will try to do reference counting, but specifically in the init function we must avoid reference counting the base object, as otherwise the refcount can hit 0 since Base doesn't initialize the refcount. But as soon as we let people access the base object as a Gd it is impossible to prevent them from just getting a Gd out of it which will attempt to reference count stuff.

[lilizoey]

I guess one thing that could be doable is, initialize the refcount early. And then keep track in Base if the refcount needs to be decremented at some point, and find an appropriate place which we know for sure will be called after initialization at some point for all classes in which we can decrement the refcount.