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

Use inventory or linkme to register scripts automatically #350

Closed
ghost opened this issue May 20, 2020 · 6 comments · Fixed by #999
Closed

Use inventory or linkme to register scripts automatically #350

ghost opened this issue May 20, 2020 · 6 comments · Fixed by #999
Labels
c: export Component: export (mod export, derive) feature Adds functionality to the library help wanted
Milestone

Comments

@ghost
Copy link

ghost commented May 20, 2020

While using the bindings, one might create a new script type but forget to register it in init. It's a minor annoyance, but one that often leads to confusing errors when the new type is used, and is not handled by the compiler. Implementing automatic script registration improves usability and reduces the chances of runtime bugs.

Provisions for automatic script registration seem to exist in the bindings for D, Kotlin, and Nim.

Currently, there are two crates in the Rust ecosystem that enable relatively easy automatic registration: inventory and linkme. The former is implemented through module initialization/teardown functions. The latter is based on link_section attributes. Both allow us to collect a list of fn(&mut InitHandle) -> () pointers before nativescript_init and use them to automatically register all script types that derived NativeClass. The expected user-facing API is the same regardless of the implementation detail:

// The derive macro will automatically insert the relevant code.
#[derive(NativeClass)]
struct Foo;

// A separate attribute can be used for types with manual `NativeClass` impls.
#[gdnative::register]
struct Bar;

impl NativeClass for Bar {
    /* - snip - */
}

// No callbacks necessary. It just works!
godot_gdnative_init!();
godot_nativescript_init!();
godot_gdnative_terminate!();

Manual registration should still be possible for types with manual NativeClass impls:

struct ILikeBoilerplateCode;

impl NativeClass for ILikeBoilerplateCode {
    /* - snip - */
}

fn init(handle: gdnative::init::InitHandle) {
    handle.add_class::<ILikeBoilerplateCode>();
}

godot_gdnative_init!();
godot_nativescript_init!(init);
godot_gdnative_terminate!();

Implementation options

As mentioned before, there are two crates that can be used. They have slightly different pros/cons from each other, that are given here. In any case, this will be an implementation detail, and we should be able to swap implementations freely at a later time.

Both crates need some support with platform compatibility before they can be used in godot-rust.

inventory

Pros:

  • Supports Android and iOS out of the box.

Cons:

linkme

Pros:

  • Works at link time. No life-before-main or runtime cost.

Cons:

  • No official support for both Android and iOS (no issues in repo). Implementation difficulty unknown.

Considerations

Compatibility

It's possible to maintain compatibility with current manually registering code by making InitHandle track registered types, and ignore types that are already registered.

Tool scripts

A #[gdnative::tool] attribute may be added for tool scripts, alongside gdnative::register.

@ghost ghost added feature Adds functionality to the library c: export Component: export (mod export, derive) labels May 20, 2020
@ghost ghost added this to the 0.9 milestone May 22, 2020
@ghost ghost added the help wanted label May 24, 2020
@ghost
Copy link
Author

ghost commented May 24, 2020

Added the "help wanted" tag for ctor / linkme platform support.

@ghost ghost modified the milestones: 0.9, 0.10 Jun 13, 2020
@ghost
Copy link
Author

ghost commented Aug 13, 2020

Update: iOS is now supported by inventory.

@Bromeon Bromeon modified the milestones: v0.10, v0.11 Aug 25, 2021
@Bauxitedev
Copy link

Bauxitedev commented Sep 24, 2021

In my game that uses godot-rust I've tried using the inventory crate, and it seems to work pretty well.
This is what I came up with so far...

For every NativeClass I only need this bit of code to register it... (which could easily be turned into a derive macro to reduce boilerplate)

inventory::submit! {
    AutoRegisterInfo::new("MyClass", 0, |handle| {
        handle.add_class::<MyClass>();
    })
}

