From 00dc66fa0c271f31ae7b800d3efaf602759f9949 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Tue, 23 Aug 2022 22:23:05 -0700 Subject: [PATCH 01/17] Add automatic reflection registration --- .../bevy_reflect_derive/src/derive_data.rs | 30 ++++- .../bevy_reflect_derive/src/impls/enums.rs | 4 +- .../bevy_reflect_derive/src/registration.rs | 15 ++- crates/bevy_reflect/src/enums/mod.rs | 2 +- crates/bevy_reflect/src/impls/std.rs | 127 +++++++++--------- crates/bevy_reflect/src/lib.rs | 116 +++++++++++++++- crates/bevy_reflect/src/tuple.rs | 19 +-- crates/bevy_reflect/src/type_registry.rs | 30 +++++ examples/reflection/generic_reflection.rs | 3 +- 9 files changed, 261 insertions(+), 85 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..c523be7baba4c 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, + 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..625bb91af6202 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -4,17 +4,28 @@ 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! { + fn register_type_dependencies(registry: &mut #bevy_reflect_path::TypeRegistry) { + #(registry.try_register::<#deps>();)* + } + } + }); + 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 +55,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/src/enums/mod.rs b/crates/bevy_reflect/src/enums/mod.rs index b78af06f86729..9aa6a9f0e9952 100644 --- a/crates/bevy_reflect/src/enums/mod.rs +++ b/crates/bevy_reflect/src/enums/mod.rs @@ -324,7 +324,7 @@ mod tests { #[test] fn enum_should_allow_generics() { #[derive(Reflect, Debug, PartialEq)] - enum TestEnum { + enum TestEnum { A, B(T), C { value: T }, diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 07500814d0263..c56a756d20802 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.try_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.try_register::(); + registry.try_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.try_register::(); + } } -impl GetTypeRegistration for Option { +impl GetTypeRegistration for Option { fn get_type_registration() -> TypeRegistration { TypeRegistration::of::>() } + + fn register_type_dependencies(registry: &mut TypeRegistry) { + registry.try_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.try_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..512dcded5f34d 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")] @@ -1002,6 +1005,99 @@ 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_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)] @@ -1314,7 +1410,7 @@ mod tests { // Struct (generic) #[derive(Reflect)] - struct MyGenericStruct { + struct MyGenericStruct { foo: T, bar: usize, } @@ -1903,8 +1999,8 @@ bevy_reflect::tests::Test { #[test] fn should_allow_multiple_custom_where() { #[derive(Reflect)] - #[reflect(where T: Default)] - #[reflect(where U: std::ops::Add)] + #[reflect(where T: Default + GetTypeRegistration)] + #[reflect(where U: std::ops::Add < T > + GetTypeRegistration)] struct Foo(T, U); #[derive(Reflect)] @@ -1925,7 +2021,7 @@ bevy_reflect::tests::Test { // We don't need `T` to be `Reflect` since we only care about `T::Assoc` #[derive(Reflect)] - #[reflect(where T::Assoc: core::fmt::Display)] + #[reflect(where T::Assoc: core::fmt::Display + GetTypeRegistration)] struct Foo(T::Assoc); #[derive(TypePath)] @@ -1949,7 +2045,7 @@ bevy_reflect::tests::Test { #[test] fn recursive_typed_storage_does_not_hang() { #[derive(Reflect)] - struct Recurse(T); + struct Recurse(T); let _ = > as Typed>::type_info(); let _ = > as TypePath>::type_path(); @@ -1981,6 +2077,16 @@ 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::>>(); + } + #[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..6d6a80a9b9f32 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.try_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..ef8cd81871023 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -57,7 +57,18 @@ impl Debug for TypeRegistryArc { /// /// [crate-level documentation]: crate pub trait GetTypeRegistration { + /// 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. + /// + /// If manually implementing, it is _highly_ recommended to use [`TypeRegistry::try_register`] for these dependent types. + /// Using this method allows the type to be skipped if it has already been registered, thus preventing any + /// undesired overwrites and reducing registration costs. + #[allow(unused_variables)] + fn register_type_dependencies(registry: &mut TypeRegistry) {} } impl Default for TypeRegistry { @@ -110,6 +121,21 @@ impl TypeRegistry { T: GetTypeRegistration, { self.add_registration(T::get_type_registration()); + T::register_type_dependencies(self); + } + + /// Attempts to register the type `T` if it has not yet been registered already. + /// + /// If the registration for type `T` already exists, it will not be registered again. + /// + /// To register the type, overwriting any existing registration, use [register](Self::register) instead. + pub fn try_register(&mut self) + where + T: GetTypeRegistration + 'static, + { + if !self.contains(TypeId::of::()) { + self.register::(); + } } /// Registers the type described by `registration`. @@ -162,6 +188,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/examples/reflection/generic_reflection.rs b/examples/reflection/generic_reflection.rs index 409d814c327d0..2cc4768dbed6f 100644 --- a/examples/reflection/generic_reflection.rs +++ b/examples/reflection/generic_reflection.rs @@ -1,6 +1,7 @@ //! Demonstrates how reflection is used with generic Rust types. use bevy::prelude::*; +use bevy::reflect::GetTypeRegistration; use std::any::TypeId; fn main() { @@ -13,7 +14,7 @@ fn main() { } #[derive(Reflect)] -struct MyType { +struct MyType { value: T, } From 56dc5bb3bd43cf92be08ea1753f4f084e49cd1d3 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Thu, 10 Nov 2022 00:05:00 -0700 Subject: [PATCH 02/17] Remove try_register --- .../bevy_reflect_derive/src/registration.rs | 2 +- crates/bevy_reflect/src/impls/std.rs | 12 ++--- crates/bevy_reflect/src/tuple.rs | 2 +- crates/bevy_reflect/src/type_registry.rs | 52 +++++++++++-------- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 625bb91af6202..1f80a90afa3d0 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -21,7 +21,7 @@ pub(crate) fn impl_get_type_registration<'a>( let type_deps_fn = type_dependencies.map(|deps| { quote! { fn register_type_dependencies(registry: &mut #bevy_reflect_path::TypeRegistry) { - #(registry.try_register::<#deps>();)* + #(registry.register::<#deps>();)* } } }); diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index c56a756d20802..578879091bfcc 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -364,7 +364,7 @@ macro_rules! impl_reflect_for_veclike { } fn register_type_dependencies(registry: &mut TypeRegistry) { - registry.try_register::(); + registry.register::(); } } @@ -594,8 +594,8 @@ macro_rules! impl_reflect_for_hashmap { } fn register_type_dependencies(registry: &mut TypeRegistry) { - registry.try_register::(); - registry.try_register::(); + registry.register::(); + registry.register::(); } } @@ -990,7 +990,7 @@ impl GetTypeRegistr } fn register_type_dependencies(registry: &mut TypeRegistry) { - registry.try_register::(); + registry.register::(); } } @@ -1000,7 +1000,7 @@ impl GetTypeRegistration for Op } fn register_type_dependencies(registry: &mut TypeRegistry) { - registry.try_register::(); + registry.register::(); } } @@ -1532,7 +1532,7 @@ impl GetTypeRegistratio } fn register_type_dependencies(registry: &mut TypeRegistry) { - registry.try_register::(); + registry.register::(); } } diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index 6d6a80a9b9f32..de6af437df4bb 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -594,7 +594,7 @@ macro_rules! impl_reflect_tuple { } fn register_type_dependencies(_registry: &mut TypeRegistry) { - $(_registry.try_register::<$name>();)* + $(_registry.register::<$name>();)* } } diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index ef8cd81871023..3a1ee47ff4b6b 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -63,10 +63,6 @@ pub trait GetTypeRegistration { /// /// 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. - /// - /// If manually implementing, it is _highly_ recommended to use [`TypeRegistry::try_register`] for these dependent types. - /// Using this method allows the type to be skipped if it has already been registered, thus preventing any - /// undesired overwrites and reducing registration costs. #[allow(unused_variables)] fn register_type_dependencies(registry: &mut TypeRegistry) {} } @@ -111,39 +107,53 @@ impl TypeRegistry { registry } - /// Registers the type `T`, adding reflect data as specified in the [`Reflect`] derive: + /// Attempts to register the type `T` if it has not yet been registered already. + /// + /// If the registration for type `T` already exists, it will not be registered again. + /// To register the type, overwriting any existing registration, use [register](Self::register) instead. + /// + /// Additionally, this will add the reflect [data](TypeData) as specified in the [`Reflect`] derive: /// ```ignore (Neither bevy_ecs nor serde "derive" are available.) - /// #[derive(Component, serde::Serialize, serde::Deserialize, Reflect)] + /// #[derive(Reflect)] /// #[reflect(Component, Serialize, Deserialize)] // will register ReflectComponent, ReflectSerialize, ReflectDeserialize + /// struct Foo; /// ``` pub fn register(&mut self) where T: GetTypeRegistration, { - self.add_registration(T::get_type_registration()); + if !self.add_registration(T::get_type_registration()) { + return; + } + T::register_type_dependencies(self); } - /// Attempts to register the type `T` if it has not yet been registered already. + /// Attempts to register the type described by `registration`. /// - /// If the registration for type `T` already exists, it will not be registered again. + /// If the registration for the type already exists, it will not be registered again. /// - /// To register the type, overwriting any existing registration, use [register](Self::register) instead. - pub fn try_register(&mut self) - where - T: GetTypeRegistration + 'static, - { - if !self.contains(TypeId::of::()) { - self.register::(); + /// To forcibly register the type, overwriting any existing registration, use the + /// [`force_add_registration`](Self::force_add_registration) method instead. + /// + /// Returns `true` if the registration was successfully added, + /// or `false` if it already exists. + pub fn add_registration(&mut self, registration: TypeRegistration) -> bool { + if self.contains(registration.type_id()) { + false + } else { + self.force_add_registration(registration); + true } } /// 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. + pub fn force_add_registration(&mut self, registration: TypeRegistration) { 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) From ff7f86d7449a4b46cf891eedde530cbe0c5be461 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sat, 25 Mar 2023 12:53:01 -0700 Subject: [PATCH 03/17] Automatically add GetTypeRegistration in Reflect derive --- .../bevy_reflect/bevy_reflect_derive/src/utility.rs | 6 +++++- crates/bevy_reflect/src/enums/mod.rs | 2 +- crates/bevy_reflect/src/lib.rs | 12 ++++++------ examples/reflection/generic_reflection.rs | 5 +++-- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 5e619cf1ade45..6c4de1727ec3a 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -217,10 +217,14 @@ 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. + // 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)), + .map(move |ty| quote!( + #ty : #reflect_bound + #bevy_reflect_path::TypePath + #bevy_reflect_path::GetTypeRegistration + )), ) } } diff --git a/crates/bevy_reflect/src/enums/mod.rs b/crates/bevy_reflect/src/enums/mod.rs index 9aa6a9f0e9952..b78af06f86729 100644 --- a/crates/bevy_reflect/src/enums/mod.rs +++ b/crates/bevy_reflect/src/enums/mod.rs @@ -324,7 +324,7 @@ mod tests { #[test] fn enum_should_allow_generics() { #[derive(Reflect, Debug, PartialEq)] - enum TestEnum { + enum TestEnum { A, B(T), C { value: T }, diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 512dcded5f34d..63795fa5dbc51 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -1410,7 +1410,7 @@ mod tests { // Struct (generic) #[derive(Reflect)] - struct MyGenericStruct { + struct MyGenericStruct { foo: T, bar: usize, } @@ -1999,8 +1999,8 @@ bevy_reflect::tests::Test { #[test] fn should_allow_multiple_custom_where() { #[derive(Reflect)] - #[reflect(where T: Default + GetTypeRegistration)] - #[reflect(where U: std::ops::Add < T > + GetTypeRegistration)] + #[reflect(where T: Default)] + #[reflect(where U: std::ops::Add < T >)] struct Foo(T, U); #[derive(Reflect)] @@ -2021,7 +2021,7 @@ bevy_reflect::tests::Test { // We don't need `T` to be `Reflect` since we only care about `T::Assoc` #[derive(Reflect)] - #[reflect(where T::Assoc: core::fmt::Display + GetTypeRegistration)] + #[reflect(where T::Assoc: core::fmt::Display)] struct Foo(T::Assoc); #[derive(TypePath)] @@ -2045,7 +2045,7 @@ bevy_reflect::tests::Test { #[test] fn recursive_typed_storage_does_not_hang() { #[derive(Reflect)] - struct Recurse(T); + struct Recurse(T); let _ = > as Typed>::type_info(); let _ = > as TypePath>::type_path(); @@ -2080,7 +2080,7 @@ bevy_reflect::tests::Test { #[test] fn recursive_registration_does_not_hang() { #[derive(Reflect)] - struct Recurse(T); + struct Recurse(T); let mut registry = TypeRegistry::empty(); diff --git a/examples/reflection/generic_reflection.rs b/examples/reflection/generic_reflection.rs index 2cc4768dbed6f..7a3d3917adae8 100644 --- a/examples/reflection/generic_reflection.rs +++ b/examples/reflection/generic_reflection.rs @@ -1,7 +1,6 @@ //! Demonstrates how reflection is used with generic Rust types. use bevy::prelude::*; -use bevy::reflect::GetTypeRegistration; use std::any::TypeId; fn main() { @@ -13,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, } From f924d8971982ff2a5bf5b69c9caca3f6590614f7 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 27 Mar 2023 18:41:52 -0700 Subject: [PATCH 04/17] Fix compile fail test --- .../tests/reflect_derive/generics.fail.rs | 4 +- .../tests/reflect_derive/generics.fail.stderr | 74 ++++++++++++++++--- 2 files changed, 66 insertions(+), 12 deletions(-) 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..faa5156a65b00 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}; #[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..30f277e5609a4 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,70 @@ -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 +error: cannot find derive macro `TypePath` in this scope + --> tests/reflect_derive/generics.fail.rs:10:10 + | +10 | #[derive(TypePath)] + | ^^^^^^^^ + | +help: consider importing this derive macro + | +1 + use bevy_reflect::TypePath; | -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` + | + = 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 `GetTypeRegistration` is not implemented for `NoReflect` + | + = help: the following other types implement trait `GetTypeRegistration`: + bool + char + isize + i8 + i16 + i32 + i64 + i128 + and $N others +note: required for `Foo` to implement `bevy_reflect::Struct` + --> tests/reflect_derive/generics.fail.rs:3:10 + | +3 | #[derive(Reflect)] + | ^^^^^^^ unsatisfied trait bound introduced in this `derive` macro +4 | #[reflect(from_reflect = false)] +5 | struct Foo { + | ^^^^^^ + = 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) + +error[E0277]: the trait bound `NoReflect: TypePath` 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 `TypePath` is not implemented for `NoReflect` | - = help: the following other types implement trait `Reflect`: + = help: the following other types implement trait `TypePath`: bool char isize @@ -20,7 +74,7 @@ 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 `Foo` to implement `bevy_reflect::Struct` --> tests/reflect_derive/generics.fail.rs:3:10 | 3 | #[derive(Reflect)] @@ -28,5 +82,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) From b9a9805873032a6ffd4d7b1f0286db9806efb367 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Wed, 13 Sep 2023 19:31:12 -0700 Subject: [PATCH 05/17] Remove premature get_type_registration call --- crates/bevy_reflect/src/type_registry.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 3a1ee47ff4b6b..971c6921dd3d6 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -56,7 +56,7 @@ 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. @@ -122,10 +122,11 @@ impl TypeRegistry { where T: GetTypeRegistration, { - if !self.add_registration(T::get_type_registration()) { + if self.contains(TypeId::of::()) { return; } + self.force_add_registration(T::get_type_registration()); T::register_type_dependencies(self); } From c4863e38c4caa441d1db7a833d00e02b60187b95 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 23 Feb 2024 13:44:50 -0700 Subject: [PATCH 06/17] Re-allow dynamic types as active fields --- .../bevy_reflect_derive/src/registration.rs | 2 +- .../bevy_reflect_derive/src/utility.rs | 14 ++-- crates/bevy_reflect/src/lib.rs | 66 ++++++++++++++++++- 3 files changed, 73 insertions(+), 9 deletions(-) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index 1f80a90afa3d0..fce0f6d4efbbc 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -21,7 +21,7 @@ pub(crate) fn impl_get_type_registration<'a>( let type_deps_fn = type_dependencies.map(|deps| { quote! { fn register_type_dependencies(registry: &mut #bevy_reflect_path::TypeRegistry) { - #(registry.register::<#deps>();)* + #(<#deps as #bevy_reflect_path::__macro_exports::RegisterForReflection>::__register(registry);)* } } }); diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs index 6c4de1727ec3a..a32fcabeea640 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/utility.rs @@ -219,13 +219,13 @@ impl<'a, 'b> WhereClauseOptions<'a, 'b> { // construct `NamedField` and `UnnamedField` instances for the `Typed` impl. // 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::GetTypeRegistration - )), - ) + 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/lib.rs b/crates/bevy_reflect/src/lib.rs index 63795fa5dbc51..1f7d18596e174 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -538,9 +538,48 @@ pub use erased_serde; extern crate alloc; +/// Exorts 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)] @@ -1075,6 +1114,31 @@ mod tests { ); } + #[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)] From a1183a116e639b449992232e3573a5d316f8e807 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 23 Feb 2024 13:56:34 -0700 Subject: [PATCH 07/17] Add missing GetTypeRegistration impl --- crates/bevy_render/src/render_asset.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index 3eb0d2f0b610e..a182cade22032 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}; @@ -155,6 +157,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::()?; From 7d55bf8c800b7d510d2f20c5f60858121cfcb75a Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 23 Feb 2024 14:06:16 -0700 Subject: [PATCH 08/17] Fix compile fail tests --- .../tests/reflect_derive/generics.fail.rs | 2 +- .../tests/reflect_derive/generics.fail.stderr | 43 ++----------------- 2 files changed, 5 insertions(+), 40 deletions(-) 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 faa5156a65b00..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::{GetField, Reflect, Struct}; +use bevy_reflect::{GetField, Reflect, Struct, TypePath}; #[derive(Reflect)] #[reflect(from_reflect = false)] 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 30f277e5609a4..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,19 +1,10 @@ -error: cannot find derive macro `TypePath` in this scope - --> tests/reflect_derive/generics.fail.rs:10:10 - | -10 | #[derive(TypePath)] - | ^^^^^^^^ - | -help: consider importing this derive macro - | -1 + use bevy_reflect::TypePath; - | - error[E0277]: the trait bound `NoReflect: Reflect` is not satisfied --> tests/reflect_derive/generics.fail.rs:16:21 | 16 | foo.get_field::("a").unwrap(); - | ^^^^^^^^^ the trait `Reflect` is not implemented for `NoReflect` + | --------- ^^^^^^^^^ 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 @@ -47,33 +38,7 @@ error[E0277]: the trait bound `NoReflect: GetTypeRegistration` is not satisfied i64 i128 and $N others -note: required for `Foo` to implement `bevy_reflect::Struct` - --> tests/reflect_derive/generics.fail.rs:3:10 - | -3 | #[derive(Reflect)] - | ^^^^^^^ unsatisfied trait bound introduced in this `derive` macro -4 | #[reflect(from_reflect = false)] -5 | struct Foo { - | ^^^^^^ - = 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) - -error[E0277]: the trait bound `NoReflect: TypePath` 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 `TypePath` is not implemented for `NoReflect` - | - = help: the following other types implement trait `TypePath`: - bool - char - isize - i8 - i16 - i32 - i64 - i128 - and $N others + = note: required for `NoReflect` to implement `RegisterForReflection` note: required for `Foo` to implement `bevy_reflect::Struct` --> tests/reflect_derive/generics.fail.rs:3:10 | From 164b0f8ea790f268d78a34199f53f3aed3e2f110 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Sat, 24 Feb 2024 15:40:56 -0700 Subject: [PATCH 09/17] Update crates/bevy_reflect/src/lib.rs Co-authored-by: James Liu --- crates/bevy_reflect/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 1f7d18596e174..c657418c67d2b 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -538,7 +538,7 @@ pub use erased_serde; extern crate alloc; -/// Exorts used by the reflection macros. +/// Exports used by the reflection macros. /// /// These are not meant to be used directly and are subject to breaking changes. #[doc(hidden)] From 548a472b9ef94e0202c8ec25c34480a3de737253 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Sun, 25 Feb 2024 21:07:50 -0700 Subject: [PATCH 10/17] Apply suggestions from code review Co-authored-by: radiish --- crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs | 2 +- crates/bevy_reflect/src/lib.rs | 2 +- 2 files changed, 2 insertions(+), 2 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 c523be7baba4c..48625ec3ca283 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/derive_data.rs @@ -474,7 +474,7 @@ impl<'a> ReflectMeta<'a> { self, where_clause_options, None, - None::>, + Option::>::None, ) } diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index c657418c67d2b..fca9632cfaada 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2064,7 +2064,7 @@ bevy_reflect::tests::Test { fn should_allow_multiple_custom_where() { #[derive(Reflect)] #[reflect(where T: Default)] - #[reflect(where U: std::ops::Add < T >)] + #[reflect(where U: std::ops::Add)] struct Foo(T, U); #[derive(Reflect)] From a5ca22dc4aea8c5e599654c8cefcb896b2d36512 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 25 Feb 2024 21:09:53 -0700 Subject: [PATCH 11/17] Rename force_add_registration -> overwrite_registration --- crates/bevy_reflect/src/type_registry.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 971c6921dd3d6..f37725a253458 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -126,7 +126,7 @@ impl TypeRegistry { return; } - self.force_add_registration(T::get_type_registration()); + self.overwrite_registration(T::get_type_registration()); T::register_type_dependencies(self); } @@ -135,7 +135,7 @@ impl TypeRegistry { /// 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 - /// [`force_add_registration`](Self::force_add_registration) method instead. + /// [`overwrite_registration`](Self::overwrite_registration) method instead. /// /// Returns `true` if the registration was successfully added, /// or `false` if it already exists. @@ -143,7 +143,7 @@ impl TypeRegistry { if self.contains(registration.type_id()) { false } else { - self.force_add_registration(registration); + self.overwrite_registration(registration); true } } @@ -154,7 +154,7 @@ impl TypeRegistry { /// /// To avoid overwriting existing registrations, it's recommended to use the /// [`register`](Self::register) or [`add_registration`](Self::add_registration) methods instead. - pub fn force_add_registration(&mut self, registration: TypeRegistration) { + pub fn overwrite_registration(&mut self, registration: TypeRegistration) { 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) From d4fe740b3f81a8acbd7999b7656389ee0067494e Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 25 Feb 2024 21:27:58 -0700 Subject: [PATCH 12/17] Change return type of TypeRegistry::add_registration --- crates/bevy_reflect/src/type_registry.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index f37725a253458..3fe67f719ba03 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -137,14 +137,17 @@ impl TypeRegistry { /// To forcibly register the type, overwriting any existing registration, use the /// [`overwrite_registration`](Self::overwrite_registration) method instead. /// - /// Returns `true` if the registration was successfully added, - /// or `false` if it already exists. - pub fn add_registration(&mut self, registration: TypeRegistration) -> bool { + /// Returns `None` if the registration was successfully added, + /// or `Some(existing)` if it already exists. + pub fn add_registration( + &mut self, + registration: TypeRegistration, + ) -> Option<&TypeRegistration> { if self.contains(registration.type_id()) { - false + self.get(registration.type_id()) } else { self.overwrite_registration(registration); - true + None } } From 0bf553bd64ce891539f2f96dc6c409fe243e719a Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 1 Mar 2024 11:29:21 -0700 Subject: [PATCH 13/17] Added more test cases for recursive registration --- crates/bevy_reflect/src/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index fca9632cfaada..52eae900d6d67 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -2149,6 +2149,29 @@ bevy_reflect::tests::Test { 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] From 799a23a93a000b4fb764f0984cff3f75b2544b3d Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 1 Mar 2024 12:53:48 -0700 Subject: [PATCH 14/17] Improve docs --- crates/bevy_reflect/src/type_registry.rs | 48 ++++++++++++++++++++---- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index 3fe67f719ba03..bbfcafd0c65ca 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -109,14 +109,40 @@ impl TypeRegistry { /// Attempts to register the type `T` if it has not yet been registered already. /// - /// If the registration for type `T` already exists, it will not be registered again. - /// To register the type, overwriting any existing registration, use [register](Self::register) instead. - /// - /// Additionally, this will add the reflect [data](TypeData) as specified in the [`Reflect`] derive: - /// ```ignore (Neither bevy_ecs nor serde "derive" are available.) - /// #[derive(Reflect)] - /// #[reflect(Component, Serialize, Deserialize)] // will register ReflectComponent, ReflectSerialize, ReflectDeserialize - /// struct Foo; + /// 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 @@ -137,6 +163,9 @@ impl TypeRegistry { /// 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 `None` if the registration was successfully added, /// or `Some(existing)` if it already exists. pub fn add_registration( @@ -157,6 +186,9 @@ impl TypeRegistry { /// /// 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) { let short_name = registration.type_info().type_path_table().short_path(); if self.short_path_to_id.contains_key(short_name) From 726857f3f2df60d71e9712a36869d7c6419e934e Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sun, 3 Mar 2024 13:59:05 -0800 Subject: [PATCH 15/17] Remove redundant hashing in register() --- crates/bevy_reflect/src/type_registry.rs | 64 ++++++++++++++---------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index bbfcafd0c65ca..eb93b153a0150 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -148,12 +148,9 @@ impl TypeRegistry { where T: GetTypeRegistration, { - if self.contains(TypeId::of::()) { - return; + if self.add_registration(T::get_type_registration()) { + T::register_type_dependencies(self); } - - self.overwrite_registration(T::get_type_registration()); - T::register_type_dependencies(self); } /// Attempts to register the type described by `registration`. @@ -166,17 +163,20 @@ impl TypeRegistry { /// This method will _not_ register type dependencies. /// Use [`register`](Self::register) to register a type with its dependencies. /// - /// Returns `None` if the registration was successfully added, - /// or `Some(existing)` if it already exists. - pub fn add_registration( - &mut self, - registration: TypeRegistration, - ) -> Option<&TypeRegistration> { - if self.contains(registration.type_id()) { - self.get(registration.type_id()) - } else { - self.overwrite_registration(registration); - None + /// Returns `true` if the registration was added and `false` if it already exists. + pub fn add_registration(&mut self, registration: TypeRegistration) -> bool { + match self.registrations.entry(registration.type_id()) { + bevy_utils::hashbrown::hash_map::Entry::Occupied(_) => false, + bevy_utils::hashbrown::hash_map::Entry::Vacant(entry) => { + Self::update_registration_indices( + &mut self.short_path_to_id, + &mut self.type_path_to_id, + &mut self.ambiguous_names, + ®istration, + ); + entry.insert(registration); + true + } } } @@ -190,21 +190,31 @@ impl TypeRegistry { /// 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( + &mut self.short_path_to_id, + &mut self.type_path_to_id, + &mut self.ambiguous_names, + ®istration, + ); + self.registrations + .insert(registration.type_id(), registration); + } + + fn update_registration_indices( + short_path_to_id: &mut HashMap<&'static str, TypeId>, + type_path_to_id: &mut HashMap<&'static str, TypeId>, + ambiguous_names: &mut HashSet<&'static str>, + registration: &TypeRegistration, + ) { 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`. From e9faa7961ddfa4b207534b6e54159a0bbc6befa1 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 3 Mar 2024 15:52:34 -0700 Subject: [PATCH 16/17] Add TypeRegistry::register_internal --- crates/bevy_reflect/src/type_registry.rs | 50 ++++++++++++++++-------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/crates/bevy_reflect/src/type_registry.rs b/crates/bevy_reflect/src/type_registry.rs index eb93b153a0150..3fdd296d3eb5e 100644 --- a/crates/bevy_reflect/src/type_registry.rs +++ b/crates/bevy_reflect/src/type_registry.rs @@ -148,7 +148,7 @@ impl TypeRegistry { where T: GetTypeRegistration, { - if self.add_registration(T::get_type_registration()) { + if self.register_internal(TypeId::of::(), T::get_type_registration) { T::register_type_dependencies(self); } } @@ -165,19 +165,8 @@ impl TypeRegistry { /// /// Returns `true` if the registration was added and `false` if it already exists. pub fn add_registration(&mut self, registration: TypeRegistration) -> bool { - match self.registrations.entry(registration.type_id()) { - bevy_utils::hashbrown::hash_map::Entry::Occupied(_) => false, - bevy_utils::hashbrown::hash_map::Entry::Vacant(entry) => { - Self::update_registration_indices( - &mut self.short_path_to_id, - &mut self.type_path_to_id, - &mut self.ambiguous_names, - ®istration, - ); - entry.insert(registration); - true - } - } + let type_id = registration.type_id(); + self.register_internal(type_id, || registration) } /// Registers the type described by `registration`. @@ -191,20 +180,49 @@ impl TypeRegistry { /// 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, - ®istration, ); 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>, - registration: &TypeRegistration, ) { let short_name = registration.type_info().type_path_table().short_path(); if short_path_to_id.contains_key(short_name) || ambiguous_names.contains(short_name) { From a14793a231ebcfe1d4fce2bdcd6a9d7ea94e1838 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Sun, 3 Mar 2024 19:55:04 -0700 Subject: [PATCH 17/17] Add #[inline(never)] --- crates/bevy_reflect/bevy_reflect_derive/src/registration.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs index fce0f6d4efbbc..d277d823c59f9 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/registration.rs @@ -20,6 +20,7 @@ pub(crate) fn impl_get_type_registration<'a>( 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);)* }