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

OnReady<T> for late-init fields #534

Merged
merged 4 commits into from
Dec 12, 2023
Merged

OnReady<T> for late-init fields #534

merged 4 commits into from
Dec 12, 2023

Conversation

Bromeon
Copy link
Member

@Bromeon Bromeon commented Dec 11, 2023

OnReady: Ergonomic late-initialization container with ready() support

While deferred initialization is generally seen as bad practice, it is often inevitable in game development. Godot in particular encourages initialization inside ready(), e.g. to access the scene tree after a node is inserted into it. The alternative to using this pattern is Option<T>, which needs to be explicitly unwrapped with unwrap() or expect() each time, and can be accidentally overridden.

OnReady<T> should always be used as a field. There are two modes to use it:

  1. Automatic mode, using new().
    Before ready() is called, all OnReady fields constructed with new() are automatically initialized, in the order of
    declaration. This means that you can safely access them in ready().
  2. Manual mode, using manual().
    These fields are left uninitialized until you call init() on them. This is useful if you need more complex
    initialization scenarios than a closure allows. If you forget initialization, a panic will occur on first access.

Conceptually, OnReady<T> is very close to once_cell's Lazy<T>, with additional hooks into the Godot lifecycle.
The absence of methods to check initialization state is deliberate: you don't need them if you follow the above two patterns.
This container is not designed as a general late-initialization solution, but tailored to the ready() semantics of Godot.

This type is not thread-safe. ready() runs on the main thread and you are expected to access its value on the main thread, as well.

Example

use godot::prelude::*;

#[derive(GodotClass)]
#[class(base = Node)]
struct MyClass {
   auto: OnReady<i32>,
   manual: OnReady<i32>,
}

#[godot_api]
impl INode for MyClass {
    fn init(_base: Base<Node>) -> Self {
       Self {
           auto: OnReady::new(|| 11),
           manual: OnReady::manual(),
       }
    }

    fn ready(&mut self) {
       // self.auto is now ready with value 11.
       assert_eq!(*self.auto, 11);

       // self.manual needs to be initialized manually.
       self.manual.init(22);
       assert_eq!(*self.manual, 22);
    }
}

@Bromeon Bromeon added feature Adds functionality to the library c: register Register classes, functions and other symbols to GDScript labels Dec 11, 2023
@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-534

@Bromeon
Copy link
Member Author

Bromeon commented Dec 11, 2023

Alternative designs

Self type parameter

I was considering OnReady<T, C> where C would be Self of the containing class (or default () if not needed).
The idea was to support initializations like this:

    fn init(_base: Base<Node>) -> Self {
       Self {
           auto: OnReady::new_self(|this: &mut Self| { // [1]
                self.compute_thing()
           }),
       }
    }

The problem here is that it's not possible to hand out &mut self and &mut self.field (the one being initialized) at the same time. To work around this, we'd need interior mutability or some way to partially borrow, which would make the whole thing considerably more complex.

Dependencies between fields are not ideal, but they do happen, and I think they are best solved in a procedural way inside the actual ready() function. I'm generally wondering if this "auto-init" is that useful in practice, maybe it turns out that the "auto-deref" is the actual feature here, and the container sees more use via OnReady::manual().

Before-ready vs. Lazy

I was considering to just mimick Lazy's semantics, with the added option of manual initialization (and panic on access). However, I wanted to stay as close as possible to Godot's @onready attribute, which means the value is initialized before ready. We also ensure initialization in the order of declaration (not sure if often needed in practice, though).

This was actually non-trivial to implement, because I needed to consider 3 cases:

  1. User declares an impl INode which overrides virtual functions, and in that defines ready.
  2. User declares impl INode, but does not override ready -> gdext needs to manually supply a ready that does the init.
  3. User declares only the struct without an impl -> gdext still needs to manually register the ready lifecycle hook, otherwise the fields are uninitialized.

After-ready checks

What is not yet implemented but possible, would be to verify that manual OnReady instances are truly initialized after ready() is invoked. This would mean:

  • Classes that have manual OnReady fields but no ready method would always panic, when added to the scene tree.
  • "Forgot initialization" errors are revealed at a deterministic time, not only at the point of access.
  • It might make some prototyping slightly more cumbersome, as it's not possible to leave variables temporarily uninitialized.

Auto-detecting type in proc-macro

I first had an explicit #[onready] attribute, but would like to experiment with detecting the type from the signature in the field. I also have some escape hatches planned if this heuristic ever fails (e.g. type aliases, foreign types with same name).

This detection not only leads to more concise code, but also prevents the user from forgetting an attribute and then wondering why variables are not initialized. If this works well, we could apply the same principle to #[base], effectively detecting Base<T> types.

Extra checks

We could also check whether the class truly inherits Node, as the fields make no sense otherwise. Some of these checks might be more straightforward with the Rust type system than proc-macro impls, so they could also be postponed to builder APIs.

Wider initialization support

This purely provides a value container at the moment. There were discussions about mapping scene trees to Rust data structures, see e.g. #130 (comment). This is something that can be discussed for the future, depending on the experience with this addition.

Besides OnReady, I'll rework some of the initialization. I'm not happy with #[init(default)] which doesn't solve a real problem (beyond syntax) and causes confusion in presence of init function, for example. Or the fact that #[class(init)] can be easily forgotten, leading to weird runtime errors.


[1]: Note that |this| won't work, we need |this: &mut Self|, even though the type is unambiguous.
See Rust discussion about type inference rules as well as Discord thread.

@Bromeon Bromeon linked an issue Dec 11, 2023 that may be closed by this pull request
@Bromeon
Copy link
Member Author

Bromeon commented Dec 12, 2023

Merging this for now, allowing for early feedback and follow-up improvements in separate PRs.

@Bromeon Bromeon added this pull request to the merge queue Dec 12, 2023
Merged via the queue into master with commit ef5f388 Dec 12, 2023
17 checks passed
@Bromeon Bromeon deleted the feature/onready branch December 12, 2023 16:45
@Bromeon Bromeon mentioned this pull request Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: register Register classes, functions and other symbols to GDScript feature Adds functionality to the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mimic onready vars
2 participants