Skip to content

Commit

Permalink
bevy_reflect: Recursive registration (#5781)
Browse files Browse the repository at this point in the history
# Objective

Resolves #4154

Currently, registration must all be done manually:

```rust
#[derive(Reflect)]
struct Foo(Bar);

#[derive(Reflect)]
struct Bar(Baz);

#[derive(Reflect)]
struct Baz(usize);

fn main() {
  // ...
  app
    .register_type::<Foo>()
    .register_type::<Bar>()
    .register_type::<Baz>()
    // .register_type::<usize>() <- This one is handled by Bevy, thankfully
  // ...
}
```

This can grow really quickly and become very annoying to add, remove,
and update as types change. It would be great if we could help reduce
the number of types that a user must manually implement themselves.

## Solution

As suggested in #4154, this PR adds automatic recursive registration.
Essentially, when a type is registered, it may now also choose to
register additional types along with it using the new
`GetTypeRegistration::register_type_dependencies` trait method.

The `Reflect` derive macro now automatically does this for all fields in
structs, tuple structs, struct variants, and tuple variants. This is
also done for tuples, arrays, `Vec<T>`, `HashMap<K, V>`, and
`Option<T>`.

This allows us to simplify the code above like:

```rust
#[derive(Reflect)]
struct Foo(Bar);

#[derive(Reflect)]
struct Bar(Baz);

#[derive(Reflect)]
struct Baz(usize);

fn main() {
  // ...
  app.register_type::<Foo>()
  // ...
}
```

This automatic registration only occurs if the type has not yet been
registered. If it has been registered, we simply skip it and move to the
next one. This reduces the cost of registration and prevents overwriting
customized registrations.

## Considerations

While this does improve ergonomics on one front, it's important to look
at some of the arguments against adopting a PR like this.

#### Generic Bounds

~~Since we need to be able to register the fields individually, we need
those fields to implement `GetTypeRegistration`. This forces users to
then add this trait as a bound on their generic arguments. This
annoyance could be relieved with something like #5772.~~

This is no longer a major issue as the `Reflect` derive now adds the
`GetTypeRegistration` bound by default. This should technically be okay,
since we already add the `Reflect` bound.

However, this can also be considered a breaking change for manual
implementations that left out a `GetTypeRegistration` impl ~~or for
items that contain dynamic types (e.g. `DynamicStruct`) since those also
do not implement `GetTypeRegistration`~~.

#### Registration Assumptions

By automatically registering fields, users might inadvertently be
relying on certain types to be automatically registered. If `Foo`
auto-registers `Bar`, but `Foo` is later removed from the code, then
anywhere that previously used or relied on `Bar`'s registration would
now fail.

---

## Changelog

- Added recursive type registration to structs, tuple structs, struct
variants, tuple variants, tuples, arrays, `Vec<T>`, `HashMap<K, V>`, and
`Option<T>`
- Added a new trait in the hidden `bevy_reflect::__macro_exports` module
called `RegisterForReflection`
- Added `GetTypeRegistration` impl for
`bevy_render::render_asset::RenderAssetUsages`

## Migration Guide

All types that derive `Reflect` will now automatically add
`GetTypeRegistration` as a bound on all (unignored) fields. This means
that all reflected fields will need to also implement
`GetTypeRegistration`.

If all fields **derive** `Reflect` or are implemented in `bevy_reflect`,
this should not cause any issues. However, manual implementations of
`Reflect` that excluded a `GetTypeRegistration` impl for their type will
need to add one.

```rust
#[derive(Reflect)]
struct Foo<T: FromReflect> {
  data: MyCustomType<T>
}

// OLD
impl<T: FromReflect> Reflect for MyCustomType<T> {/* ... */}

// NEW
impl<T: FromReflect + GetTypeRegistration> Reflect for MyCustomType<T> {/* ... */}
impl<T: FromReflect + GetTypeRegistration> GetTypeRegistration for MyCustomType<T> {/* ... */}
```

---------

Co-authored-by: James Liu <contact@jamessliu.com>
Co-authored-by: radiish <cb.setho@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
  • Loading branch information
4 people committed Mar 4, 2024
1 parent d90cb57 commit ccb9d05
Show file tree
Hide file tree
Showing 12 changed files with 497 additions and 121 deletions.
30 changes: 25 additions & 5 deletions crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,12 @@ impl<'a> ReflectMeta<'a> {
&self,
where_clause_options: &WhereClauseOptions,
) -> proc_macro2::TokenStream {
crate::registration::impl_get_type_registration(self, where_clause_options, None)
crate::registration::impl_get_type_registration(
self,
where_clause_options,
None,
Option::<std::iter::Empty<&Type>>::None,
)
}

/// The collection of docstrings for this type, if any.
Expand Down Expand Up @@ -502,6 +507,7 @@ impl<'a> ReflectStruct<'a> {
self.meta(),
where_clause_options,
self.serialization_data(),
Some(self.active_types().iter()),
)
}

