From 75130bd5ecfe58693b6a6777fd5cee04b6a34489 Mon Sep 17 00:00:00 2001 From: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Date: Wed, 26 Apr 2023 05:17:46 -0700 Subject: [PATCH] bevy_reflect: Better proxies (#6971) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Objective > This PR is based on discussion from #6601 The Dynamic types (e.g. `DynamicStruct`, `DynamicList`, etc.) act as both: 1. Dynamic containers which may hold any arbitrary data 2. Proxy types which may represent any other type Currently, the only way we can represent the proxy-ness of a Dynamic is by giving it a name. ```rust // This is just a dynamic container let mut data = DynamicStruct::default(); // This is a "proxy" data.set_name(std::any::type_name::()); ``` This type name is the only way we check that the given Dynamic is a proxy of some other type. When we need to "assert the type" of a `dyn Reflect`, we call `Reflect::type_name` on it. However, because we're only using a string to denote the type, we run into a few gotchas and limitations. For example, hashing a Dynamic proxy may work differently than the type it proxies: ```rust #[derive(Reflect, Hash)] #[reflect(Hash)] struct Foo(i32); let concrete = Foo(123); let dynamic = concrete.clone_dynamic(); let concrete_hash = concrete.reflect_hash(); let dynamic_hash = dynamic.reflect_hash(); // The hashes are not equal because `concrete` uses its own `Hash` impl // while `dynamic` uses a reflection-based hashing algorithm assert_ne!(concrete_hash, dynamic_hash); ``` Because the Dynamic proxy only knows about the name of the type, it's unaware of any other information about it. This means it also differs on `Reflect::reflect_partial_eq`, and may include ignored or skipped fields in places the concrete type wouldn't. ## Solution Rather than having Dynamics pass along just the type name of proxied types, we can instead have them pass around the `TypeInfo`. Now all Dynamic types contain an `Option<&'static TypeInfo>` rather than a `String`: ```diff pub struct DynamicTupleStruct { - type_name: String, + represented_type: Option<&'static TypeInfo>, fields: Vec>, } ``` By changing `Reflect::get_type_info` to `Reflect::represented_type_info`, hopefully we make this behavior a little clearer. And to account for `None` values on these dynamic types, `Reflect::represented_type_info` now returns `Option<&'static TypeInfo>`. ```rust let mut data = DynamicTupleStruct::default(); // Not proxying any specific type assert!(dyn_tuple_struct.represented_type_info().is_none()); let type_info = ::type_info(); dyn_tuple_struct.set_represented_type(Some(type_info)); // Alternatively: // let dyn_tuple_struct = foo.clone_dynamic(); // Now we're proxying `Foo` assert!(dyn_tuple_struct.represented_type_info().is_some()); ``` This means that we can have full access to all the static type information for the proxied type. Future work would include transitioning more static type information (trait impls, attributes, etc.) over to the `TypeInfo` so it can actually be utilized by Dynamic proxies. ### Alternatives & Rationale > **Note** > These alternatives were written when this PR was first made using a `Proxy` trait. This trait has since been removed.
View #### Alternative: The `Proxy` Approach I had considered adding something like a `Proxy` type where `T` would be the Dynamic and would contain the proxied type information. This was nice in that it allows us to explicitly determine whether something is a proxy or not at a type level. `Proxy` proxies a struct. Makes sense. The reason I didn't go with this approach is because (1) tuples, (2) complexity, and (3) `PartialReflect`. The `DynamicTuple` struct allows us to represent tuples at runtime. It also allows us to do something you normally can't with tuples: add new fields. Because of this, adding a field immediately invalidates the proxy (e.g. our info for `(i32, i32)` doesn't apply to `(i32, i32, NewField)`). By going with this PR's approach, we can just remove the type info on `DynamicTuple` when that happens. However, with the `Proxy` approach, it becomes difficult to represent this behavior— we'd have to completely control how we access data for `T` for each `T`. Secondly, it introduces some added complexities (aside from the manual impls for each `T`). Does `Proxy` impl `Reflect`? Likely yes, if we want to represent it as `dyn Reflect`. What `TypeInfo` do we give it? How would we forward reflection methods to the inner type (remember, we don't have specialization)? How do we separate this from Dynamic types? And finally, how do all this in a way that's both logical and intuitive for users? Lastly, introducing a `Proxy` trait rather than a `Proxy` struct is actually more inline with the [Unique Reflect RFC](https://github.com/bevyengine/rfcs/pull/56). In a way, the `Proxy` trait is really one part of the `PartialReflect` trait introduced in that RFC (it's technically not in that RFC but it fits well with it), where the `PartialReflect` serves as a way for proxies to work _like_ concrete types without having full access to everything a concrete `Reflect` type can do. This would help bridge the gap between the current state of the crate and the implementation of that RFC. All that said, this is still a viable solution. If the community believes this is the better path forward, then we can do that instead. These were just my reasons for not initially going with it in this PR. #### Alternative: The Type Registry Approach The `Proxy` trait is great and all, but how does it solve the original problem? Well, it doesn't— yet! The goal would be to start moving information from the derive macro and its attributes to the generated `TypeInfo` since these are known statically and shouldn't change. For example, adding `ignored: bool` to `[Un]NamedField` or a list of impls. However, there is another way of storing this information. This is, of course, one of the uses of the `TypeRegistry`. If we're worried about Dynamic proxies not aligning with their concrete counterparts, we could move more type information to the registry and require its usage. For example, we could replace `Reflect::reflect_hash(&self)` with `Reflect::reflect_hash(&self, registry: &TypeRegistry)`. That's not the _worst_ thing in the world, but it is an ergonomics loss. Additionally, other attributes may have their own requirements, further restricting what's possible without the registry. The `Reflect::apply` method will require the registry as well now. Why? Well because the `map_apply` function used for the `Reflect::apply` impls on `Map` types depends on `Map::insert_boxed`, which (at least for `DynamicMap`) requires `Reflect::reflect_hash`. The same would apply when adding support for reflection-based diffing, which will require `Reflect::reflect_partial_eq`. Again, this is a totally viable alternative. I just chose not to go with it for the reasons above. If we want to go with it, then we can close this PR and we can pursue this alternative instead. #### Downsides Just to highlight a quick potential downside (likely needs more investigation): retrieving the `TypeInfo` requires acquiring a lock on the `GenericTypeInfoCell` used by the `Typed` impls for generic types (non-generic types use a `OnceBox which should be faster). I am not sure how much of a performance hit that is and will need to run some benchmarks to compare against.
### Open Questions 1. Should we use `Cow<'static, TypeInfo>` instead? I think that might be easier for modding? Perhaps, in that case, we need to update `Typed::type_info` and friends as well? 2. Are the alternatives better than the approach this PR takes? Are there other alternatives? --- ## Changelog ### Changed - `Reflect::get_type_info` has been renamed to `Reflect::represented_type_info` - This method now returns `Option<&'static TypeInfo>` rather than just `&'static TypeInfo` ### Added - Added `Reflect::is_dynamic` method to indicate when a type is dynamic - Added a `set_represented_type` method on all dynamic types ### Removed - Removed `TypeInfo::Dynamic` (use `Reflect::is_dynamic` instead) - Removed `Typed` impls for all dynamic types ## Migration Guide - The Dynamic types no longer take a string type name. Instead, they require a static reference to `TypeInfo`: ```rust #[derive(Reflect)] struct MyTupleStruct(f32, f32); let mut dyn_tuple_struct = DynamicTupleStruct::default(); dyn_tuple_struct.insert(1.23_f32); dyn_tuple_struct.insert(3.21_f32); // BEFORE: let type_name = std::any::type_name::(); dyn_tuple_struct.set_name(type_name); // AFTER: let type_info = ::type_info(); dyn_tuple_struct.set_represented_type(Some(type_info)); ``` - `Reflect::get_type_info` has been renamed to `Reflect::represented_type_info` and now also returns an `Option<&'static TypeInfo>` (instead of just `&'static TypeInfo`): ```rust // BEFORE: let info: &'static TypeInfo = value.get_type_info(); // AFTER: let info: &'static TypeInfo = value.represented_type_info().unwrap(); ``` - `TypeInfo::Dynamic` and `DynamicInfo` has been removed. Use `Reflect::is_dynamic` instead: ```rust // BEFORE: if matches!(value.get_type_info(), TypeInfo::Dynamic) { // ... } // AFTER: if value.is_dynamic() { // ... } ``` --------- Co-authored-by: radiish --- benches/benches/bevy_reflect/struct.rs | 4 +- .../bevy_reflect_derive/src/impls/enums.rs | 4 +- .../bevy_reflect_derive/src/impls/structs.rs | 6 +- .../src/impls/tuple_structs.rs | 6 +- .../bevy_reflect_derive/src/impls/values.rs | 4 +- crates/bevy_reflect/src/array.rs | 58 +++++++------- crates/bevy_reflect/src/enums/dynamic_enum.rs | 77 +++++++++---------- crates/bevy_reflect/src/enums/mod.rs | 12 +-- crates/bevy_reflect/src/from_reflect.rs | 4 +- crates/bevy_reflect/src/impls/smallvec.rs | 4 +- crates/bevy_reflect/src/impls/std.rs | 26 +++---- crates/bevy_reflect/src/lib.rs | 46 +++++------ crates/bevy_reflect/src/list.rs | 58 +++++++------- crates/bevy_reflect/src/map.rs | 54 ++++++------- crates/bevy_reflect/src/reflect.rs | 27 ++++++- crates/bevy_reflect/src/serde/de.rs | 21 ++--- crates/bevy_reflect/src/serde/mod.rs | 12 +++ crates/bevy_reflect/src/serde/ser.rs | 59 ++++++-------- crates/bevy_reflect/src/struct_trait.rs | 52 +++++++------ crates/bevy_reflect/src/tuple.rs | 71 +++++++++-------- crates/bevy_reflect/src/tuple_struct.rs | 52 +++++++------ crates/bevy_reflect/src/type_info.rs | 72 ++--------------- crates/bevy_reflect/src/utility.rs | 4 +- 23 files changed, 357 insertions(+), 376 deletions(-) diff --git a/benches/benches/bevy_reflect/struct.rs b/benches/benches/bevy_reflect/struct.rs index 782d5e0b73d9d..0495701807c9e 100644 --- a/benches/benches/bevy_reflect/struct.rs +++ b/benches/benches/bevy_reflect/struct.rs @@ -148,14 +148,14 @@ fn concrete_struct_type_info(criterion: &mut Criterion) { BenchmarkId::new("NonGeneric", field_count), &standard, |bencher, s| { - bencher.iter(|| black_box(s.get_type_info())); + bencher.iter(|| black_box(s.get_represented_type_info())); }, ); group.bench_with_input( BenchmarkId::new("Generic", field_count), &generic, |bencher, s| { - bencher.iter(|| black_box(s.get_type_info())); + bencher.iter(|| black_box(s.get_represented_type_info())); }, ); } 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 666283615d985..2f2f5cf5e3582 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/enums.rs @@ -192,8 +192,8 @@ pub(crate) fn impl_enum(reflect_enum: &ReflectEnum) -> TokenStream { } #[inline] - fn get_type_info(&self) -> &'static #bevy_reflect_path::TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> #FQOption<&'static #bevy_reflect_path::TypeInfo> { + #FQOption::Some(::type_info()) } #[inline] diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs index e22634e5de762..2b7fd1815c607 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/structs.rs @@ -151,7 +151,7 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicStruct { let mut dynamic: #bevy_reflect_path::DynamicStruct = #FQDefault::default(); - dynamic.set_name(::std::string::ToString::to_string(#bevy_reflect_path::Reflect::type_name(self))); + dynamic.set_represented_type(#bevy_reflect_path::Reflect::get_represented_type_info(self)); #(dynamic.insert_boxed(#field_names, #bevy_reflect_path::Reflect::clone_value(&self.#field_idents));)* dynamic } @@ -164,8 +164,8 @@ pub(crate) fn impl_struct(reflect_struct: &ReflectStruct) -> TokenStream { } #[inline] - fn get_type_info(&self) -> &'static #bevy_reflect_path::TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> #FQOption<&'static #bevy_reflect_path::TypeInfo> { + #FQOption::Some(::type_info()) } #[inline] diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs index c41c53b86ef55..c74d4d9064f80 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/tuple_structs.rs @@ -122,7 +122,7 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { fn clone_dynamic(&self) -> #bevy_reflect_path::DynamicTupleStruct { let mut dynamic: #bevy_reflect_path::DynamicTupleStruct = #FQDefault::default(); - dynamic.set_name(::std::string::ToString::to_string(#bevy_reflect_path::Reflect::type_name(self))); + dynamic.set_represented_type(#bevy_reflect_path::Reflect::get_represented_type_info(self)); #(dynamic.insert_boxed(#bevy_reflect_path::Reflect::clone_value(&self.#field_idents));)* dynamic } @@ -135,8 +135,8 @@ pub(crate) fn impl_tuple_struct(reflect_struct: &ReflectStruct) -> TokenStream { } #[inline] - fn get_type_info(&self) -> &'static #bevy_reflect_path::TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> #FQOption<&'static #bevy_reflect_path::TypeInfo> { + #FQOption::Some(::type_info()) } #[inline] diff --git a/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs b/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs index 4aa3aed418fb6..964ef1f2ce8a8 100644 --- a/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs +++ b/crates/bevy_reflect/bevy_reflect_derive/src/impls/values.rs @@ -49,8 +49,8 @@ pub(crate) fn impl_value(meta: &ReflectMeta) -> TokenStream { } #[inline] - fn get_type_info(&self) -> &'static #bevy_reflect_path::TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> #FQOption<&'static #bevy_reflect_path::TypeInfo> { + #FQOption::Some(::type_info()) } #[inline] diff --git a/crates/bevy_reflect/src/array.rs b/crates/bevy_reflect/src/array.rs index 947601a1548e5..0367e1add4f79 100644 --- a/crates/bevy_reflect/src/array.rs +++ b/crates/bevy_reflect/src/array.rs @@ -1,7 +1,4 @@ -use crate::{ - utility::{reflect_hasher, NonGenericTypeInfoCell}, - DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, -}; +use crate::{utility::reflect_hasher, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo}; use std::{ any::{Any, TypeId}, fmt::Debug, @@ -67,7 +64,7 @@ pub trait Array: Reflect { /// Clones the list, producing a [`DynamicArray`]. fn clone_dynamic(&self) -> DynamicArray { DynamicArray { - name: self.type_name().to_string(), + represented_type: self.get_represented_type_info(), values: self.iter().map(|value| value.clone_value()).collect(), } } @@ -167,7 +164,7 @@ impl ArrayInfo { /// [`DynamicList`]: crate::DynamicList #[derive(Debug)] pub struct DynamicArray { - pub(crate) name: String, + pub(crate) represented_type: Option<&'static TypeInfo>, pub(crate) values: Box<[Box]>, } @@ -175,14 +172,14 @@ impl DynamicArray { #[inline] pub fn new(values: Box<[Box]>) -> Self { Self { - name: String::default(), + represented_type: None, values, } } pub fn from_vec(values: Vec) -> Self { Self { - name: String::default(), + represented_type: None, values: values .into_iter() .map(|field| Box::new(field) as Box) @@ -191,26 +188,37 @@ impl DynamicArray { } } - #[inline] - pub fn name(&self) -> &str { - &self.name - } + /// Sets the [type] to be represented by this `DynamicArray`. + /// + /// # Panics + /// + /// Panics if the given [type] is not a [`TypeInfo::Array`]. + /// + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::Array(_)), + "expected TypeInfo::Array but received: {:?}", + represented_type + ); + } - #[inline] - pub fn set_name(&mut self, name: String) { - self.name = name; + self.represented_type = represented_type; } } impl Reflect for DynamicArray { #[inline] fn type_name(&self) -> &str { - self.name.as_str() + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_else(|| std::any::type_name::()) } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } #[inline] @@ -281,6 +289,11 @@ impl Reflect for DynamicArray { fn reflect_partial_eq(&self, value: &dyn Reflect) -> Option { array_partial_eq(self, value) } + + #[inline] + fn is_dynamic(&self) -> bool { + true + } } impl Array for DynamicArray { @@ -312,7 +325,7 @@ impl Array for DynamicArray { #[inline] fn clone_dynamic(&self) -> DynamicArray { DynamicArray { - name: self.name.clone(), + represented_type: self.represented_type, values: self .values .iter() @@ -322,13 +335,6 @@ impl Array for DynamicArray { } } -impl Typed for DynamicArray { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) - } -} - /// An iterator over an [`Array`]. pub struct ArrayIter<'a> { array: &'a dyn Array, diff --git a/crates/bevy_reflect/src/enums/dynamic_enum.rs b/crates/bevy_reflect/src/enums/dynamic_enum.rs index db9e050fbe2ab..1e4cce12780d7 100644 --- a/crates/bevy_reflect/src/enums/dynamic_enum.rs +++ b/crates/bevy_reflect/src/enums/dynamic_enum.rs @@ -1,8 +1,6 @@ -use crate::utility::NonGenericTypeInfoCell; use crate::{ - enum_debug, enum_hash, enum_partial_eq, DynamicInfo, DynamicStruct, DynamicTuple, Enum, - Reflect, ReflectMut, ReflectOwned, ReflectRef, Struct, Tuple, TypeInfo, Typed, - VariantFieldIter, VariantType, + enum_debug, enum_hash, enum_partial_eq, DynamicStruct, DynamicTuple, Enum, Reflect, ReflectMut, + ReflectOwned, ReflectRef, Struct, Tuple, TypeInfo, VariantFieldIter, VariantType, }; use std::any::Any; use std::fmt::Formatter; @@ -58,7 +56,6 @@ impl From<()> for DynamicVariant { /// /// // Create a DynamicEnum to represent the new value /// let mut dyn_enum = DynamicEnum::new( -/// Reflect::type_name(&value), /// "None", /// DynamicVariant::Unit /// ); @@ -71,7 +68,7 @@ impl From<()> for DynamicVariant { /// ``` #[derive(Default, Debug)] pub struct DynamicEnum { - name: String, + represented_type: Option<&'static TypeInfo>, variant_name: String, variant_index: usize, variant: DynamicVariant, @@ -82,17 +79,12 @@ impl DynamicEnum { /// /// # Arguments /// - /// * `name`: The type name of the enum /// * `variant_name`: The name of the variant to set /// * `variant`: The variant data /// - pub fn new, V: Into>( - name: I, - variant_name: I, - variant: V, - ) -> Self { + pub fn new, V: Into>(variant_name: I, variant: V) -> Self { Self { - name: name.into(), + represented_type: None, variant_index: 0, variant_name: variant_name.into(), variant: variant.into(), @@ -103,33 +95,40 @@ impl DynamicEnum { /// /// # Arguments /// - /// * `name`: The type name of the enum /// * `variant_index`: The index of the variant to set /// * `variant_name`: The name of the variant to set /// * `variant`: The variant data /// pub fn new_with_index, V: Into>( - name: I, variant_index: usize, variant_name: I, variant: V, ) -> Self { Self { - name: name.into(), + represented_type: None, variant_index, variant_name: variant_name.into(), variant: variant.into(), } } - /// Returns the type name of the enum. - pub fn name(&self) -> &str { - &self.name - } + /// Sets the [type] to be represented by this `DynamicEnum`. + /// + /// # Panics + /// + /// Panics if the given [type] is not a [`TypeInfo::Enum`]. + /// + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::Enum(_)), + "expected TypeInfo::Enum but received: {:?}", + represented_type + ); + } - /// Sets the type name of the enum. - pub fn set_name(&mut self, name: String) { - self.name = name; + self.represented_type = represented_type; } /// Set the current enum variant represented by this struct. @@ -142,11 +141,11 @@ impl DynamicEnum { pub fn set_variant_with_index, V: Into>( &mut self, variant_index: usize, - name: I, + variant_name: I, variant: V, ) { self.variant_index = variant_index; - self.variant_name = name.into(); + self.variant_name = variant_name.into(); self.variant = variant.into(); } @@ -161,9 +160,9 @@ impl DynamicEnum { /// /// This is functionally the same as [`DynamicEnum::from`] except it takes a reference. pub fn from_ref(value: &TEnum) -> Self { - match value.variant_type() { + let type_info = value.get_represented_type_info(); + let mut dyn_enum = match value.variant_type() { VariantType::Unit => DynamicEnum::new_with_index( - value.type_name(), value.variant_index(), value.variant_name(), DynamicVariant::Unit, @@ -174,7 +173,6 @@ impl DynamicEnum { data.insert_boxed(field.value().clone_value()); } DynamicEnum::new_with_index( - value.type_name(), value.variant_index(), value.variant_name(), DynamicVariant::Tuple(data), @@ -187,13 +185,15 @@ impl DynamicEnum { data.insert_boxed(name, field.value().clone_value()); } DynamicEnum::new_with_index( - value.type_name(), value.variant_index(), value.variant_name(), DynamicVariant::Struct(data), ) } - } + }; + + dyn_enum.set_represented_type(type_info); + dyn_enum } } @@ -276,7 +276,7 @@ impl Enum for DynamicEnum { fn clone_dynamic(&self) -> DynamicEnum { Self { - name: self.name.clone(), + represented_type: self.represented_type, variant_index: self.variant_index, variant_name: self.variant_name.clone(), variant: self.variant.clone(), @@ -287,12 +287,14 @@ impl Enum for DynamicEnum { impl Reflect for DynamicEnum { #[inline] fn type_name(&self) -> &str { - &self.name + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_default() } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } #[inline] @@ -418,10 +420,3 @@ impl Reflect for DynamicEnum { write!(f, ")") } } - -impl Typed for DynamicEnum { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) - } -} diff --git a/crates/bevy_reflect/src/enums/mod.rs b/crates/bevy_reflect/src/enums/mod.rs index e8c32e73ef9c5..7693bf9794acc 100644 --- a/crates/bevy_reflect/src/enums/mod.rs +++ b/crates/bevy_reflect/src/enums/mod.rs @@ -342,14 +342,14 @@ mod tests { // === Tuple === // let mut data = DynamicTuple::default(); data.insert(1.23_f32); - let dyn_enum = DynamicEnum::new(std::any::type_name::>(), "B", data); + let dyn_enum = DynamicEnum::new("B", data); value.apply(&dyn_enum); assert_eq!(TestEnum::B(1.23), value); // === Struct === // let mut data = DynamicStruct::default(); data.insert("value", 1.23_f32); - let dyn_enum = DynamicEnum::new(std::any::type_name::>(), "C", data); + let dyn_enum = DynamicEnum::new("C", data); value.apply(&dyn_enum); assert_eq!(TestEnum::C { value: 1.23 }, value); } @@ -371,14 +371,14 @@ mod tests { // === Tuple === // let mut data = DynamicTuple::default(); data.insert(TestStruct(123)); - let dyn_enum = DynamicEnum::new(std::any::type_name::(), "B", data); + let dyn_enum = DynamicEnum::new("B", data); value.apply(&dyn_enum); assert_eq!(TestEnum::B(TestStruct(123)), value); // === Struct === // let mut data = DynamicStruct::default(); data.insert("value", TestStruct(123)); - let dyn_enum = DynamicEnum::new(std::any::type_name::(), "C", data); + let dyn_enum = DynamicEnum::new("C", data); value.apply(&dyn_enum); assert_eq!( TestEnum::C { @@ -409,14 +409,14 @@ mod tests { // === Tuple === // let mut data = DynamicTuple::default(); data.insert(OtherEnum::B(123)); - let dyn_enum = DynamicEnum::new(std::any::type_name::(), "B", data); + let dyn_enum = DynamicEnum::new("B", data); value.apply(&dyn_enum); assert_eq!(TestEnum::B(OtherEnum::B(123)), value); // === Struct === // let mut data = DynamicStruct::default(); data.insert("value", OtherEnum::C { value: 1.23 }); - let dyn_enum = DynamicEnum::new(std::any::type_name::(), "C", data); + let dyn_enum = DynamicEnum::new("C", data); value.apply(&dyn_enum); assert_eq!( TestEnum::C { diff --git a/crates/bevy_reflect/src/from_reflect.rs b/crates/bevy_reflect/src/from_reflect.rs index 4d98698398bf0..f4abb65d7e0f6 100644 --- a/crates/bevy_reflect/src/from_reflect.rs +++ b/crates/bevy_reflect/src/from_reflect.rs @@ -75,7 +75,7 @@ pub trait FromReflect: Reflect + Sized { /// # Example /// /// ``` -/// # use bevy_reflect::{DynamicTupleStruct, FromReflect, Reflect, ReflectFromReflect, TypeRegistry}; +/// # use bevy_reflect::{DynamicTupleStruct, FromReflect, Reflect, ReflectFromReflect, Typed, TypeRegistry}; /// # #[derive(Reflect, FromReflect, PartialEq, Eq, Debug)] /// # #[reflect(FromReflect)] /// # struct Foo(#[reflect(default = "default_value")] usize); @@ -84,7 +84,7 @@ pub trait FromReflect: Reflect + Sized { /// # registry.register::(); /// /// let mut reflected = DynamicTupleStruct::default(); -/// reflected.set_name(std::any::type_name::().to_string()); +/// reflected.set_represented_type(Some(::type_info())); /// /// let registration = registry.get_with_name(reflected.type_name()).unwrap(); /// let rfr = registration.data::().unwrap(); diff --git a/crates/bevy_reflect/src/impls/smallvec.rs b/crates/bevy_reflect/src/impls/smallvec.rs index 769c541d9d6cf..918dc849b62eb 100644 --- a/crates/bevy_reflect/src/impls/smallvec.rs +++ b/crates/bevy_reflect/src/impls/smallvec.rs @@ -82,8 +82,8 @@ where std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } fn into_any(self: Box) -> Box { diff --git a/crates/bevy_reflect/src/impls/std.rs b/crates/bevy_reflect/src/impls/std.rs index 0b42a707930b8..db8baa3d9d768 100644 --- a/crates/bevy_reflect/src/impls/std.rs +++ b/crates/bevy_reflect/src/impls/std.rs @@ -244,8 +244,8 @@ macro_rules! impl_reflect_for_veclike { std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } fn into_any(self: Box) -> Box { @@ -397,7 +397,7 @@ macro_rules! impl_reflect_for_hashmap { fn clone_dynamic(&self) -> DynamicMap { let mut dynamic_map = DynamicMap::default(); - dynamic_map.set_name(self.type_name().to_string()); + dynamic_map.set_represented_type(self.get_represented_type_info()); for (k, v) in self { let key = K::from_reflect(k).unwrap_or_else(|| { panic!("Attempted to clone invalid key of type {}.", k.type_name()) @@ -450,8 +450,8 @@ macro_rules! impl_reflect_for_hashmap { std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } fn into_any(self: Box) -> Box { @@ -595,8 +595,8 @@ impl Reflect for [T; N] { std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } #[inline] @@ -800,8 +800,8 @@ impl Reflect for Option { } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } #[inline] @@ -965,8 +965,8 @@ impl Reflect for Cow<'static, str> { std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } fn into_any(self: Box) -> Box { @@ -1073,8 +1073,8 @@ impl Reflect for &'static Path { std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } fn into_any(self: Box) -> Box { diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 571fce022afbf..a2ef6d4af6161 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -169,7 +169,7 @@ //! ``` //! # use bevy_reflect::{DynamicEnum, Reflect}; //! let mut value = Some(123_i32); -//! let patch = DynamicEnum::new(std::any::type_name::>(), "None", ()); +//! let patch = DynamicEnum::new("None", ()); //! value.apply(&patch); //! assert_eq!(None, value); //! ``` @@ -1100,7 +1100,7 @@ mod tests { // TypeInfo (instance) let value: &dyn Reflect = &123_i32; - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); // Struct @@ -1133,7 +1133,7 @@ mod tests { } let value: &dyn Reflect = &MyStruct { foo: 123, bar: 321 }; - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); // Struct (generic) @@ -1167,7 +1167,7 @@ mod tests { foo: String::from("Hello!"), bar: 321, }; - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::>()); // Tuple Struct @@ -1203,7 +1203,7 @@ mod tests { } let value: &dyn Reflect = &(123_u32, 1.23_f32, String::from("Hello!")); - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); // List @@ -1220,7 +1220,7 @@ mod tests { } let value: &dyn Reflect = &vec![123_usize]; - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); // List (SmallVec) @@ -1240,7 +1240,7 @@ mod tests { let value: MySmallVec = smallvec::smallvec![String::default(); 2]; let value: &dyn Reflect = &value; - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); } @@ -1259,7 +1259,7 @@ mod tests { } let value: &dyn Reflect = &[1usize, 2usize, 3usize]; - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); // Map @@ -1278,7 +1278,7 @@ mod tests { } let value: &dyn Reflect = &MyMap::new(); - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); // Value @@ -1293,23 +1293,23 @@ mod tests { } let value: &dyn Reflect = &String::from("Hello!"); - let info = value.get_type_info(); + let info = value.get_represented_type_info().unwrap(); assert!(info.is::()); + } - // Dynamic - type MyDynamic = DynamicList; - - let info = MyDynamic::type_info(); - if let TypeInfo::Dynamic(info) = info { - assert!(info.is::()); - assert_eq!(std::any::type_name::(), info.type_name()); - } else { - panic!("Expected `TypeInfo::Dynamic`"); - } + #[test] + fn should_permit_valid_represented_type_for_dynamic() { + let type_info = <[i32; 2] as Typed>::type_info(); + let mut dynamic_array = [123; 2].clone_dynamic(); + dynamic_array.set_represented_type(Some(type_info)); + } - let value: &dyn Reflect = &DynamicList::default(); - let info = value.get_type_info(); - assert!(info.is::()); + #[test] + #[should_panic(expected = "expected TypeInfo::Array but received")] + fn should_prohibit_invalid_represented_type_for_dynamic() { + let type_info = <(i32, i32) as Typed>::type_info(); + let mut dynamic_array = [123; 2].clone_dynamic(); + dynamic_array.set_represented_type(Some(type_info)); } #[cfg(feature = "documentation")] diff --git a/crates/bevy_reflect/src/list.rs b/crates/bevy_reflect/src/list.rs index d13e7776f0693..6b17527302f02 100644 --- a/crates/bevy_reflect/src/list.rs +++ b/crates/bevy_reflect/src/list.rs @@ -2,10 +2,8 @@ use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; use std::hash::{Hash, Hasher}; -use crate::utility::{reflect_hasher, NonGenericTypeInfoCell}; -use crate::{ - DynamicInfo, FromReflect, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, -}; +use crate::utility::reflect_hasher; +use crate::{FromReflect, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo}; /// A trait used to power [list-like] operations via [reflection]. /// @@ -97,7 +95,7 @@ pub trait List: Reflect { /// Clones the list, producing a [`DynamicList`]. fn clone_dynamic(&self) -> DynamicList { DynamicList { - name: self.type_name().to_string(), + represented_type: self.get_represented_type_info(), values: self.iter().map(|value| value.clone_value()).collect(), } } @@ -177,25 +175,27 @@ impl ListInfo { /// A list of reflected values. #[derive(Default)] pub struct DynamicList { - name: String, + represented_type: Option<&'static TypeInfo>, values: Vec>, } impl DynamicList { - /// Returns the type name of the list. + /// Sets the [type] to be represented by this `DynamicList`. + /// # Panics /// - /// The value returned by this method is the same value returned by - /// [`Reflect::type_name`]. - pub fn name(&self) -> &str { - &self.name - } - - /// Sets the type name of the list. + /// Panics if the given [type] is not a [`TypeInfo::List`]. /// - /// The value set by this method is the value returned by - /// [`Reflect::type_name`]. - pub fn set_name(&mut self, name: String) { - self.name = name; + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::List(_)), + "expected TypeInfo::List but received: {:?}", + represented_type + ); + } + + self.represented_type = represented_type; } /// Appends a typed value to the list. @@ -248,7 +248,7 @@ impl List for DynamicList { fn clone_dynamic(&self) -> DynamicList { DynamicList { - name: self.name.clone(), + represented_type: self.represented_type, values: self .values .iter() @@ -261,12 +261,14 @@ impl List for DynamicList { impl Reflect for DynamicList { #[inline] fn type_name(&self) -> &str { - self.name.as_str() + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_else(|| std::any::type_name::()) } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } #[inline] @@ -343,6 +345,11 @@ impl Reflect for DynamicList { list_debug(self, f)?; write!(f, ")") } + + #[inline] + fn is_dynamic(&self) -> bool { + true + } } impl Debug for DynamicList { @@ -351,13 +358,6 @@ impl Debug for DynamicList { } } -impl Typed for DynamicList { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) - } -} - impl IntoIterator for DynamicList { type Item = Box; type IntoIter = std::vec::IntoIter; diff --git a/crates/bevy_reflect/src/map.rs b/crates/bevy_reflect/src/map.rs index 8dcd28386a2e5..6e997ffd32e26 100644 --- a/crates/bevy_reflect/src/map.rs +++ b/crates/bevy_reflect/src/map.rs @@ -4,8 +4,7 @@ use std::hash::Hash; use bevy_utils::{Entry, HashMap}; -use crate::utility::NonGenericTypeInfoCell; -use crate::{DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed}; +use crate::{Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo}; /// A trait used to power [map-like] operations via [reflection]. /// @@ -184,26 +183,29 @@ const HASH_ERROR: &str = "the given key does not support hashing"; /// An ordered mapping between reflected values. #[derive(Default)] pub struct DynamicMap { - name: String, + represented_type: Option<&'static TypeInfo>, values: Vec<(Box, Box)>, indices: HashMap, } impl DynamicMap { - /// Returns the type name of the map. + /// Sets the [type] to be represented by this `DynamicMap`. /// - /// The value returned by this method is the same value returned by - /// [`Reflect::type_name`]. - pub fn name(&self) -> &str { - &self.name - } - - /// Sets the type name of the map. + /// # Panics + /// + /// Panics if the given [type] is not a [`TypeInfo::Map`]. /// - /// The value set by this method is the same value returned by - /// [`Reflect::type_name`]. - pub fn set_name(&mut self, name: String) { - self.name = name; + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::Map(_)), + "expected TypeInfo::Map but received: {:?}", + represented_type + ); + } + + self.represented_type = represented_type; } /// Inserts a typed key-value pair into the map. @@ -232,7 +234,7 @@ impl Map for DynamicMap { fn clone_dynamic(&self) -> DynamicMap { DynamicMap { - name: self.name.clone(), + represented_type: self.represented_type, values: self .values .iter() @@ -289,12 +291,14 @@ impl Map for DynamicMap { impl Reflect for DynamicMap { fn type_name(&self) -> &str { - &self.name + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_else(|| std::any::type_name::()) } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } fn into_any(self: Box) -> Box { @@ -358,6 +362,11 @@ impl Reflect for DynamicMap { map_debug(self, f)?; write!(f, ")") } + + #[inline] + fn is_dynamic(&self) -> bool { + true + } } impl Debug for DynamicMap { @@ -366,13 +375,6 @@ impl Debug for DynamicMap { } } -impl Typed for DynamicMap { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) - } -} - /// An iterator over the key-value pairs of a [`Map`]. pub struct MapIter<'a> { pub(crate) map: &'a dyn Map, diff --git a/crates/bevy_reflect/src/reflect.rs b/crates/bevy_reflect/src/reflect.rs index 8abfeef742fbd..c0749f5067caa 100644 --- a/crates/bevy_reflect/src/reflect.rs +++ b/crates/bevy_reflect/src/reflect.rs @@ -76,15 +76,22 @@ pub trait Reflect: Any + Send + Sync { /// Returns the [type name][std::any::type_name] of the underlying type. fn type_name(&self) -> &str; - /// Returns the [`TypeInfo`] of the underlying type. + /// Returns the [`TypeInfo`] of the type _represented_ by this value. + /// + /// For most types, this will simply return their own `TypeInfo`. + /// However, for dynamic types, such as [`DynamicStruct`] or [`DynamicList`], + /// this will return the type they represent + /// (or `None` if they don't represent any particular type). /// /// This method is great if you have an instance of a type or a `dyn Reflect`, /// and want to access its [`TypeInfo`]. However, if this method is to be called /// frequently, consider using [`TypeRegistry::get_type_info`] as it can be more /// performant for such use cases. /// + /// [`DynamicStruct`]: crate::DynamicStruct + /// [`DynamicList`]: crate::DynamicList /// [`TypeRegistry::get_type_info`]: crate::TypeRegistry::get_type_info - fn get_type_info(&self) -> &'static TypeInfo; + fn get_represented_type_info(&self) -> Option<&'static TypeInfo>; /// Returns the value as a [`Box`][std::any::Any]. fn into_any(self: Box) -> Box; @@ -216,6 +223,22 @@ pub trait Reflect: Any + Send + Sync { fn serializable(&self) -> Option { None } + + /// Indicates whether or not this type is a _dynamic_ type. + /// + /// Dynamic types include the ones built-in to this [crate], + /// such as [`DynamicStruct`], [`DynamicList`], and [`DynamicTuple`]. + /// However, they may be custom types used as proxies for other types + /// or to facilitate scripting capabilities. + /// + /// By default, this method will return `false`. + /// + /// [`DynamicStruct`]: crate::DynamicStruct + /// [`DynamicList`]: crate::DynamicList + /// [`DynamicTuple`]: crate::DynamicTuple + fn is_dynamic(&self) -> bool { + false + } } impl Debug for dyn Reflect { diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs index 60d3b0aa2047c..de7c47438104c 100644 --- a/crates/bevy_reflect/src/serde/de.rs +++ b/crates/bevy_reflect/src/serde/de.rs @@ -389,7 +389,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { registry: self.registry, }, )?; - dynamic_struct.set_name(struct_info.type_name().to_string()); + dynamic_struct.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_struct)) } TypeInfo::TupleStruct(tuple_struct_info) => { @@ -402,7 +402,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { registration: self.registration, }, )?; - dynamic_tuple_struct.set_name(tuple_struct_info.type_name().to_string()); + dynamic_tuple_struct.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_tuple_struct)) } TypeInfo::List(list_info) => { @@ -410,7 +410,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { list_info, registry: self.registry, })?; - dynamic_list.set_name(list_info.type_name().to_string()); + dynamic_list.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_list)) } TypeInfo::Array(array_info) => { @@ -421,7 +421,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { registry: self.registry, }, )?; - dynamic_array.set_name(array_info.type_name().to_string()); + dynamic_array.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_array)) } TypeInfo::Map(map_info) => { @@ -429,7 +429,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { map_info, registry: self.registry, })?; - dynamic_map.set_name(map_info.type_name().to_string()); + dynamic_map.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_map)) } TypeInfo::Tuple(tuple_info) => { @@ -440,7 +440,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { registry: self.registry, }, )?; - dynamic_tuple.set_name(tuple_info.type_name().to_string()); + dynamic_tuple.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_tuple)) } TypeInfo::Enum(enum_info) => { @@ -461,7 +461,7 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { }, )? }; - dynamic_enum.set_name(type_name.to_string()); + dynamic_enum.set_represented_type(Some(self.registration.type_info())); Ok(Box::new(dynamic_enum)) } TypeInfo::Value(_) => { @@ -470,13 +470,6 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> { "the TypeRegistration for {type_name} doesn't have ReflectDeserialize", ))) } - TypeInfo::Dynamic(_) => { - // We could potentially allow this but we'd have no idea what the actual types of the - // fields are and would rely on the deserializer to determine them (e.g. `i32` vs `i64`) - Err(de::Error::custom(format_args!( - "cannot deserialize arbitrary dynamic type {type_name}", - ))) - } } } } diff --git a/crates/bevy_reflect/src/serde/mod.rs b/crates/bevy_reflect/src/serde/mod.rs index 3355ed38d0b11..58231b2654ac2 100644 --- a/crates/bevy_reflect/src/serde/mod.rs +++ b/crates/bevy_reflect/src/serde/mod.rs @@ -92,4 +92,16 @@ mod tests { "Expected {expected:?} found {deserialized:?}" ); } + + #[test] + #[should_panic(expected = "cannot get type info for bevy_reflect::struct_trait::DynamicStruct")] + fn unproxied_dynamic_should_not_serialize() { + let registry = TypeRegistry::default(); + + let mut value = DynamicStruct::default(); + value.insert("foo", 123_u32); + + let serializer = ReflectSerializer::new(&value, ®istry); + ron::ser::to_string(&serializer).unwrap(); + } } diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index e9e4abe1d852d..38d4529a3a40b 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -43,26 +43,6 @@ fn get_serializable<'a, E: serde::ser::Error>( Ok(reflect_serialize.get_serializable(reflect_value)) } -/// Get the underlying [`TypeInfo`] of a given type. -/// -/// If the given type is a [`TypeInfo::Dynamic`] then we need to try and look -/// up the actual type in the registry. -fn get_type_info( - type_info: &'static TypeInfo, - type_name: &str, - registry: &TypeRegistry, -) -> Result<&'static TypeInfo, E> { - match type_info { - TypeInfo::Dynamic(..) => match registry.get_with_name(type_name) { - Some(registration) => Ok(registration.type_info()), - None => Err(Error::custom(format_args!( - "no registration found for dynamic type with name {type_name}", - ))), - }, - info => Ok(info), - } -} - /// A general purpose serializer for reflected types. /// /// The serialized data will take the form of a map containing the following entries: @@ -186,11 +166,15 @@ impl<'a> Serialize for StructSerializer<'a> { where S: serde::Serializer, { - let type_info = get_type_info( - self.struct_value.get_type_info(), - self.struct_value.type_name(), - self.registry, - )?; + let type_info = self + .struct_value + .get_represented_type_info() + .ok_or_else(|| { + Error::custom(format_args!( + "cannot get type info for {}", + self.struct_value.type_name() + )) + })?; let struct_info = match type_info { TypeInfo::Struct(struct_info) => struct_info, @@ -235,11 +219,15 @@ impl<'a> Serialize for TupleStructSerializer<'a> { where S: serde::Serializer, { - let type_info = get_type_info( - self.tuple_struct.get_type_info(), - self.tuple_struct.type_name(), - self.registry, - )?; + let type_info = self + .tuple_struct + .get_represented_type_info() + .ok_or_else(|| { + Error::custom(format_args!( + "cannot get type info for {}", + self.tuple_struct.type_name() + )) + })?; let tuple_struct_info = match type_info { TypeInfo::TupleStruct(tuple_struct_info) => tuple_struct_info, @@ -283,11 +271,12 @@ impl<'a> Serialize for EnumSerializer<'a> { where S: serde::Serializer, { - let type_info = get_type_info( - self.enum_value.get_type_info(), - self.enum_value.type_name(), - self.registry, - )?; + let type_info = self.enum_value.get_represented_type_info().ok_or_else(|| { + Error::custom(format_args!( + "cannot get type info for {}", + self.enum_value.type_name() + )) + })?; let enum_info = match type_info { TypeInfo::Enum(enum_info) => enum_info, diff --git a/crates/bevy_reflect/src/struct_trait.rs b/crates/bevy_reflect/src/struct_trait.rs index ab146d1b36a43..ede4ddec4d04d 100644 --- a/crates/bevy_reflect/src/struct_trait.rs +++ b/crates/bevy_reflect/src/struct_trait.rs @@ -1,7 +1,4 @@ -use crate::utility::NonGenericTypeInfoCell; -use crate::{ - DynamicInfo, NamedField, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, -}; +use crate::{NamedField, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo}; use bevy_utils::{Entry, HashMap}; use std::fmt::{Debug, Formatter}; use std::{ @@ -272,21 +269,30 @@ impl GetField for dyn Struct { /// A struct type which allows fields to be added at runtime. #[derive(Default)] pub struct DynamicStruct { - name: String, + represented_type: Option<&'static TypeInfo>, fields: Vec>, field_names: Vec>, field_indices: HashMap, usize>, } impl DynamicStruct { - /// Returns the type name of the struct. - pub fn name(&self) -> &str { - &self.name - } + /// Sets the [type] to be represented by this `DynamicStruct`. + /// + /// # Panics + /// + /// Panics if the given [type] is not a [`TypeInfo::Struct`]. + /// + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::Struct(_)), + "expected TypeInfo::Struct but received: {:?}", + represented_type + ); + } - /// Sets the type name of the struct. - pub fn set_name(&mut self, name: String) { - self.name = name; + self.represented_type = represented_type; } /// Inserts a field named `name` with value `value` into the struct. @@ -370,7 +376,7 @@ impl Struct for DynamicStruct { fn clone_dynamic(&self) -> DynamicStruct { DynamicStruct { - name: self.name.clone(), + represented_type: self.get_represented_type_info(), field_names: self.field_names.clone(), field_indices: self.field_indices.clone(), fields: self @@ -385,12 +391,14 @@ impl Struct for DynamicStruct { impl Reflect for DynamicStruct { #[inline] fn type_name(&self) -> &str { - &self.name + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_else(|| std::any::type_name::()) } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } #[inline] @@ -470,6 +478,11 @@ impl Reflect for DynamicStruct { struct_debug(self, f)?; write!(f, ")") } + + #[inline] + fn is_dynamic(&self) -> bool { + true + } } impl Debug for DynamicStruct { @@ -478,13 +491,6 @@ impl Debug for DynamicStruct { } } -impl Typed for DynamicStruct { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) - } -} - /// Compares a [`Struct`] with a [`Reflect`] value. /// /// Returns true if and only if all of the following are true: diff --git a/crates/bevy_reflect/src/tuple.rs b/crates/bevy_reflect/src/tuple.rs index f0e20aff6ba25..8f0340bcbb55e 100644 --- a/crates/bevy_reflect/src/tuple.rs +++ b/crates/bevy_reflect/src/tuple.rs @@ -1,9 +1,9 @@ -use crate::utility::NonGenericTypeInfoCell; use crate::{ - DynamicInfo, FromReflect, GetTypeRegistration, Reflect, ReflectMut, ReflectOwned, ReflectRef, - TypeInfo, TypeRegistration, Typed, UnnamedField, + FromReflect, GetTypeRegistration, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, + TypeRegistration, Typed, UnnamedField, }; use std::any::{Any, TypeId}; +use std::borrow::Cow; use std::fmt::{Debug, Formatter}; use std::slice::Iter; @@ -207,39 +207,48 @@ impl TupleInfo { /// A tuple which allows fields to be added at runtime. #[derive(Default, Debug)] pub struct DynamicTuple { - name: String, + name: Cow<'static, str>, + represented_type: Option<&'static TypeInfo>, fields: Vec>, } impl DynamicTuple { - /// Returns the type name of the tuple. + /// Sets the [type] to be represented by this `DynamicTuple`. /// - /// The tuple's name is automatically generated from its element types. - pub fn name(&self) -> &str { - &self.name - } - - /// Manually sets the type name of the tuple. + /// # Panics /// - /// Note that the tuple name will be overwritten when elements are added. - pub fn set_name(&mut self, name: String) { - self.name = name; + /// Panics if the given [type] is not a [`TypeInfo::Tuple`]. + /// + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::Tuple(_)), + "expected TypeInfo::Tuple but received: {:?}", + represented_type + ); + + self.name = Cow::Borrowed(represented_type.type_name()); + } + self.represented_type = represented_type; } /// Appends an element with value `value` to the tuple. pub fn insert_boxed(&mut self, value: Box) { + self.represented_type = None; self.fields.push(value); self.generate_name(); } /// Appends a typed element with value `value` to the tuple. pub fn insert(&mut self, value: T) { + self.represented_type = None; self.insert_boxed(Box::new(value)); self.generate_name(); } fn generate_name(&mut self) { - let name = &mut self.name; + let mut name = self.name.to_string(); name.clear(); name.push('('); for (i, field) in self.fields.iter().enumerate() { @@ -249,6 +258,7 @@ impl DynamicTuple { name.push_str(field.type_name()); } name.push(')'); + self.name = Cow::Owned(name); } } @@ -285,6 +295,7 @@ impl Tuple for DynamicTuple { fn clone_dynamic(&self) -> DynamicTuple { DynamicTuple { name: self.name.clone(), + represented_type: self.represented_type, fields: self .fields .iter() @@ -297,12 +308,14 @@ impl Tuple for DynamicTuple { impl Reflect for DynamicTuple { #[inline] fn type_name(&self) -> &str { - self.name() + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_else(|| &self.name) } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } #[inline] @@ -373,12 +386,10 @@ impl Reflect for DynamicTuple { tuple_debug(self, f)?; write!(f, ")") } -} -impl Typed for DynamicTuple { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) + #[inline] + fn is_dynamic(&self) -> bool { + true } } @@ -496,15 +507,15 @@ macro_rules! impl_reflect_tuple { #[inline] fn clone_dynamic(&self) -> DynamicTuple { - let mut dyn_tuple = DynamicTuple { - name: String::default(), + let info = self.get_represented_type_info(); + DynamicTuple { + name: Cow::Borrowed(::core::any::type_name::()), + represented_type: info, fields: self .iter_fields() .map(|value| value.clone_value()) .collect(), - }; - dyn_tuple.generate_name(); - dyn_tuple + } } } @@ -513,8 +524,8 @@ macro_rules! impl_reflect_tuple { std::any::type_name::() } - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + Some(::type_info()) } fn into_any(self: Box) -> Box { diff --git a/crates/bevy_reflect/src/tuple_struct.rs b/crates/bevy_reflect/src/tuple_struct.rs index 33edd7bb172e3..9b7466d5757ad 100644 --- a/crates/bevy_reflect/src/tuple_struct.rs +++ b/crates/bevy_reflect/src/tuple_struct.rs @@ -1,7 +1,4 @@ -use crate::utility::NonGenericTypeInfoCell; -use crate::{ - DynamicInfo, Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, Typed, UnnamedField, -}; +use crate::{Reflect, ReflectMut, ReflectOwned, ReflectRef, TypeInfo, UnnamedField}; use std::any::{Any, TypeId}; use std::fmt::{Debug, Formatter}; use std::slice::Iter; @@ -222,19 +219,28 @@ impl GetTupleStructField for dyn TupleStruct { /// A tuple struct which allows fields to be added at runtime. #[derive(Default)] pub struct DynamicTupleStruct { - name: String, + represented_type: Option<&'static TypeInfo>, fields: Vec>, } impl DynamicTupleStruct { - /// Returns the type name of the tuple struct. - pub fn name(&self) -> &str { - &self.name - } + /// Sets the [type] to be represented by this `DynamicTupleStruct`. + /// + /// # Panics + /// + /// Panics if the given [type] is not a [`TypeInfo::TupleStruct`]. + /// + /// [type]: TypeInfo + pub fn set_represented_type(&mut self, represented_type: Option<&'static TypeInfo>) { + if let Some(represented_type) = represented_type { + assert!( + matches!(represented_type, TypeInfo::TupleStruct(_)), + "expected TypeInfo::TupleStruct but received: {:?}", + represented_type + ); + } - /// Sets the type name of the tuple struct. - pub fn set_name(&mut self, name: String) { - self.name = name; + self.represented_type = represented_type; } /// Appends an element with value `value` to the tuple struct. @@ -274,7 +280,7 @@ impl TupleStruct for DynamicTupleStruct { fn clone_dynamic(&self) -> DynamicTupleStruct { DynamicTupleStruct { - name: self.name.clone(), + represented_type: self.represented_type, fields: self .fields .iter() @@ -287,12 +293,14 @@ impl TupleStruct for DynamicTupleStruct { impl Reflect for DynamicTupleStruct { #[inline] fn type_name(&self) -> &str { - self.name.as_str() + self.represented_type + .map(|info| info.type_name()) + .unwrap_or_else(|| std::any::type_name::()) } #[inline] - fn get_type_info(&self) -> &'static TypeInfo { - ::type_info() + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.represented_type } #[inline] @@ -371,6 +379,11 @@ impl Reflect for DynamicTupleStruct { tuple_struct_debug(self, f)?; write!(f, ")") } + + #[inline] + fn is_dynamic(&self) -> bool { + true + } } impl Debug for DynamicTupleStruct { @@ -379,13 +392,6 @@ impl Debug for DynamicTupleStruct { } } -impl Typed for DynamicTupleStruct { - fn type_info() -> &'static TypeInfo { - static CELL: NonGenericTypeInfoCell = NonGenericTypeInfoCell::new(); - CELL.get_or_set(|| TypeInfo::Dynamic(DynamicInfo::new::())) - } -} - /// Compares a [`TupleStruct`] with a [`Reflect`] value. /// /// Returns true if and only if all of the following are true: diff --git a/crates/bevy_reflect/src/type_info.rs b/crates/bevy_reflect/src/type_info.rs index 27df34ea767e3..c99beece3a9a4 100644 --- a/crates/bevy_reflect/src/type_info.rs +++ b/crates/bevy_reflect/src/type_info.rs @@ -2,6 +2,7 @@ use crate::{ ArrayInfo, EnumInfo, ListInfo, MapInfo, Reflect, StructInfo, TupleInfo, TupleStructInfo, }; use std::any::{Any, TypeId}; +use std::fmt::Debug; /// A static accessor to compile-time type information. /// @@ -49,7 +50,7 @@ use std::any::{Any, TypeId}; /// # /// # impl Reflect for MyStruct { /// # fn type_name(&self) -> &str { todo!() } -/// # fn get_type_info(&self) -> &'static TypeInfo { todo!() } +/// # fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { todo!() } /// # fn into_any(self: Box) -> Box { todo!() } /// # fn as_any(&self) -> &dyn Any { todo!() } /// # fn as_any_mut(&mut self) -> &mut dyn Any { todo!() } @@ -78,12 +79,12 @@ pub trait Typed: Reflect { /// Generally, for any given type, this value can be retrieved one of three ways: /// /// 1. [`Typed::type_info`] -/// 2. [`Reflect::get_type_info`] +/// 2. [`Reflect::get_represented_type_info`] /// 3. [`TypeRegistry::get_type_info`] /// /// Each return a static reference to [`TypeInfo`], but they all have their own use cases. /// For example, if you know the type at compile time, [`Typed::type_info`] is probably -/// the simplest. If all you have is a `dyn Reflect`, you'll probably want [`Reflect::get_type_info`]. +/// the simplest. If all you have is a `dyn Reflect`, you'll probably want [`Reflect::get_represented_type_info`]. /// Lastly, if all you have is a [`TypeId`] or [type name], you will need to go through /// [`TypeRegistry::get_type_info`]. /// @@ -91,7 +92,7 @@ pub trait Typed: Reflect { /// it can be more performant. This is because those other methods may require attaining a lock on /// the static [`TypeInfo`], while the registry simply checks a map. /// -/// [`Reflect::get_type_info`]: crate::Reflect::get_type_info +/// [`Reflect::get_represented_type_info`]: crate::Reflect::get_represented_type_info /// [`TypeRegistry::get_type_info`]: crate::TypeRegistry::get_type_info /// [`TypeId`]: std::any::TypeId /// [type name]: std::any::type_name @@ -105,10 +106,6 @@ pub enum TypeInfo { Map(MapInfo), Enum(EnumInfo), Value(ValueInfo), - /// Type information for "dynamic" types whose metadata can't be known at compile-time. - /// - /// This includes structs like [`DynamicStruct`](crate::DynamicStruct) and [`DynamicList`](crate::DynamicList). - Dynamic(DynamicInfo), } impl TypeInfo { @@ -123,7 +120,6 @@ impl TypeInfo { Self::Map(info) => info.type_id(), Self::Enum(info) => info.type_id(), Self::Value(info) => info.type_id(), - Self::Dynamic(info) => info.type_id(), } } @@ -140,7 +136,6 @@ impl TypeInfo { Self::Map(info) => info.type_name(), Self::Enum(info) => info.type_name(), Self::Value(info) => info.type_name(), - Self::Dynamic(info) => info.type_name(), } } @@ -161,7 +156,6 @@ impl TypeInfo { Self::Map(info) => info.docs(), Self::Enum(info) => info.docs(), Self::Value(info) => info.docs(), - Self::Dynamic(info) => info.docs(), } } } @@ -221,59 +215,3 @@ impl ValueInfo { self.docs } } - -/// A container for compile-time info related to Bevy's _dynamic_ types, including primitives. -/// -/// This is functionally the same as [`ValueInfo`], however, semantically it refers to dynamic -/// types such as [`DynamicStruct`], [`DynamicTuple`], [`DynamicList`], etc. -/// -/// [`DynamicStruct`]: crate::DynamicStruct -/// [`DynamicTuple`]: crate::DynamicTuple -/// [`DynamicList`]: crate::DynamicList -#[derive(Debug, Clone)] -pub struct DynamicInfo { - type_name: &'static str, - type_id: TypeId, - #[cfg(feature = "documentation")] - docs: Option<&'static str>, -} - -impl DynamicInfo { - pub fn new() -> Self { - Self { - type_name: std::any::type_name::(), - type_id: TypeId::of::(), - #[cfg(feature = "documentation")] - docs: None, - } - } - - /// Sets the docstring for this dynamic value. - #[cfg(feature = "documentation")] - pub fn with_docs(self, docs: Option<&'static str>) -> Self { - Self { docs, ..self } - } - - /// The [type name] of the dynamic value. - /// - /// [type name]: std::any::type_name - pub fn type_name(&self) -> &'static str { - self.type_name - } - - /// The [`TypeId`] of the dynamic value. - pub fn type_id(&self) -> TypeId { - self.type_id - } - - /// Check if the given type matches the dynamic value type. - pub fn is(&self) -> bool { - TypeId::of::() == self.type_id - } - - /// The docstring of this value, if any. - #[cfg(feature = "documentation")] - pub fn docs(&self) -> Option<&'static str> { - self.docs - } -} diff --git a/crates/bevy_reflect/src/utility.rs b/crates/bevy_reflect/src/utility.rs index 3af535a1925ca..7c203631aa785 100644 --- a/crates/bevy_reflect/src/utility.rs +++ b/crates/bevy_reflect/src/utility.rs @@ -39,7 +39,7 @@ use std::{ /// # /// # impl Reflect for Foo { /// # fn type_name(&self) -> &str { todo!() } -/// # fn get_type_info(&self) -> &'static TypeInfo { todo!() } +/// # fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { todo!() } /// # fn into_any(self: Box) -> Box { todo!() } /// # fn as_any(&self) -> &dyn Any { todo!() } /// # fn as_any_mut(&mut self) -> &mut dyn Any { todo!() } @@ -102,7 +102,7 @@ impl NonGenericTypeInfoCell { /// # /// # impl Reflect for Foo { /// # fn type_name(&self) -> &str { todo!() } -/// # fn get_type_info(&self) -> &'static TypeInfo { todo!() } +/// # fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { todo!() } /// # fn into_any(self: Box) -> Box { todo!() } /// # fn as_any(&self) -> &dyn Any { todo!() } /// # fn as_any_mut(&mut self) -> &mut dyn Any { todo!() }