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

Implement automatic NativeClass registration via inventory. Implement mix-ins. #999

Merged
merged 7 commits into from
Jan 4, 2023

Conversation

chitoyuu
Copy link
Contributor

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.

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.
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.

All mix-in blocks have to be manually registered for gdnative v0.11.x.
This can be relaxed in the future with a redesign of the `NativeClass`
hierarchy of traits.
@chitoyuu chitoyuu added feature Adds functionality to the library c: export Component: export (mod export, derive) labels Dec 10, 2022
@chitoyuu chitoyuu added this to the v0.11.1 milestone Dec 10, 2022
@chitoyuu
Copy link
Contributor Author

Some minor unresolved questions:

  • Should the inventory feature become default?
  • As an extension to the former, should we modify our examples to use the inventory feature, with the trade off that they'll no longer be working on WASM?

Ideas for future development:

  • Run-time diagnostic that catches mixins that are declared but never registered (in inventory-enabled builds)

bors try

bors bot added a commit that referenced this pull request Dec 10, 2022
@chitoyuu chitoyuu marked this pull request as draft December 10, 2022 10:57
@chitoyuu
Copy link
Contributor Author

Converting to draft: forgot to update documentation for the macros. Will do this later.

@bors
Copy link
Contributor

bors bot commented Dec 10, 2022

try

Build succeeded:

@chitoyuu chitoyuu marked this pull request as ready for review January 4, 2023 16:34
@chitoyuu
Copy link
Contributor Author

chitoyuu commented Jan 4, 2023

Left this hanging for a while but it should be good to go. For v0.11 I think we'll keep the feature opt-in and the examples as-is.

bors r+

bors bot added a commit that referenced this pull request 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
Copy link
Contributor

bors bot commented Jan 4, 2023

Build failed:

@chitoyuu
Copy link
Contributor Author

chitoyuu commented Jan 4, 2023

bors retry

bors bot added a commit that referenced this pull request 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
Copy link
Contributor

bors bot commented Jan 4, 2023

Build failed:

I don't understand why it's $DIR for some of the tests and tests/ui for
the others. Perhaps this is something upstream should have considered?
@chitoyuu
Copy link
Contributor Author

chitoyuu commented Jan 4, 2023

This should finally do it...

bors retry

@bors
Copy link
Contributor

bors bot commented Jan 4, 2023

Build succeeded:

@bors bors bot merged commit 16e5847 into godot-rust:master Jan 4, 2023
@chitoyuu chitoyuu deleted the feature/inventory branch January 4, 2023 18:33
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
Projects
None yet
1 participant