godot-rust/gdext

Mimic `onready` vars

WinstonHartnett opened this issue ยท 3 comments

Currently the pattern for structs with Gd references (which have to be manually initialized in ready) looks like:

#[derive(GodotClass)]
#[class(base=Node)]
struct MyStruct {
    my_child: Option<Gd<Node>>
    #[base]
    base: Base<Node>
}

Then, map, unwrap, or user-defined accessors are needed to bypass the Option.

How about:

#[derive(GodotClass)]
#[class(base=Node)]
struct MyStruct {
    #[on_ready("MyChild")]
    my_child: OnReady<Gd<Node>>
    #[base]
    base: Base<Node>
}

where OnReady is a MaybeUninit<Gd<T>> with a Deref/DerefMut instance to avoid unwrapping.

It certainly clutters the struct definition a bit more, but you avoid:

  • Initializing an Option in ready (which can easily be forgotten)
  • Using map or accessors to get the obviously initialized field
  • Paying for the Option<Gd<T>>'s 16 bytes compared to Gd<T>

That's a nice idea, and it already came up in godot-rust/gdnative#758 (comment) ๐Ÿ™‚

A few things to note:

  • Option<Gd<T>> could in theory be optimized by storing the object pointer as NonNull<...> instead of OpaqueObject, or another encoding pattern of which rustc recognizes the null state. Or we use a union and do it manually. This is very low priority though.
  • "Using map or accessors to get the obviously initialized field" -- this is not true. _ready is invoked when the node is added to the tree; initialization happens before. There are even two callbacks _init and _enter_tree that happen prior to _ready. Which implies that the container must be able to represent a null state -- preferably a safe one.
  • The OnReady<T> type needs to make sure it correctly handles reference-counting and destruction, especially in case it doesn't directly hold a Gd<T> type.

An alternative syntax to consider is generalization into a trait (all identifiers subject to bikeshedding):

trait Fetch {
    fn fetch(root: Gd<Node>, path: &str) -> Result<Self, FetchError>;
}

impl<T: GodotClass + Inherits<Node>> Fetch for Gd<T> {
    // snip
}

#[derive(GodotClass)]
#[class(base=Node)]
struct MyStruct {
    #[on_ready(fetch("MyChild"))]
    my_child: Option<Gd<Node>>,
    #[base]
    base: Base<Node>
}

Which could later open up possibilities of using separate types for sub-trees, for code organization and/or reuse:

#[derive(Fetch)]
struct MyChildren {
    #[fetch("MyChild")]
    my_child: Gd<Node>,
    #[fetch("MyChild2")]
    my_child_2: Gd<Node>,
    #[fetch("MyChild3")]
    my_child_3: Gd<Node>,
}

#[derive(GodotClass)]
#[class(base=Node)]
struct MyStruct {
    #[on_ready(fetch("."))]
    my_child: Option<MyChildren>,
    #[base]
    base: Base<Node>
}

There is some existing discussion about this syntax here, that apart from the safety aspects should also apply to gdextension: godot-rust/gdnative#980

I have some experiments ongoing on branch late-init ๐Ÿ™‚

It's indeed a bit tricky, my current approach is a specific OnReady<T> type, which is basically Option<T> but more ergonomic. I'm not yet sure if/how it will play together with the proc-macro API, but also I'm not too big of a fan of #[init(default)], so some things may change there...

Anyway, I'll open a PR once there's something ready (no pun intended).