Using a simple struct AutoRegisterInfo, that holds a closure for automatic class registration, and a priority for ordering purposes (the order of registrations seem to matter sometimes? I've had it crash on me before when the order was wrong)...

pub struct AutoRegisterInfo {
    pub name: String,  //Only used for debugging
    pub priority: i32, //Use this to put registrations in the right order (higher = first)
    pub register: fn(InitHandle),
}

impl AutoRegisterInfo {
    pub fn new(name: &str, priority: i32, register: fn(InitHandle)) -> Self {
        Self {
            name: name.to_owned(),
            priority,
            register,
        }
    }
}
inventory::collect!(AutoRegisterInfo);

Then, in lib.rs, I loop over all registrations and call them, ordered by priority...

godot_init!(init);

fn init(handle: InitHandle) {
    for info in inventory::iter::<AutoRegisterInfo>
        .into_iter()
        .sorted_by_key(|info| -info.priority)
    {
        println!("Registering class {}", info.name);
        (info.register)(handle);
    }

}

By the way, as a con for inventory, you mention "inventory currently uses ctor under the hood, which is life-before-main." I wonder, what does "life-before-main" mean exactly? And why is it a bad thing?

@Bromeon
Copy link
Member

Bromeon commented May 10, 2022

Both linkme and inventory have been archived, leaving room for speculation:

Update: unarchived, since the underlying rustc bug rust-lang/rust#95604 was resolved.

@Bogay
Copy link
Contributor

Bogay commented Aug 22, 2022

Hello, does this improvement also applies to godot_test! / godot_itest!?
They also need to be declared and invoked manually.

@Bromeon
Copy link
Member

Bromeon commented Aug 22, 2022

I'm planning to do something like that for GDExtension. Not sure about GDNative -- the added value at this point is relatively little, as so many tests already exist, and this would burden the library with more dependencies.

@chitoyuu chitoyuu modified the milestones: unplanned, v0.11.1 Dec 7, 2022
bors bot added a commit that referenced this issue Jan 4, 2023
999: Implement automatic `NativeClass` registration via inventory. Implement mix-ins. r=chitoyuu a=chitoyuu

## Implement automatic NativeClass registration via inventory

This adds the optional `inventory` feature, which allows `NativeClass` types to be automatically registered on supported platforms (everything that OSS Godot currently supports except WASM).

Run-time diagnostic functions are added to help debug missing registration problems that are highly likely to arise when porting `inventory`-enabled projects to WASM.

An internal `cfg_ex` attribute macro is added to help manage cfg conditions.

Close #350. Note that the standalone registration attribute syntax proposed by the original issue isn't implemented, for the limited usefulness -- there are much fewer cases where manual `NativeClass` impls are necessary thanks to all the improvements since the original issue.

## Implement mix-in `#[methods]` blocks

Adds the `#[methods(mixin = "Name")]` syntax for declaring mix-in blocks. Mix-in blocks have a many-to-many relationship with `NativeClass` types. Both `impl Type` and `impl Trait for Type` blocks are accepted.

The argument name is changed from `as` in the original proposal to `mixin`, because we might still want to keep universal `#[methods]` blocks in the future for ease of use with WASM. `#[methods(mixin)]` makes a lot more sense for a future auto-registered mixin block than `#[methods(as /* as what? */)]`.

All mix-in blocks have to be manually registered for gdnative v0.11.x. Some difficulty was encountered when trying to make auto-mixins compatible with existing code. It might still be possible with some tricks like autoref-specialization, but that might be too much effort given that we likely want to re-design much of the hierarchy for 0.12.

Close #984.

## Allow `#[register_with]` for `#[monomorphize]`

Enables `#[monomorphize]` to take the same standalone `#[register_with]` attribute as `#[derive(NativeClass)]`. This is chosen for short term consistency, but will probably change in a later version w/ #848, which might not still get implemented for a fair bit of time.


Co-authored-by: Chitose Yuuzaki <chitoyuu@potatoes.gay>
bors bot added a commit that referenced this issue Jan 4, 2023
999: Implement automatic `NativeClass` registration via inventory. Implement mix-ins. r=chitoyuu a=chitoyuu

## Implement automatic NativeClass registration via inventory

This adds the optional `inventory` feature, which allows `NativeClass` types to be automatically registered on supported platforms (everything that OSS Godot currently supports except WASM).

Run-time diagnostic functions are added to help debug missing registration problems that are highly likely to arise when porting `inventory`-enabled projects to WASM.

An internal `cfg_ex` attribute macro is added to help manage cfg conditions.

Close #350. Note that the standalone registration attribute syntax proposed by the original issue isn't implemented, for the limited usefulness -- there are much fewer cases where manual `NativeClass` impls are necessary thanks to all the improvements since the original issue.

## Implement mix-in `#[methods]` blocks

Adds the `#[methods(mixin = "Name")]` syntax for declaring mix-in blocks. Mix-in blocks have a many-to-many relationship with `NativeClass` types. Both `impl Type` and `impl Trait for Type` blocks are accepted.

The argument name is changed from `as` in the original proposal to `mixin`, because we might still want to keep universal `#[methods]` blocks in the future for ease of use with WASM. `#[methods(mixin)]` makes a lot more sense for a future auto-registered mixin block than `#[methods(as /* as what? */)]`.

All mix-in blocks have to be manually registered for gdnative v0.11.x. Some difficulty was encountered when trying to make auto-mixins compatible with existing code. It might still be possible with some tricks like autoref-specialization, but that might be too much effort given that we likely want to re-design much of the hierarchy for 0.12.

Close #984.

## Allow `#[register_with]` for `#[monomorphize]`

Enables `#[monomorphize]` to take the same standalone `#[register_with]` attribute as `#[derive(NativeClass)]`. This is chosen for short term consistency, but will probably change in a later version w/ #848, which might not still get implemented for a fair bit of time.


Co-authored-by: Chitose Yuuzaki <chitoyuu@potatoes.gay>
@bors bors bot closed this as completed in 16e5847 Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: export Component: export (mod export, derive) feature Adds functionality to the library help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants