From ccb9d0500f88fe93bf7c43647fe6a8efc5d82030 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Mon, 4 Mar 2024 12:04:10 -0700 Subject: [PATCH] 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::() .register_type::() .register_type::() // .register_type::() <- 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`, `HashMap`, and `Option`. 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::() // ... } ``` 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`, `HashMap`, and `Option` - 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 { data: MyCustomType } // OLD impl Reflect for MyCustomType {/* ... */} // NEW impl Reflect for MyCustomType {/* ... */} impl GetTypeRegistration for MyCustomType {/* ... */} ``` --------- Co-authored-by: James Liu Co-authored-by: radiish Co-authored-by: Carter Anderson --- .../bevy_reflect_derive/src/derive_data.rs | 30 ++- .../bevy_reflect_derive/src/impls/enums.rs | 4 +- .../bevy_reflect_derive/src/registration.rs | 16 +- .../bevy_reflect_derive/src/utility.rs | 14 +- crates/bevy_reflect/src/impls/std.rs | 127 ++++++------ crates/bevy_reflect/src/lib.rs | 195 +++++++++++++++++- crates/bevy_reflect/src/tuple.rs | 19 +- crates/bevy_reflect/src/type_registry.rs | 144 +++++++++++-- .../tests/reflect_derive/generics.fail.rs | 4 +- .../tests/reflect_derive/generics.fail.stderr | 43 ++-- crates/bevy_render/src/render_asset.rs | 18 +- examples/reflection/generic_reflection.rs | 4 +- 12 files changed, 497 insertions(+), 121 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs index 869ca861328c3..48625ec3ca283 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -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::>::None, + ) } /// The collection of docstrings for this type, if any. @@ -502,6 +507,7 @@ impl<'a> ReflectStruct<'a> { self.meta(), where_clause_options, self.serialization_data(), + Some(self.active_types().iter()), ) } @@ -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> { - 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> { - 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 } @@ -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> { diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs index 65aea76b9627d..056a293ec91be 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -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(); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 04189073a3886..d277d823c59f9 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -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>, ) -> 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); @@ -44,6 +56,8 @@ pub(crate) fn impl_get_type_registration( #(registration.insert::<#registration_data>(#bevy_reflect_path::FromType::::from_type());)* registration } + + #type_deps_fn } } } diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 5e619cf1ade45..a32fcabeea640 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -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 + ) + })) } } diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 07500814d0263..578879091bfcc 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -1,5 +1,5 @@ use crate::std_traits::ReflectDefault; -use crate::{self as bevy_reflect, ReflectFromPtr, ReflectFromReflect, ReflectOwned}; +use crate::{self as bevy_reflect, ReflectFromPtr, ReflectFromReflect, ReflectOwned, TypeRegistry}; use crate::{ impl_type_path, map_apply, map_partial_eq, Array, ArrayInfo, ArrayIter, DynamicEnum, DynamicMap, Enum, EnumInfo, FromReflect, FromType, GetTypeRegistration, List, ListInfo, @@ -221,7 +221,7 @@ impl_reflect_value!(::std::ffi::OsString(Debug, Hash, PartialEq)); macro_rules! impl_reflect_for_veclike { ($ty:path, $insert:expr, $remove:expr, $push:expr, $pop:expr, $sub:ty) => { - impl List for $ty { + impl List for $ty { #[inline] fn get(&self, index: usize) -> Option<&dyn Reflect> { <$sub>::get(self, index).map(|value| value as &dyn Reflect) @@ -280,7 +280,7 @@ macro_rules! impl_reflect_for_veclike { } } - impl Reflect for $ty { + impl Reflect for $ty { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) } @@ -347,7 +347,7 @@ macro_rules! impl_reflect_for_veclike { } } - impl Typed for $ty { + impl Typed for $ty { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); CELL.get_or_insert::(|| TypeInfo::List(ListInfo::new::())) @@ -356,15 +356,19 @@ macro_rules! impl_reflect_for_veclike { impl_type_path!($ty); - impl GetTypeRegistration for $ty { + impl GetTypeRegistration for $ty { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::<$ty>(); registration.insert::(FromType::<$ty>::from_type()); registration } + + fn register_type_dependencies(registry: &mut TypeRegistry) { + registry.register::(); + } } - impl FromReflect for $ty { + impl FromReflect for $ty { fn from_reflect(reflect: &dyn Reflect) -> Option { if let ReflectRef::List(ref_list) = reflect.reflect_ref() { let mut new_list = Self::with_capacity(ref_list.len()); @@ -401,8 +405,8 @@ macro_rules! impl_reflect_for_hashmap { ($ty:path) => { impl Map for $ty where - K: FromReflect + TypePath + Eq + Hash, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, + V: FromReflect + TypePath + GetTypeRegistration, S: TypePath + BuildHasher + Send + Sync, { fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> { @@ -498,8 +502,8 @@ macro_rules! impl_reflect_for_hashmap { impl Reflect for $ty where - K: FromReflect + TypePath + Eq + Hash, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, + V: FromReflect + TypePath + GetTypeRegistration, S: TypePath + BuildHasher + Send + Sync, { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { @@ -567,8 +571,8 @@ macro_rules! impl_reflect_for_hashmap { impl Typed for $ty where - K: FromReflect + TypePath + Eq + Hash, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, + V: FromReflect + TypePath + GetTypeRegistration, S: TypePath + BuildHasher + Send + Sync, { fn type_info() -> &'static TypeInfo { @@ -579,8 +583,8 @@ macro_rules! impl_reflect_for_hashmap { impl GetTypeRegistration for $ty where - K: FromReflect + TypePath + Eq + Hash, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, + V: FromReflect + TypePath + GetTypeRegistration, S: TypePath + BuildHasher + Send + Sync, { fn get_type_registration() -> TypeRegistration { @@ -588,12 +592,17 @@ macro_rules! impl_reflect_for_hashmap { registration.insert::(FromType::::from_type()); registration } + + fn register_type_dependencies(registry: &mut TypeRegistry) { + registry.register::(); + registry.register::(); + } } impl FromReflect for $ty where - K: FromReflect + TypePath + Eq + Hash, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Hash, + V: FromReflect + TypePath + GetTypeRegistration, S: TypePath + BuildHasher + Default + Send + Sync, { fn from_reflect(reflect: &dyn Reflect) -> Option { @@ -624,8 +633,8 @@ impl_type_path!(::bevy_utils::hashbrown::HashMap); impl Map for ::std::collections::BTreeMap where - K: FromReflect + TypePath + Eq + Ord, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Ord, + V: FromReflect + TypePath + GetTypeRegistration, { fn get(&self, key: &dyn Reflect) -> Option<&dyn Reflect> { key.downcast_ref::() @@ -720,8 +729,8 @@ where impl Reflect for ::std::collections::BTreeMap where - K: FromReflect + TypePath + Eq + Ord, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Ord, + V: FromReflect + TypePath + GetTypeRegistration, { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) @@ -788,8 +797,8 @@ where impl Typed for ::std::collections::BTreeMap where - K: FromReflect + TypePath + Eq + Ord, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Ord, + V: FromReflect + TypePath + GetTypeRegistration, { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); @@ -799,8 +808,8 @@ where impl GetTypeRegistration for ::std::collections::BTreeMap where - K: FromReflect + TypePath + Eq + Ord, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Ord, + V: FromReflect + TypePath + GetTypeRegistration, { fn get_type_registration() -> TypeRegistration { let mut registration = TypeRegistration::of::(); @@ -811,8 +820,8 @@ where impl FromReflect for ::std::collections::BTreeMap where - K: FromReflect + TypePath + Eq + Ord, - V: FromReflect + TypePath, + K: FromReflect + TypePath + GetTypeRegistration + Eq + Ord, + V: FromReflect + TypePath + GetTypeRegistration, { fn from_reflect(reflect: &dyn Reflect) -> Option { if let ReflectRef::Map(ref_map) = reflect.reflect_ref() { @@ -831,7 +840,7 @@ where impl_type_path!(::std::collections::BTreeMap); -impl Array for [T; N] { +impl Array for [T; N] { #[inline] fn get(&self, index: usize) -> Option<&dyn Reflect> { <[T]>::get(self, index).map(|value| value as &dyn Reflect) @@ -860,7 +869,7 @@ impl Array for [T; N] { } } -impl Reflect for [T; N] { +impl Reflect for [T; N] { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) } @@ -942,7 +951,7 @@ impl Reflect for [T; N] { } } -impl FromReflect for [T; N] { +impl FromReflect for [T; N] { fn from_reflect(reflect: &dyn Reflect) -> Option { if let ReflectRef::Array(ref_array) = reflect.reflect_ref() { let mut temp_vec = Vec::with_capacity(ref_array.len()); @@ -956,7 +965,7 @@ impl FromReflect for [T; N] { } } -impl Typed for [T; N] { +impl Typed for [T; N] { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); CELL.get_or_insert::(|| TypeInfo::Array(ArrayInfo::new::(N))) @@ -975,37 +984,27 @@ impl TypePath for [T; N] { } } -// TODO: -// `FromType::from_type` requires `Deserialize<'de>` to be implemented for `T`. -// Currently serde only supports `Deserialize<'de>` for arrays up to size 32. -// This can be changed to use const generics once serde utilizes const generics for arrays. -// Tracking issue: https://github.com/serde-rs/serde/issues/1937 -macro_rules! impl_array_get_type_registration { - ($($N:expr)+) => { - $( - impl GetTypeRegistration for [T; $N] { - fn get_type_registration() -> TypeRegistration { - TypeRegistration::of::<[T; $N]>() - } - } - )+ - }; -} +impl GetTypeRegistration for [T; N] { + fn get_type_registration() -> TypeRegistration { + TypeRegistration::of::<[T; N]>() + } -impl_array_get_type_registration! { - 0 1 2 3 4 5 6 7 8 9 - 10 11 12 13 14 15 16 17 18 19 - 20 21 22 23 24 25 26 27 28 29 - 30 31 32 + fn register_type_dependencies(registry: &mut TypeRegistry) { + registry.register::(); + } } -impl GetTypeRegistration for Option { +impl GetTypeRegistration for Option { fn get_type_registration() -> TypeRegistration { TypeRegistration::of::>() } + + fn register_type_dependencies(registry: &mut TypeRegistry) { + registry.register::(); + } } -impl Enum for Option { +impl Enum for Option { fn field(&self, _name: &str) -> Option<&dyn Reflect> { None } @@ -1076,7 +1075,7 @@ impl Enum for Option { } } -impl Reflect for Option { +impl Reflect for Option { #[inline] fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) @@ -1189,7 +1188,7 @@ impl Reflect for Option { } } -impl FromReflect for Option { +impl FromReflect for Option { fn from_reflect(reflect: &dyn Reflect) -> Option { if let ReflectRef::Enum(dyn_enum) = reflect.reflect_ref() { match dyn_enum.variant_name() { @@ -1227,7 +1226,7 @@ impl FromReflect for Option { } } -impl Typed for Option { +impl Typed for Option { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); CELL.get_or_insert::(|| { @@ -1392,7 +1391,7 @@ where } } -impl List for Cow<'static, [T]> { +impl List for Cow<'static, [T]> { fn get(&self, index: usize) -> Option<&dyn Reflect> { self.as_ref().get(index).map(|x| x as &dyn Reflect) } @@ -1451,7 +1450,7 @@ impl List for Cow<'static, [T]> { } } -impl Reflect for Cow<'static, [T]> { +impl Reflect for Cow<'static, [T]> { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) } @@ -1518,20 +1517,26 @@ impl Reflect for Cow<'static, [T]> { } } -impl Typed for Cow<'static, [T]> { +impl Typed for Cow<'static, [T]> { fn type_info() -> &'static TypeInfo { static CELL: GenericTypeInfoCell = GenericTypeInfoCell::new(); CELL.get_or_insert::(|| TypeInfo::List(ListInfo::new::())) } } -impl GetTypeRegistration for Cow<'static, [T]> { +impl GetTypeRegistration + for Cow<'static, [T]> +{ fn get_type_registration() -> TypeRegistration { TypeRegistration::of::>() } + + fn register_type_dependencies(registry: &mut TypeRegistry) { + registry.register::(); + } } -impl FromReflect for Cow<'static, [T]> { +impl FromReflect for Cow<'static, [T]> { fn from_reflect(reflect: &dyn Reflect) -> Option { if let ReflectRef::List(ref_list) = reflect.reflect_ref() { let mut temp_vec = Vec::with_capacity(ref_list.len()); diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index ab7b2d0a0adb1..52eae900d6d67 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -481,9 +481,11 @@ mod tuple_struct; mod type_info; mod type_path; mod type_registry; + mod impls { #[cfg(feature = "glam")] mod glam; + #[cfg(feature = "bevy_math")] mod math { mod direction; @@ -491,6 +493,7 @@ mod impls { mod primitives3d; mod rect; } + #[cfg(feature = "smallvec")] mod smallvec; #[cfg(feature = "smol_str")] @@ -535,9 +538,48 @@ pub use erased_serde; extern crate alloc; +/// Exports used by the reflection macros. +/// +/// These are not meant to be used directly and are subject to breaking changes. #[doc(hidden)] pub mod __macro_exports { - pub use bevy_utils::uuid::generate_composite_uuid; + use crate::{ + DynamicArray, DynamicEnum, DynamicList, DynamicMap, DynamicStruct, DynamicTuple, + DynamicTupleStruct, GetTypeRegistration, TypeRegistry, + }; + + /// A wrapper trait around [`GetTypeRegistration`]. + /// + /// This trait is used by the derive macro to recursively register all type dependencies. + /// It's used instead of `GetTypeRegistration` directly to avoid making dynamic types also + /// implement `GetTypeRegistration` in order to be used as active fields. + /// + /// This trait has a blanket implementation for all types that implement `GetTypeRegistration` + /// and manual implementations for all dynamic types (which simply do nothing). + pub trait RegisterForReflection { + #[allow(unused_variables)] + fn __register(registry: &mut TypeRegistry) {} + } + + impl RegisterForReflection for T { + fn __register(registry: &mut TypeRegistry) { + registry.register::(); + } + } + + impl RegisterForReflection for DynamicEnum {} + + impl RegisterForReflection for DynamicTupleStruct {} + + impl RegisterForReflection for DynamicStruct {} + + impl RegisterForReflection for DynamicMap {} + + impl RegisterForReflection for DynamicList {} + + impl RegisterForReflection for DynamicArray {} + + impl RegisterForReflection for DynamicTuple {} } #[cfg(test)] @@ -1002,6 +1044,124 @@ mod tests { assert_eq!(new_foo, expected_new_foo); } + #[test] + fn should_auto_register_fields() { + #[derive(Reflect)] + struct Foo { + bar: Bar, + } + + #[derive(Reflect)] + enum Bar { + Variant(Baz), + } + + #[derive(Reflect)] + struct Baz(usize); + + // === Basic === // + let mut registry = TypeRegistry::empty(); + registry.register::(); + + assert!( + registry.contains(TypeId::of::()), + "registry should contain auto-registered `Bar` from `Foo`" + ); + + // === Option === // + let mut registry = TypeRegistry::empty(); + registry.register::>(); + + assert!( + registry.contains(TypeId::of::()), + "registry should contain auto-registered `Bar` from `Option`" + ); + + // === Tuple === // + let mut registry = TypeRegistry::empty(); + registry.register::<(Foo, Foo)>(); + + assert!( + registry.contains(TypeId::of::()), + "registry should contain auto-registered `Bar` from `(Foo, Foo)`" + ); + + // === Array === // + let mut registry = TypeRegistry::empty(); + registry.register::<[Foo; 3]>(); + + assert!( + registry.contains(TypeId::of::()), + "registry should contain auto-registered `Bar` from `[Foo; 3]`" + ); + + // === Vec === // + let mut registry = TypeRegistry::empty(); + registry.register::>(); + + assert!( + registry.contains(TypeId::of::()), + "registry should contain auto-registered `Bar` from `Vec`" + ); + + // === HashMap === // + let mut registry = TypeRegistry::empty(); + registry.register::>(); + + assert!( + registry.contains(TypeId::of::()), + "registry should contain auto-registered `Bar` from `HashMap`" + ); + } + + #[test] + fn should_allow_dynamic_fields() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct MyStruct( + DynamicEnum, + DynamicTupleStruct, + DynamicStruct, + DynamicMap, + DynamicList, + DynamicArray, + DynamicTuple, + i32, + ); + + assert_impl_all!(MyStruct: Reflect, GetTypeRegistration); + + let mut registry = TypeRegistry::empty(); + registry.register::(); + + assert_eq!(2, registry.iter().count()); + assert!(registry.contains(TypeId::of::())); + assert!(registry.contains(TypeId::of::())); + } + + #[test] + fn should_not_auto_register_existing_types() { + #[derive(Reflect)] + struct Foo { + bar: Bar, + } + + #[derive(Reflect, Default)] + struct Bar(usize); + + let mut registry = TypeRegistry::empty(); + registry.register::(); + registry.register_type_data::(); + registry.register::(); + + assert!( + registry + .get_type_data::(TypeId::of::()) + .is_some(), + "registry should contain existing registration for `Bar`" + ); + } + #[test] fn reflect_serialize() { #[derive(Reflect)] @@ -1981,6 +2141,39 @@ bevy_reflect::tests::Test { let _ = ::type_path(); } + #[test] + fn recursive_registration_does_not_hang() { + #[derive(Reflect)] + struct Recurse(T); + + let mut registry = TypeRegistry::empty(); + + registry.register::>>(); + + #[derive(Reflect)] + #[reflect(no_field_bounds)] + struct SelfRecurse { + recurse: Vec, + } + + registry.register::(); + + #[derive(Reflect)] + #[reflect(no_field_bounds)] + enum RecurseA { + Recurse(RecurseB), + } + + #[derive(Reflect)] + struct RecurseB { + vector: Vec, + } + + registry.register::(); + assert!(registry.contains(TypeId::of::())); + assert!(registry.contains(TypeId::of::())); + } + #[test] fn can_opt_out_type_path() { #[derive(Reflect)] diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 78d78e1a42ea2..de6af437df4bb 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -3,8 +3,8 @@ use bevy_utils::all_tuples; use crate::{ self as bevy_reflect, utility::GenericTypePathCell, FromReflect, GetTypeRegistration, Reflect, - ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, TypeRegistration, Typed, - UnnamedField, + ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, TypeRegistration, TypeRegistry, + Typed, UnnamedField, }; use crate::{ReflectKind, TypePathTable}; use std::any::{Any, TypeId}; @@ -461,7 +461,7 @@ pub fn tuple_debug(dyn_tuple: &dyn Tuple, f: &mut Formatter<'_>) -> std::fmt::Re macro_rules! impl_reflect_tuple { {$($index:tt : $name:tt),*} => { - impl<$($name: Reflect + TypePath),*> Tuple for ($($name,)*) { + impl<$($name: Reflect + TypePath + GetTypeRegistration),*> Tuple for ($($name,)*) { #[inline] fn field(&self, index: usize) -> Option<&dyn Reflect> { match index { @@ -512,7 +512,7 @@ macro_rules! impl_reflect_tuple { } } - impl<$($name: Reflect + TypePath),*> Reflect for ($($name,)*) { + impl<$($name: Reflect + TypePath + GetTypeRegistration),*> Reflect for ($($name,)*) { fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { Some(::type_info()) } @@ -575,7 +575,7 @@ macro_rules! impl_reflect_tuple { } } - impl <$($name: Reflect + TypePath),*> Typed for ($($name,)*) { + impl <$($name: Reflect + TypePath + GetTypeRegistration),*> Typed for ($($name,)*) { fn type_info() -> &'static TypeInfo { static CELL: $crate::utility::GenericTypeInfoCell = $crate::utility::GenericTypeInfoCell::new(); CELL.get_or_insert::(|| { @@ -588,14 +588,17 @@ macro_rules! impl_reflect_tuple { } } - - impl<$($name: Reflect + TypePath),*> GetTypeRegistration for ($($name,)*) { + impl<$($name: Reflect + TypePath + GetTypeRegistration),*> GetTypeRegistration for ($($name,)*) { fn get_type_registration() -> TypeRegistration { TypeRegistration::of::<($($name,)*)>() } + + fn register_type_dependencies(_registry: &mut TypeRegistry) { + $(_registry.register::<$name>();)* + } } - impl<$($name: FromReflect + TypePath),*> FromReflect for ($($name,)*) + impl<$($name: FromReflect + TypePath + GetTypeRegistration),*> FromReflect for ($($name,)*) { fn from_reflect(reflect: &dyn Reflect) -> Option { if let ReflectRef::Tuple(_ref_tuple) = reflect.reflect_ref() { diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 8fc39297f3d5a..3fdd296d3eb5e 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -56,8 +56,15 @@ impl Debug for TypeRegistryArc { /// See the [crate-level documentation] for more information on type registration. /// /// [crate-level documentation]: crate -pub trait GetTypeRegistration { +pub trait GetTypeRegistration: 'static { + /// Returns the default [`TypeRegistration`] for this type. fn get_type_registration() -> TypeRegistration; + /// Registers other types needed by this type. + /// + /// This method is called by [`TypeRegistry::register`] to register any other required types. + /// Often, this is done for fields of structs and enum variants to ensure all types are properly registered. + #[allow(unused_variables)] + fn register_type_dependencies(registry: &mut TypeRegistry) {} } impl Default for TypeRegistry { @@ -100,39 +107,132 @@ impl TypeRegistry { registry } - /// Registers the type `T`, adding reflect data as specified in the [`Reflect`] derive: - /// ```ignore (Neither bevy_ecs nor serde "derive" are available.) - /// #[derive(Component, serde::Serialize, serde::Deserialize, Reflect)] - /// #[reflect(Component, Serialize, Deserialize)] // will register ReflectComponent, ReflectSerialize, ReflectDeserialize + /// Attempts to register the type `T` if it has not yet been registered already. + /// + /// This will also recursively register any type dependencies as specified by [`GetTypeRegistration::register_type_dependencies`]. + /// When deriving `Reflect`, this will generally be all the fields of the struct or enum variant. + /// As with any type registration, these type dependencies will not be registered more than once. + /// + /// If the registration for type `T` already exists, it will not be registered again and neither will its type dependencies. + /// To register the type, overwriting any existing registration, use [register](Self::overwrite_registration) instead. + /// + /// Additionally, this will add any reflect [type data](TypeData) as specified in the [`Reflect`] derive. + /// + /// # Example + /// + /// ``` + /// # use std::any::TypeId; + /// # use bevy_reflect::{Reflect, TypeRegistry, std_traits::ReflectDefault}; + /// #[derive(Reflect, Default)] + /// #[reflect(Default)] + /// struct Foo { + /// name: Option, + /// value: i32 + /// } + /// + /// let mut type_registry = TypeRegistry::default(); + /// + /// type_registry.register::(); + /// + /// // The main type + /// assert!(type_registry.contains(TypeId::of::())); + /// + /// // Its type dependencies + /// assert!(type_registry.contains(TypeId::of::>())); + /// assert!(type_registry.contains(TypeId::of::())); + /// + /// // Its type data + /// assert!(type_registry.get_type_data::(TypeId::of::()).is_some()); /// ``` pub fn register(&mut self) where T: GetTypeRegistration, { - self.add_registration(T::get_type_registration()); + if self.register_internal(TypeId::of::(), T::get_type_registration) { + T::register_type_dependencies(self); + } + } + + /// Attempts to register the type described by `registration`. + /// + /// If the registration for the type already exists, it will not be registered again. + /// + /// To forcibly register the type, overwriting any existing registration, use the + /// [`overwrite_registration`](Self::overwrite_registration) method instead. + /// + /// This method will _not_ register type dependencies. + /// Use [`register`](Self::register) to register a type with its dependencies. + /// + /// Returns `true` if the registration was added and `false` if it already exists. + pub fn add_registration(&mut self, registration: TypeRegistration) -> bool { + let type_id = registration.type_id(); + self.register_internal(type_id, || registration) } /// Registers the type described by `registration`. - pub fn add_registration(&mut self, registration: TypeRegistration) { - if self.registrations.contains_key(®istration.type_id()) { - return; + /// + /// If the registration for the type already exists, it will be overwritten. + /// + /// To avoid overwriting existing registrations, it's recommended to use the + /// [`register`](Self::register) or [`add_registration`](Self::add_registration) methods instead. + /// + /// This method will _not_ register type dependencies. + /// Use [`register`](Self::register) to register a type with its dependencies. + pub fn overwrite_registration(&mut self, registration: TypeRegistration) { + Self::update_registration_indices( + ®istration, + &mut self.short_path_to_id, + &mut self.type_path_to_id, + &mut self.ambiguous_names, + ); + self.registrations + .insert(registration.type_id(), registration); + } + + /// Internal method to register a type with a given [`TypeId`] and [`TypeRegistration`]. + /// + /// By using this method, we are able to reduce the number of `TypeId` hashes and lookups needed + /// to register a type. + /// + /// This method is internal to prevent users from accidentally registering a type with a `TypeId` + /// that does not match the type in the `TypeRegistration`. + fn register_internal( + &mut self, + type_id: TypeId, + get_registration: impl FnOnce() -> TypeRegistration, + ) -> bool { + match self.registrations.entry(type_id) { + bevy_utils::hashbrown::hash_map::Entry::Occupied(_) => false, + bevy_utils::hashbrown::hash_map::Entry::Vacant(entry) => { + let registration = get_registration(); + Self::update_registration_indices( + ®istration, + &mut self.short_path_to_id, + &mut self.type_path_to_id, + &mut self.ambiguous_names, + ); + entry.insert(registration); + true + } } + } + /// Internal method to register additional lookups for a given [`TypeRegistration`]. + fn update_registration_indices( + registration: &TypeRegistration, + short_path_to_id: &mut HashMap<&'static str, TypeId>, + type_path_to_id: &mut HashMap<&'static str, TypeId>, + ambiguous_names: &mut HashSet<&'static str>, + ) { let short_name = registration.type_info().type_path_table().short_path(); - if self.short_path_to_id.contains_key(short_name) - || self.ambiguous_names.contains(short_name) - { + if short_path_to_id.contains_key(short_name) || ambiguous_names.contains(short_name) { // name is ambiguous. fall back to long names for all ambiguous types - self.short_path_to_id.remove(short_name); - self.ambiguous_names.insert(short_name); + short_path_to_id.remove(short_name); + ambiguous_names.insert(short_name); } else { - self.short_path_to_id - .insert(short_name, registration.type_id()); + short_path_to_id.insert(short_name, registration.type_id()); } - self.type_path_to_id - .insert(registration.type_info().type_path(), registration.type_id()); - self.registrations - .insert(registration.type_id(), registration); + type_path_to_id.insert(registration.type_info().type_path(), registration.type_id()); } /// Registers the type data `D` for type `T`. @@ -162,6 +262,10 @@ impl TypeRegistry { data.insert(D::from_type()); } + pub fn contains(&self, type_id: TypeId) -> bool { + self.registrations.contains_key(&type_id) + } + /// Returns a reference to the [`TypeRegistration`] of the type with the /// given [`TypeId`]. /// diff --git a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.rs b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.rs index c3693d06310db..5788063757ce3 100644 --- a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.rs +++ b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.rs @@ -1,4 +1,4 @@ -use bevy_reflect::{Reflect, TypePath}; +use bevy_reflect::{GetField, Reflect, Struct, TypePath}; #[derive(Reflect)] #[reflect(from_reflect = false)] @@ -11,7 +11,7 @@ struct Foo { struct NoReflect(f32); fn main() { - let mut foo: Box = Box::new(Foo:: { a: NoReflect(42.0) }); + let mut foo: Box = Box::new(Foo:: { a: NoReflect(42.0) }); // foo doesn't implement Reflect because NoReflect doesn't implement Reflect foo.get_field::("a").unwrap(); } diff --git a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.stderr b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.stderr index 40d36dcf977ae..22a6cc8d53ccf 100644 --- a/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.stderr +++ b/crates/bevy_reflect_compile_fail_tests/tests/reflect_derive/generics.fail.stderr @@ -1,16 +1,34 @@ -error[E0599]: no method named `get_field` found for struct `Box<(dyn Reflect + 'static)>` in the current scope - --> tests/reflect_derive/generics.fail.rs:16:9 - | -16 | foo.get_field::("a").unwrap(); - | ^^^^^^^^^ method not found in `Box` - error[E0277]: the trait bound `NoReflect: Reflect` is not satisfied - --> tests/reflect_derive/generics.fail.rs:14:37 + --> tests/reflect_derive/generics.fail.rs:16:21 + | +16 | foo.get_field::("a").unwrap(); + | --------- ^^^^^^^^^ the trait `Reflect` is not implemented for `NoReflect` + | | + | required by a bound introduced by this call + | + = help: the following other types implement trait `Reflect`: + bool + char + isize + i8 + i16 + i32 + i64 + i128 + and $N others +note: required by a bound in `bevy_reflect::GetField::get_field` + --> /home/runner/work/bevy/bevy/crates/bevy_reflect/src/struct_trait.rs:242:21 + | +242 | fn get_field(&self, name: &str) -> Option<&T>; + | ^^^^^^^ required by this bound in `GetField::get_field` + +error[E0277]: the trait bound `NoReflect: GetTypeRegistration` is not satisfied + --> tests/reflect_derive/generics.fail.rs:14:36 | -14 | let mut foo: Box = Box::new(Foo:: { a: NoReflect(42.0) }); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Reflect` is not implemented for `NoReflect` +14 | let mut foo: Box = Box::new(Foo:: { a: NoReflect(42.0) }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `GetTypeRegistration` is not implemented for `NoReflect` | - = help: the following other types implement trait `Reflect`: + = help: the following other types implement trait `GetTypeRegistration`: bool char isize @@ -20,7 +38,8 @@ error[E0277]: the trait bound `NoReflect: Reflect` is not satisfied i64 i128 and $N others -note: required for `Foo` to implement `Reflect` + = note: required for `NoReflect` to implement `RegisterForReflection` +note: required for `Foo` to implement `bevy_reflect::Struct` --> tests/reflect_derive/generics.fail.rs:3:10 | 3 | #[derive(Reflect)] @@ -28,5 +47,5 @@ note: required for `Foo` to implement `Reflect` 4 | #[reflect(from_reflect = false)] 5 | struct Foo { | ^^^^^^ - = note: required for the cast from `Box>` to `Box<(dyn Reflect + 'static)>` + = note: required for the cast from `Box>` to `Box<(dyn bevy_reflect::Struct + 'static)>` = note: this error originates in the derive macro `Reflect` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index ebb3e717e1654..f4d6234aec5c3 100644 --- a/crates/bevy_render/src/render_asset.rs +++ b/crates/bevy_render/src/render_asset.rs @@ -7,10 +7,12 @@ use bevy_ecs::{ system::{StaticSystemParam, SystemParam, SystemParamItem, SystemState}, world::{FromWorld, Mut}, }; +use bevy_reflect::std_traits::ReflectDefault; use bevy_reflect::{ utility::{reflect_hasher, NonGenericTypeInfoCell}, - FromReflect, Reflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, TypePath, - Typed, ValueInfo, + FromReflect, FromType, GetTypeRegistration, Reflect, ReflectDeserialize, ReflectFromPtr, + ReflectFromReflect, ReflectKind, ReflectMut, ReflectOwned, ReflectRef, ReflectSerialize, + TypeInfo, TypePath, TypeRegistration, Typed, ValueInfo, }; use bevy_utils::{thiserror::Error, HashMap, HashSet}; use serde::{Deserialize, Serialize}; @@ -162,6 +164,18 @@ impl Reflect for RenderAssetUsages { } } +impl GetTypeRegistration for RenderAssetUsages { + fn get_type_registration() -> TypeRegistration { + let mut registration = TypeRegistration::of::(); + registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); + registration.insert::(FromType::::from_type()); + registration + } +} + impl FromReflect for RenderAssetUsages { fn from_reflect(reflect: &dyn Reflect) -> Option { let raw_value = *reflect.as_any().downcast_ref::()?; diff --git a/examples/reflection/generic_reflection.rs b/examples/reflection/generic_reflection.rs index 409d814c327d0..7a3d3917adae8 100644 --- a/examples/reflection/generic_reflection.rs +++ b/examples/reflection/generic_reflection.rs @@ -12,8 +12,10 @@ fn main() { .run(); } +/// The `#[derive(Reflect)]` macro will automatically add any required bounds to `T`, +/// such as `Reflect` and `GetTypeRegistration`. #[derive(Reflect)] -struct MyType { +struct MyType { value: T, }