Skip to content

Commit ccb9d05

Browse files
MrGVSVjames7132soqbcart
authored
bevy_reflect: Recursive registration (#5781)
# 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 <[email protected]> Co-authored-by: radiish <[email protected]> Co-authored-by: Carter Anderson <[email protected]>
1 parent d90cb57 commit ccb9d05

File tree

12 files changed

+497
-121
lines changed

12 files changed

+497
-121
lines changed

crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,12 @@ impl<'a> ReflectMeta<'a> {
470470
&self,
471471
where_clause_options: &WhereClauseOptions,
472472
) -> proc_macro2::TokenStream {
473-
crate::registration::impl_get_type_registration(self, where_clause_options, None)
473+
crate::registration::impl_get_type_registration(
474+
self,
475+
where_clause_options,
476+
None,
477+
Option::<std::iter::Empty<&Type>>::None,
478+
)
474479
}
475480

476481
/// The collection of docstrings for this type, if any.
@@ -502,6 +507,7 @@ impl<'a> ReflectStruct<'a> {
502507
self.meta(),
503508
where_clause_options,
504509
self.serialization_data(),
510+
Some(self.active_types().iter()),
505511
)
506512
}
507513

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

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

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

529535
/// The complete set of fields in this struct.
530-
#[allow(dead_code)]
531536
pub fn fields(&self) -> &[StructField<'a>] {
532537
&self.fields
533538
}
@@ -573,6 +578,21 @@ impl<'a> ReflectEnum<'a> {
573578
pub fn where_clause_options(&self) -> WhereClauseOptions {
574579
WhereClauseOptions::new_with_fields(self.meta(), self.active_types().into_boxed_slice())
575580
}
581+
582+
/// Returns the `GetTypeRegistration` impl as a `TokenStream`.
583+
///
584+
/// Returns a specific implementation for enums and this method should be preferred over the generic [`get_type_registration`](crate::ReflectMeta) method
585+
pub fn get_type_registration(
586+
&self,
587+
where_clause_options: &WhereClauseOptions,
588+
) -> proc_macro2::TokenStream {
589+
crate::registration::impl_get_type_registration(
590+
self.meta(),
591+
where_clause_options,
592+
None,
593+
Some(self.active_fields().map(|field| &field.data.ty)),
594+
)
595+
}
576596
}
577597

578598
impl<'a> EnumVariant<'a> {

crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,7 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> proc_macro2::TokenStream
8484

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

87-
let get_type_registration_impl = reflect_enum
88-
.meta()
89-
.get_type_registration(&where_clause_options);
87+
let get_type_registration_impl = reflect_enum.get_type_registration(&where_clause_options);
9088

9189
let (impl_generics, ty_generics, where_clause) =
9290
reflect_enum.meta().type_path().generics().split_for_impl();

crates/bevy_reflect/bevy_reflect_derive/src/registration.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,29 @@ use crate::derive_data::ReflectMeta;
44
use crate::serialization::SerializationDataDef;
55
use crate::utility::WhereClauseOptions;
66
use quote::quote;
7+
use syn::Type;
78

89
/// Creates the `GetTypeRegistration` impl for the given type data.
910
#[allow(clippy::too_many_arguments)]
10-
pub(crate) fn impl_get_type_registration(
11+
pub(crate) fn impl_get_type_registration<'a>(
1112
meta: &ReflectMeta,
1213
where_clause_options: &WhereClauseOptions,
1314
serialization_data: Option<&SerializationDataDef>,
15+
type_dependencies: Option<impl Iterator<Item = &'a Type>>,
1416
) -> proc_macro2::TokenStream {
1517
let type_path = meta.type_path();
1618
let bevy_reflect_path = meta.bevy_reflect_path();
1719
let registration_data = meta.attrs().idents();
20+
21+
let type_deps_fn = type_dependencies.map(|deps| {
22+
quote! {
23+
#[inline(never)]
24+
fn register_type_dependencies(registry: &mut #bevy_reflect_path::TypeRegistry) {
25+
#(<#deps as #bevy_reflect_path::__macro_exports::RegisterForReflection>::__register(registry);)*
26+
}
27+
}
28+
});
29+
1830
let (impl_generics, ty_generics, where_clause) = type_path.generics().split_for_impl();
1931
let where_reflect_clause = where_clause_options.extend_where_clause(where_clause);
2032

@@ -44,6 +56,8 @@ pub(crate) fn impl_get_type_registration(
4456
#(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::<Self>::from_type());)*
4557
registration
4658
}
59+
60+
#type_deps_fn
4761
}
4862
}
4963
}

crates/bevy_reflect/bevy_reflect_derive/src/utility.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,15 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> {
217217

218218
// `TypePath` is always required for active fields since they are used to
219219
// construct `NamedField` and `UnnamedField` instances for the `Typed` impl.
220-
Some(
221-
self.active_fields
222-
.iter()
223-
.map(move |ty| quote!(#ty : #reflect_bound + #bevy_reflect_path::TypePath)),
224-
)
220+
// Likewise, `GetTypeRegistration` is always required for active fields since
221+
// they are used to register the type's dependencies.
222+
Some(self.active_fields.iter().map(move |ty| {
223+
quote!(
224+
#ty : #reflect_bound
225+
+ #bevy_reflect_path::TypePath
226+
+ #bevy_reflect_path::__macro_exports::RegisterForReflection
227+
)
228+
}))
225229
}
226230
}
227231

0 commit comments

Comments
 (0)