Expand All @@ -512,22 +518,21 @@ impl<'a> ReflectStruct<'a> {
.collect()
}

/// Get an iterator of fields which are exposed to the reflection API
/// Get an iterator of fields which are exposed to the reflection API.
pub fn active_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields
self.fields()
.iter()
.filter(|field| field.attrs.ignore.is_active())
}

/// Get an iterator of fields which are ignored by the reflection API
pub fn ignored_fields(&self) -> impl Iterator<Item = &StructField<'a>> {
self.fields
self.fields()
.iter()
.filter(|field| field.attrs.ignore.is_ignored())
}

/// The complete set of fields in this struct.
#[allow(dead_code)]
pub fn fields(&self) -> &[StructField<'a>] {
&self.fields
}
Expand Down Expand Up @@ -573,6 +578,21 @@ impl<'a> ReflectEnum<'a> {
pub fn where_clause_options(&self) -> WhereClauseOptions {
WhereClauseOptions::new_with_fields(self.meta(), self.active_types().into_boxed_slice())
}

/// Returns the `GetTypeRegistration` impl as a `TokenStream`.
///
/// Returns a specific implementation for enums and this method should be preferred over the generic [`get_type_registration`](crate::ReflectMeta) method
pub fn get_type_registration(
&self,
where_clause_options: &WhereClauseOptions,
) -> proc_macro2::TokenStream {
crate::registration::impl_get_type_registration(
self.meta(),
where_clause_options,
None,
Some(self.active_fields().map(|field| &field.data.ty)),
)
}
}

impl<'a> EnumVariant<'a> {
Expand Down
4 changes: 1 addition & 3 deletions crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream

let type_path_impl = impl_type_path(reflect_enum.meta());

let get_type_registration_impl = reflect_enum
.meta()
.get_type_registration(&where_clause_options);
let get_type_registration_impl = reflect_enum.get_type_registration(&where_clause_options);

let (impl_generics, ty_generics, where_clause) =
reflect_enum.meta().type_path().generics().split_for_impl();
Expand Down
16 changes: 15 additions & 1 deletion crates/bevy_reflect/bevy_reflect_derive/src/registration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,29 @@ use crate::derive_data::ReflectMeta;
use crate::serialization::SerializationDataDef;
use crate::utility::WhereClauseOptions;
use quote::quote;
use syn::Type;

/// Creates the `GetTypeRegistration` impl for the given type data.
#[allow(clippy::too_many_arguments)]
pub(crate) fn impl_get_type_registration(
pub(crate) fn impl_get_type_registration<'a>(
meta: &ReflectMeta,
where_clause_options: &WhereClauseOptions,
serialization_data: Option<&SerializationDataDef>,
type_dependencies: Option<impl Iterator<Item = &'a Type>>,
) -> proc_macro2::TokenStream {
let type_path = meta.type_path();
let bevy_reflect_path = meta.bevy_reflect_path();
let registration_data = meta.attrs().idents();

let type_deps_fn = type_dependencies.map(|deps| {
quote! {
#[inline(never)]
fn register_type_dependencies(registry: &mut #bevy_reflect_path::TypeRegistry) {
#(<#deps as #bevy_reflect_path::__macro_exports::RegisterForReflection>::__register(registry);)*
}
}
});

let (impl_generics, ty_generics, where_clause) = type_path.generics().split_for_impl();
let where_reflect_clause = where_clause_options.extend_where_clause(where_clause);

Expand Down Expand Up @@ -44,6 +56,8 @@ pub(crate) fn impl_get_type_registration(
#(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<Self>::from_type());)*
registration
}

#type_deps_fn
}
}
}
14 changes: 9 additions & 5 deletions crates/bevy_reflect/bevy_reflect_derive/src/utility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,15 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> {

// `TypePath` is always required for active fields since they are used to
// construct `NamedField` and `UnnamedField` instances for the `Typed` impl.
Some(
self.active_fields
.iter()
.map(move |ty| quote!(#ty : #reflect_bound + #bevy_reflect_path::TypePath)),
)
// Likewise, `GetTypeRegistration` is always required for active fields since
// they are used to register the type's dependencies.
Some(self.active_fields.iter().map(move |ty| {
quote!(
#ty : #reflect_bound
+ #bevy_reflect_path::TypePath
+ #bevy_reflect_path::__macro_exports::RegisterForReflection
)
}))
}
}

Expand Down
Loading

0 comments on commit ccb9d05

Please sign in to comment.