From 5c651f8692b397683b643999c04625a069fc53ff Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Thu, 15 Aug 2024 11:47:18 -0700 Subject: [PATCH 1/9] Added ReflectBox --- crates/bevy_reflect/src/boxed.rs | 636 ++++++++++++++++++++++++++++++ crates/bevy_reflect/src/lib.rs | 1 + crates/bevy_reflect/src/remote.rs | 7 +- 3 files changed, 641 insertions(+), 3 deletions(-) create mode 100644 crates/bevy_reflect/src/boxed.rs diff --git a/crates/bevy_reflect/src/boxed.rs b/crates/bevy_reflect/src/boxed.rs new file mode 100644 index 0000000000000..f9858b7626971 --- /dev/null +++ b/crates/bevy_reflect/src/boxed.rs @@ -0,0 +1,636 @@ +use core::fmt::Formatter; +use std::any::Any; + +use crate::__macro_exports::RegisterForReflection; +use crate::serde::Serializable; +use crate::utility::GenericTypePathCell; +use crate::{ + ApplyError, MaybeTyped, PartialReflect, Reflect, ReflectKind, ReflectMut, ReflectOwned, + ReflectRef, ReflectRemote, TypeInfo, TypePath, +}; + +/// A trait used to access `Self` as a [`dyn PartialReflect`]. +/// +/// This is used to provide by [`ReflectBox`] in order to remotely reflect +/// the inner type, `T`. +/// +/// This trait can be implemented on custom trait objects to allow them to be remotely reflected +/// by [`ReflectBox`]. +/// +/// [`dyn PartialReflect`]: PartialReflect +#[doc(hidden)] +pub trait ToPartialReflect: Send + Sync + 'static { + /// Get a reference to `Self` as a [`&dyn PartialReflect`](PartialReflect). + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect; + /// Get a mutable reference to `Self` as a [`&mut dyn PartialReflect`](PartialReflect). + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect; + /// Take `Self` as a [`Box`](PartialReflect). + fn to_partial_reflect_box(self: Box) -> Box; +} + +impl ToPartialReflect for T { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self + } + + fn to_partial_reflect_box(self: Box) -> Box { + self + } +} + +impl ToPartialReflect for dyn PartialReflect { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self + } + + fn to_partial_reflect_box(self: Box) -> Box { + self + } +} + +/// A trait used to access `Self` as a [`dyn Reflect`]. +/// +/// This is used to provide by [`ReflectBox`] in order to remotely reflect +/// the inner type, `T`. +/// +/// This trait can be implemented on custom trait objects to allow them to be remotely reflected +/// by [`ReflectBox`]. +/// +/// [`dyn Reflect`]: Reflect +#[doc(hidden)] +pub trait ToReflect: ToPartialReflect { + /// Get a reference to `Self` as a [`&dyn Reflect`](Reflect). + fn to_reflect_ref(&self) -> &dyn Reflect; + /// Get a mutable reference to `Self` as a [`&mut dyn Reflect`](Reflect). + fn to_reflect_mut(&mut self) -> &mut dyn Reflect; + /// Take `Self` as a [`Box`](Reflect). + fn to_reflect_box(self: Box) -> Box; +} + +impl ToReflect for T { + fn to_reflect_ref(&self) -> &dyn Reflect { + self + } + + fn to_reflect_mut(&mut self) -> &mut dyn Reflect { + self + } + + fn to_reflect_box(self: Box) -> Box { + self + } +} + +impl ToPartialReflect for dyn Reflect { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self.as_partial_reflect() + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self.as_partial_reflect_mut() + } + + fn to_partial_reflect_box(self: Box) -> Box { + self.into_partial_reflect() + } +} + +impl ToReflect for dyn Reflect { + fn to_reflect_ref(&self) -> &dyn Reflect { + self + } + + fn to_reflect_mut(&mut self) -> &mut dyn Reflect { + self + } + + fn to_reflect_box(self: Box) -> Box { + self + } +} + +/// A [remote wrapper] type around [`Box`] where `T` is either a type that implements [`PartialReflect`]/[`Reflect`] +/// or a [`dyn PartialReflect`]/[`dyn Reflect`] trait object. +/// +/// `Box` is not itself reflectable due to high likelihood of accidentally double-boxing it +/// (i.e. accidentally calling `Box::new` on an already-reflectable `Box`). +/// +/// [`ReflectBox`] should never be created or used directly outside remote reflection, +/// in order to avoid double-boxing. +/// +/// # Examples +/// +/// ``` +/// # use bevy_reflect::Reflect; +/// use bevy_reflect::boxed::ReflectBox; +/// +/// #[derive(Reflect)] +/// struct Sword { +/// attack: u32 +/// } +/// +/// #[derive(Reflect)] +/// // Keep in mind that `ReflectBox` does not implement `FromReflect`. +/// // Because of this, we will need to opt-out of the automatic `FromReflect` impl: +/// #[reflect(from_reflect = false)] +/// struct Player { +/// // Tell the `Reflect` derive to remotely reflect our `Box` +/// // using the `ReflectBox` wrapper type: +/// #[reflect(remote = ReflectBox)] +/// weapon: Box +/// } +/// +/// let player = Player { +/// // We can create our `Box` as normal: +/// weapon: Box::new(Sword { attack: 100 }) +/// }; +/// +/// // Now we can reflect `weapon`! +/// // Under the hood, this `dyn Reflect` is actually a `ReflectBox`, +/// // and is automatically delegating all reflection methods to the inner `dyn Reflect`. +/// // It will automatically convert to and from `ReflectBox` as needed. +/// let weapon: &dyn Reflect = player.weapon.as_reflect(); +/// assert!(weapon.reflect_partial_eq(&Sword { attack: 100 }).unwrap_or_default()); +/// ``` +/// +/// [remote wrapper]: ReflectRemote +/// [`dyn PartialReflect`]: PartialReflect +/// [`dyn Reflect`]: Reflect +#[repr(transparent)] +pub struct ReflectBox(Box); + +impl ReflectRemote for ReflectBox { + type Remote = Box; + + fn as_remote(&self) -> &Self::Remote { + &self.0 + } + + fn as_remote_mut(&mut self) -> &mut Self::Remote { + &mut self.0 + } + + fn into_remote(self) -> Self::Remote { + self.0 + } + + fn as_wrapper(remote: &Self::Remote) -> &Self { + // SAFETY: The wrapper type should be repr(transparent) over the remote type + #[allow(unsafe_code)] + unsafe { + core::mem::transmute::<&Self::Remote, &Self>(remote) + } + } + + fn as_wrapper_mut(remote: &mut Self::Remote) -> &mut Self { + // SAFETY: The wrapper type should be repr(transparent) over the remote type + #[allow(unsafe_code)] + unsafe { + core::mem::transmute::<&mut Self::Remote, &mut Self>(remote) + } + } + + fn into_wrapper(remote: Self::Remote) -> Self { + // SAFETY: The wrapper type should be repr(transparent) over the remote type + #[allow(unsafe_code)] + unsafe { + // Unfortunately, we have to use `transmute_copy` to avoid a compiler error: + // ``` + // error[E0512]: cannot transmute between types of different sizes, or dependently-sized types + // | + // | std::mem::transmute::(a) + // | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + // | + // = note: source type: `A` (this type does not have a fixed size) + // = note: target type: `B` (this type does not have a fixed size) + // ``` + core::mem::transmute_copy::( + // `ManuallyDrop` is used to prevent double-dropping `self` + &core::mem::ManuallyDrop::new(remote), + ) + } + } +} + +/// All methods in this implementation are delegated to the inner type, `T`. +impl PartialReflect for ReflectBox { + fn get_represented_type_info(&self) -> Option<&'static TypeInfo> { + self.0.to_partial_reflect_ref().get_represented_type_info() + } + + fn into_partial_reflect(self: Box) -> Box { + self.0.to_partial_reflect_box() + } + + fn as_partial_reflect(&self) -> &dyn PartialReflect { + self.0.to_partial_reflect_ref() + } + + fn as_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self.0.to_partial_reflect_mut() + } + + fn try_into_reflect(self: Box) -> Result, Box> { + self.0.to_partial_reflect_box().try_into_reflect() + } + + fn try_as_reflect(&self) -> Option<&dyn Reflect> { + self.0.to_partial_reflect_ref().try_as_reflect() + } + + fn try_as_reflect_mut(&mut self) -> Option<&mut dyn Reflect> { + self.0.to_partial_reflect_mut().try_as_reflect_mut() + } + + fn apply(&mut self, value: &dyn PartialReflect) { + self.0.to_partial_reflect_mut().apply(value); + } + + fn try_apply(&mut self, value: &dyn PartialReflect) -> Result<(), ApplyError> { + self.0.to_partial_reflect_mut().try_apply(value) + } + + fn reflect_kind(&self) -> ReflectKind { + self.0.to_partial_reflect_ref().reflect_kind() + } + + fn reflect_ref(&self) -> ReflectRef { + self.0.to_partial_reflect_ref().reflect_ref() + } + + fn reflect_mut(&mut self) -> ReflectMut { + self.0.to_partial_reflect_mut().reflect_mut() + } + + fn reflect_owned(self: Box) -> ReflectOwned { + self.0.to_partial_reflect_box().reflect_owned() + } + + fn clone_value(&self) -> Box { + self.0.to_partial_reflect_ref().clone_value() + } + + fn reflect_hash(&self) -> Option { + self.0.to_partial_reflect_ref().reflect_hash() + } + + fn reflect_partial_eq(&self, value: &dyn PartialReflect) -> Option { + self.0.to_partial_reflect_ref().reflect_partial_eq(value) + } + + fn debug(&self, f: &mut Formatter<'_>) -> core::fmt::Result { + write!(f, "ReflectBox(")?; + self.0.to_partial_reflect_ref().debug(f)?; + write!(f, ")") + } + + fn serializable(&self) -> Option { + self.0.to_partial_reflect_ref().serializable() + } + + fn is_dynamic(&self) -> bool { + self.0.to_partial_reflect_ref().is_dynamic() + } +} + +/// All methods in this implementation are delegated to the inner type, `T`. +impl Reflect for ReflectBox { + fn into_any(self: Box) -> Box { + self.0.to_reflect_box().into_any() + } + + fn as_any(&self) -> &dyn Any { + self.0.to_reflect_ref().as_any() + } + + fn as_any_mut(&mut self) -> &mut dyn Any { + self.0.to_reflect_mut().as_any_mut() + } + + fn into_reflect(self: Box) -> Box { + self.0.to_reflect_box() + } + + fn as_reflect(&self) -> &dyn Reflect { + self.0.to_reflect_ref() + } + + fn as_reflect_mut(&mut self) -> &mut dyn Reflect { + self.0.to_reflect_mut() + } + + fn set(&mut self, value: Box) -> Result<(), Box> { + self.0.to_reflect_mut().set(value) + } +} + +/// For most reflection methods, [`ReflectBox`] will delegate its behavior to the type, `T`, that it wraps. +/// However, due to the static nature of [`TypePath`], [`ReflectBox`] must provide its own implementation. +/// +/// # Examples +/// +/// ``` +/// # use bevy_reflect::boxed::ReflectBox; +/// # use bevy_reflect::{PartialReflect, TypePath}; +/// // Notice that we don't delegate to the `TypePath` implementation for `String`: +/// let type_path = as TypePath>::type_path(); +/// assert_eq!(type_path, "bevy_reflect::boxed::ReflectBox"); +/// +/// // The same is true for trait object types like `dyn PartialReflect`: +/// let type_path = as TypePath>::type_path(); +/// assert_eq!(type_path, "bevy_reflect::boxed::ReflectBox"); +/// ``` +impl TypePath for ReflectBox { + fn type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| { + "bevy_reflect::boxed::ReflectBox<".to_owned() + T::type_path() + ">" + }) + } + + fn short_type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| "ReflectBox<".to_owned() + T::short_type_path() + ">") + } + + fn type_ident() -> Option<&'static str> { + Some("ReflectBox") + } + + fn crate_name() -> Option<&'static str> { + Some("bevy_reflect") + } + + fn module_path() -> Option<&'static str> { + Some("bevy_reflect::boxed") + } +} + +impl MaybeTyped for ReflectBox {} +impl RegisterForReflection for ReflectBox {} + +#[cfg(feature = "functions")] +mod func { + use super::*; + use crate::func::args::{Arg, FromArg, GetOwnership, Ownership}; + use crate::func::{ArgError, IntoReturn, Return}; + + impl GetOwnership for ReflectBox { + fn ownership() -> Ownership { + Ownership::Owned + } + } + + impl GetOwnership for &'_ ReflectBox { + fn ownership() -> Ownership { + Ownership::Ref + } + } + + impl GetOwnership for &'_ mut ReflectBox { + fn ownership() -> Ownership { + Ownership::Mut + } + } + + impl FromArg for ReflectBox { + type This<'a> = ReflectBox; + + fn from_arg(arg: Arg) -> Result, ArgError> { + arg.take_owned() + } + } + + impl FromArg for &'static ReflectBox { + type This<'a> = &'a ReflectBox; + + fn from_arg(arg: Arg) -> Result, ArgError> { + arg.take_ref() + } + } + + impl FromArg for &'static mut ReflectBox { + type This<'a> = &'a mut ReflectBox; + + fn from_arg(arg: Arg) -> Result, ArgError> { + arg.take_mut() + } + } + + impl IntoReturn for ReflectBox { + fn into_return<'into_return>(self) -> Return<'into_return> + where + Self: 'into_return, + { + Return::Owned(Box::new(self)) + } + } + + impl IntoReturn for &ReflectBox { + fn into_return<'into_return>(self) -> Return<'into_return> + where + Self: 'into_return, + { + Return::Ref(self) + } + } + + impl IntoReturn for &mut ReflectBox { + fn into_return<'into_return>(self) -> Return<'into_return> + where + Self: 'into_return, + { + Return::Mut(self) + } + } +} + +#[cfg(test)] +mod tests { + use crate as bevy_reflect; + use crate::{DynamicStruct, FromReflect, GetTypeRegistration, Struct, Typed}; + use static_assertions::assert_not_impl_any; + + use super::*; + + #[test] + fn box_should_not_be_reflect() { + assert_not_impl_any!(Box: PartialReflect, Reflect, FromReflect, TypePath, Typed, GetTypeRegistration); + } + + #[test] + fn should_reflect_box() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct Container { + #[reflect(remote = ReflectBox)] + partial_reflect: Box, + #[reflect(remote = ReflectBox)] + full_reflect: Box, + #[reflect(remote = ReflectBox)] + concrete: Box, + } + + let mut container: Box = Box::new(Container { + partial_reflect: Box::new(123), + full_reflect: Box::new(456), + concrete: Box::new(789), + }); + + let partial_reflect = container.field("partial_reflect").unwrap(); + assert!(partial_reflect.reflect_partial_eq(&123).unwrap_or_default()); + + let full_reflect = container.field("full_reflect").unwrap(); + assert!(full_reflect.reflect_partial_eq(&456).unwrap_or_default()); + + let concrete = container.field("concrete").unwrap(); + assert!(concrete.reflect_partial_eq(&789).unwrap_or_default()); + + let mut patch = DynamicStruct::default(); + patch.insert("partial_reflect", ReflectBox(Box::new(321))); + patch.insert("full_reflect", ReflectBox(Box::new(654))); + patch.insert("concrete", ReflectBox(Box::new(987))); + + container.apply(&patch); + + let partial_reflect = container.field("partial_reflect").unwrap(); + assert!(partial_reflect.reflect_partial_eq(&321).unwrap_or_default()); + + let full_reflect = container.field("full_reflect").unwrap(); + assert!(full_reflect.reflect_partial_eq(&654).unwrap_or_default()); + + let concrete = container.field("concrete").unwrap(); + assert!(concrete.reflect_partial_eq(&987).unwrap_or_default()); + } + + #[test] + fn should_debug_reflect_box() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct Container { + #[reflect(remote = ReflectBox)] + partial_reflect: Box, + #[reflect(remote = ReflectBox)] + full_reflect: Box, + #[reflect(remote = ReflectBox)] + concrete: Box, + } + + let container: Box = Box::new(Container { + partial_reflect: Box::new(123), + full_reflect: Box::new(456), + concrete: Box::new(789), + }); + + let debug = format!("{:?}", container.as_partial_reflect()); + assert_eq!( + debug, + "bevy_reflect::boxed::tests::Container { partial_reflect: ReflectBox(123), full_reflect: ReflectBox(456), concrete: ReflectBox(789) }" + ); + + // Double-boxing should be near impossible for a user to create intentionally or unintentionally. + // However, we'll still test it here so that we can keep track of its behavior. + let double_box: Box = + Box::new(ReflectBox(Box::new(ReflectBox(Box::new(123))))); + + let deref_debug = format!("{:?}", &*double_box); + assert_eq!(deref_debug, "ReflectBox(ReflectBox(123))"); + + let as_partial_reflect_debug = format!("{:?}", double_box.as_partial_reflect()); + assert_eq!(as_partial_reflect_debug, "ReflectBox(123)"); + } + + #[test] + fn should_avoid_double_boxing() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct Container { + #[reflect(remote = ReflectBox)] + value: Box, + } + + let container = Box::new(Container { + value: Box::new(Some(123)), + }); + + let cloned = container.clone_value().clone_value(); + + let ReflectRef::Struct(cloned) = cloned.reflect_ref() else { + panic!("expected `ReflectRef::Struct`"); + }; + + let value = cloned.field("value").unwrap(); + let debug = format!("{:?}", value); + assert_eq!(debug, "DynamicEnum(Some(123))"); + + // Cloning a `ReflectBox` directly. Users should never have an instance of `ReflectBox`. + let value = ReflectBox(Box::new(Some(123))); + let cloned = value.clone_value(); + let debug = format!("{:?}", cloned); + assert_eq!(debug, "DynamicEnum(Some(123))"); + + // Cloning a boxed `ReflectBox`. Users should never have an instance of `ReflectBox`. + let value = Box::new(ReflectBox(Box::new(Some(123)))); + let cloned = value.clone_value(); + let debug = format!("{:?}", cloned); + assert_eq!(debug, "DynamicEnum(Some(123))"); + } + + #[test] + fn should_allow_custom_trait_objects() { + trait Equippable: Reflect {} + + impl TypePath for dyn Equippable { + fn type_path() -> &'static str { + todo!() + } + + fn short_type_path() -> &'static str { + todo!() + } + } + + impl ToPartialReflect for dyn Equippable { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self.as_partial_reflect() + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self.as_partial_reflect_mut() + } + + fn to_partial_reflect_box(self: Box) -> Box { + self.into_partial_reflect() + } + } + + impl ToReflect for dyn Equippable { + fn to_reflect_ref(&self) -> &dyn Reflect { + self.as_reflect() + } + + fn to_reflect_mut(&mut self) -> &mut dyn Reflect { + self.as_reflect_mut() + } + + fn to_reflect_box(self: Box) -> Box { + self.into_reflect() + } + } + + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct Player { + #[reflect(remote = ReflectBox)] + weapon: Box, + } + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index fca875b70b851..327ac4cf1f930 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -526,6 +526,7 @@ //! [derive `Reflect`]: derive@crate::Reflect mod array; +pub mod boxed; mod fields; mod from_reflect; #[cfg(feature = "functions")] diff --git a/crates/bevy_reflect/src/remote.rs b/crates/bevy_reflect/src/remote.rs index a19d3bdb148d6..d0d5416cbd3ee 100644 --- a/crates/bevy_reflect/src/remote.rs +++ b/crates/bevy_reflect/src/remote.rs @@ -1,4 +1,4 @@ -use crate::Reflect; +use crate::PartialReflect; /// Marks a type as a [reflectable] wrapper for a remote type. /// @@ -40,11 +40,12 @@ use crate::Reflect; /// } /// ``` /// -/// [reflectable]: Reflect +/// [reflectable]: PartialReflect /// [`transmute`]: core::mem::transmute /// [very unsafe]: https://doc.rust-lang.org/1.71.0/nomicon/transmutes.html +/// [`Reflect`]: crate::Reflect /// [`FromReflect`]: crate::FromReflect -pub trait ReflectRemote: Reflect { +pub trait ReflectRemote: PartialReflect { /// The remote type this type represents via reflection. type Remote; From 015d556b242baac52e13427ee029eadfa7f53747 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Thu, 15 Aug 2024 15:08:23 -0700 Subject: [PATCH 2/9] Make ReflectBox serializable --- crates/bevy_reflect/src/boxed.rs | 159 ++++++++++++++++++++++++++- crates/bevy_reflect/src/serde/de.rs | 97 ++++++++++------ crates/bevy_reflect/src/serde/ser.rs | 65 ++++++++++- 3 files changed, 280 insertions(+), 41 deletions(-) diff --git a/crates/bevy_reflect/src/boxed.rs b/crates/bevy_reflect/src/boxed.rs index f9858b7626971..3c396e02697e5 100644 --- a/crates/bevy_reflect/src/boxed.rs +++ b/crates/bevy_reflect/src/boxed.rs @@ -455,12 +455,13 @@ mod func { #[cfg(test)] mod tests { + use super::*; use crate as bevy_reflect; - use crate::{DynamicStruct, FromReflect, GetTypeRegistration, Struct, Typed}; + use crate::serde::{ReflectDeserializer, ReflectSerializer}; + use crate::{DynamicStruct, FromReflect, GetTypeRegistration, Struct, TypeRegistry, Typed}; + use serde::de::DeserializeSeed; use static_assertions::assert_not_impl_any; - use super::*; - #[test] fn box_should_not_be_reflect() { assert_not_impl_any!(Box: PartialReflect, Reflect, FromReflect, TypePath, Typed, GetTypeRegistration); @@ -633,4 +634,156 @@ mod tests { weapon: Box, } } + + #[test] + fn should_serialize_reflect_box() { + let input = ReflectBox(Box::new(123)); + + let registry = TypeRegistry::new(); + + let reflect_serializer = ReflectSerializer::new(&input, ®istry); + let serialized = ron::to_string(&reflect_serializer).unwrap(); + assert_eq!(serialized, r#"{"i32":123}"#); + + let reflect_deserializer = ReflectDeserializer::new(®istry); + let output = reflect_deserializer + .deserialize(&mut ron::Deserializer::from_str(&serialized).unwrap()) + .unwrap(); + assert!( + output + .as_partial_reflect() + .reflect_partial_eq(&input) + .unwrap_or_default(), + "serialization roundtrip should be lossless" + ); + } + + #[test] + fn should_serialize_reflect_box_struct() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct MyStruct { + #[reflect(remote = ReflectBox)] + partial_reflect: Box, + #[reflect(remote = ReflectBox)] + full_reflect: Box, + #[reflect(remote = ReflectBox)] + concrete: Box, + } + + let input = MyStruct { + partial_reflect: Box::new(123), + full_reflect: Box::new(456), + concrete: Box::new(789), + }; + + let mut registry = TypeRegistry::new(); + registry.register::(); + + let reflect_serializer = ReflectSerializer::new(&input, ®istry); + let serialized = ron::to_string(&reflect_serializer).unwrap(); + assert_eq!( + serialized, + r#"{"bevy_reflect::boxed::tests::MyStruct":(partial_reflect:{"i32":123},full_reflect:{"i32":456},concrete:{"i32":789})}"# + ); + + let reflect_deserializer = ReflectDeserializer::new(®istry); + let output = reflect_deserializer + .deserialize(&mut ron::Deserializer::from_str(&serialized).unwrap()) + .unwrap(); + assert!( + output + .as_partial_reflect() + .reflect_partial_eq(&input) + .unwrap_or_default(), + "serialization roundtrip should be lossless" + ); + } + + #[test] + fn should_serialize_reflect_box_tuple_struct() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct MyTupleStruct( + #[reflect(remote = ReflectBox )] Box, + #[reflect(remote = ReflectBox )] Box, + #[reflect(remote = ReflectBox )] Box, + ); + + let input = MyTupleStruct(Box::new(123), Box::new(456), Box::new(789)); + + let mut registry = TypeRegistry::new(); + registry.register::(); + + let reflect_serializer = ReflectSerializer::new(&input, ®istry); + let serialized = ron::to_string(&reflect_serializer).unwrap(); + assert_eq!( + serialized, + r#"{"bevy_reflect::boxed::tests::MyTupleStruct":({"i32":123},{"i32":456},{"i32":789})}"# + ); + + let reflect_deserializer = ReflectDeserializer::new(®istry); + let output = reflect_deserializer + .deserialize(&mut ron::Deserializer::from_str(&serialized).unwrap()) + .unwrap(); + assert!( + output + .as_partial_reflect() + .reflect_partial_eq(&input) + .unwrap_or_default(), + "serialization roundtrip should be lossless" + ); + } + + #[test] + fn should_serialize_nested_reflect_box() { + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct MyStruct { + #[reflect(remote = ReflectBox)] + partial_reflect: Box, + #[reflect(remote = ReflectBox)] + full_reflect: Box, + #[reflect(remote = ReflectBox)] + concrete: Box, + } + + #[derive(Reflect)] + #[reflect(from_reflect = false)] + struct MyNestedStruct { + #[reflect(remote = ReflectBox)] + inner: Box, + } + + let input = MyNestedStruct { + inner: Box::new(MyStruct { + partial_reflect: Box::new(123), + full_reflect: Box::new(456), + concrete: Box::new(789), + }), + }; + + let mut registry = TypeRegistry::new(); + registry.register::(); + registry.register::(); + + let reflect_serializer = ReflectSerializer::new(&input, ®istry); + let serialized = ron::to_string(&reflect_serializer).unwrap(); + assert_eq!( + serialized, + r#"{"bevy_reflect::boxed::tests::MyNestedStruct":(inner:{"bevy_reflect::boxed::tests::MyStruct":(partial_reflect:{"i32":123},full_reflect:{"i32":456},concrete:{"i32":789})})}"# + ); + + let reflect_deserializer = ReflectDeserializer::new(®istry); + let output = reflect_deserializer + .deserialize(&mut ron::Deserializer::from_str(&serialized).unwrap()) + .unwrap(); + assert!( + output + .as_partial_reflect() + .reflect_partial_eq(&input) + .unwrap_or_default(), + "serialization roundtrip should be lossless" + ); + } } diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs index 26fff47100e48..fae184dbd700a 100644 --- a/crates/bevy_reflect/src/serde/de.rs +++ b/crates/bevy_reflect/src/serde/de.rs @@ -35,11 +35,11 @@ trait TupleLikeInfo { } trait Container { - fn get_field_registration<'a, E: Error>( + fn get_field_deserializer<'a, E: Error>( &self, index: usize, registry: &'a TypeRegistry, - ) -> Result<&'a TypeRegistration, E>; + ) -> Result, E>; } impl StructLikeInfo for StructInfo { @@ -61,11 +61,11 @@ impl StructLikeInfo for StructInfo { } impl Container for StructInfo { - fn get_field_registration<'a, E: Error>( + fn get_field_deserializer<'a, E: Error>( &self, index: usize, registry: &'a TypeRegistry, - ) -> Result<&'a TypeRegistration, E> { + ) -> Result, E> { let field = self.field_at(index).ok_or_else(|| { Error::custom(format_args!( "no field at index {} on struct {}", @@ -73,7 +73,7 @@ impl Container for StructInfo { self.type_path(), )) })?; - get_registration(field.type_id(), field.type_path(), registry) + get_deserializer(field.type_id(), field.type_path(), registry) } } @@ -96,11 +96,11 @@ impl StructLikeInfo for StructVariantInfo { } impl Container for StructVariantInfo { - fn get_field_registration<'a, E: Error>( + fn get_field_deserializer<'a, E: Error>( &self, index: usize, registry: &'a TypeRegistry, - ) -> Result<&'a TypeRegistration, E> { + ) -> Result, E> { let field = self.field_at(index).ok_or_else(|| { Error::custom(format_args!( "no field at index {} on variant {}", @@ -108,7 +108,7 @@ impl Container for StructVariantInfo { self.name(), )) })?; - get_registration(field.type_id(), field.type_path(), registry) + get_deserializer(field.type_id(), field.type_path(), registry) } } @@ -119,11 +119,11 @@ impl TupleLikeInfo for TupleInfo { } impl Container for TupleInfo { - fn get_field_registration<'a, E: Error>( + fn get_field_deserializer<'a, E: Error>( &self, index: usize, registry: &'a TypeRegistry, - ) -> Result<&'a TypeRegistration, E> { + ) -> Result, E> { let field = self.field_at(index).ok_or_else(|| { Error::custom(format_args!( "no field at index {} on tuple {}", @@ -131,7 +131,10 @@ impl Container for TupleInfo { self.type_path(), )) })?; - get_registration(field.type_id(), field.type_path(), registry) + let registration = get_registration(field.type_id(), field.type_path(), registry)?; + Ok(ReflectionDeserializer::Typed( + TypedReflectDeserializer::new(registration, registry), + )) } } @@ -142,11 +145,11 @@ impl TupleLikeInfo for TupleStructInfo { } impl Container for TupleStructInfo { - fn get_field_registration<'a, E: Error>( + fn get_field_deserializer<'a, E: Error>( &self, index: usize, registry: &'a TypeRegistry, - ) -> Result<&'a TypeRegistration, E> { + ) -> Result, E> { let field = self.field_at(index).ok_or_else(|| { Error::custom(format_args!( "no field at index {} on tuple struct {}", @@ -154,7 +157,7 @@ impl Container for TupleStructInfo { self.type_path(), )) })?; - get_registration(field.type_id(), field.type_path(), registry) + get_deserializer(field.type_id(), field.type_path(), registry) } } @@ -165,11 +168,11 @@ impl TupleLikeInfo for TupleVariantInfo { } impl Container for TupleVariantInfo { - fn get_field_registration<'a, E: Error>( + fn get_field_deserializer<'a, E: Error>( &self, index: usize, registry: &'a TypeRegistry, - ) -> Result<&'a TypeRegistration, E> { + ) -> Result, E> { let field = self.field_at(index).ok_or_else(|| { Error::custom(format_args!( "no field at index {} on tuple variant {}", @@ -177,7 +180,7 @@ impl Container for TupleVariantInfo { self.name(), )) })?; - get_registration(field.type_id(), field.type_path(), registry) + get_deserializer(field.type_id(), field.type_path(), registry) } } @@ -894,11 +897,8 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> { )? .into(), VariantInfo::Tuple(tuple_info) if tuple_info.field_len() == 1 => { - let registration = tuple_info.get_field_registration(0, self.registry)?; - let value = variant.newtype_variant_seed(TypedReflectDeserializer { - registration, - registry: self.registry, - })?; + let value = variant + .newtype_variant_seed(tuple_info.get_field_deserializer(0, self.registry)?)?; let mut dynamic_tuple = DynamicTuple::default(); dynamic_tuple.insert_boxed(value); dynamic_tuple.into() @@ -1095,11 +1095,8 @@ where ExpectedValues(fields.collect()) )) })?; - let registration = get_registration(field.type_id(), field.type_path(), registry)?; - let value = map.next_value_seed(TypedReflectDeserializer { - registration, - registry, - })?; + let deserializer = get_deserializer(field.type_id(), field.type_path(), registry)?; + let value = map.next_value_seed(deserializer)?; dynamic_struct.insert_boxed(&key, value); } @@ -1146,10 +1143,7 @@ where } let value = seq - .next_element_seed(TypedReflectDeserializer { - registration: info.get_field_registration(index, registry)?, - registry, - })? + .next_element_seed(info.get_field_deserializer(index, registry)?)? .ok_or_else(|| Error::invalid_length(index, &len.to_string().as_str()))?; tuple.insert_boxed(value); } @@ -1192,10 +1186,7 @@ where } let value = seq - .next_element_seed(TypedReflectDeserializer { - registration: info.get_field_registration(index, registry)?, - registry, - })? + .next_element_seed(info.get_field_deserializer(index, registry)?)? .ok_or_else(|| Error::invalid_length(index, &len.to_string().as_str()))?; dynamic_struct.insert_boxed(name, value); } @@ -1214,6 +1205,42 @@ fn get_registration<'a, E: Error>( Ok(registration) } +fn get_deserializer<'a, E: Error>( + type_id: TypeId, + type_path: &str, + registry: &'a TypeRegistry, +) -> Result, E> { + if type_path.starts_with("bevy_reflect::boxed::ReflectBox<") { + Ok(ReflectionDeserializer::Untyped(ReflectDeserializer::new( + registry, + ))) + } else { + Ok(ReflectionDeserializer::Typed(TypedReflectDeserializer { + registration: get_registration::(type_id, type_path, registry).unwrap(), + registry, + })) + } +} + +enum ReflectionDeserializer<'a> { + Typed(TypedReflectDeserializer<'a>), + Untyped(ReflectDeserializer<'a>), +} + +impl<'a, 'de> DeserializeSeed<'de> for ReflectionDeserializer<'a> { + type Value = Box; + + fn deserialize(self, deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + match self { + Self::Typed(typed) => typed.deserialize(deserializer), + Self::Untyped(untyped) => untyped.deserialize(deserializer), + } + } +} + #[cfg(test)] mod tests { use bincode::Options; diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 8615171238035..10fd001a1dbee 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -10,6 +10,7 @@ use serde::{ ser::{SerializeMap, SerializeSeq}, Serialize, }; +use std::cell::Cell; use super::SerializationData; @@ -28,10 +29,57 @@ impl<'a> Serializable<'a> { } } +thread_local! { + /// Thread-local flag indicating whether we are currently + /// serializing a [`ReflectBox`] value. + /// + /// When a [`ReflectBox`] value is detected and this flag is `false`, + /// the serializer should attempt to serialize the full type map of the value. + /// It should then set the flag to `true` to indicate that the map has been + /// created and the inner value should be serialized as normal. + /// + /// This is necessary to prevent infinite recursion when serializing a [`ReflectBox`]. + /// + /// [`ReflectBox`]: crate::boxed::ReflectBox + static IS_BOXED: Cell = const { Cell::new(false) }; +} + +/// Get the serializer for the given reflected value. +/// +/// This function should be used in place of constructing the serializer directly, +/// as it will handle the special case of serializing a [`ReflectBox`] value. +/// +/// [`ReflectBox`]: crate::boxed::ReflectBox +fn get_serializer<'a>( + reflect_value: &'a dyn PartialReflect, + type_registry: &'a TypeRegistry, +) -> Box { + let path = reflect_value.reflect_type_path(); + if path.starts_with("bevy_reflect::boxed::ReflectBox<") { + Box::new(ReflectSerializer::new(reflect_value, type_registry)) + } else { + Box::new(TypedReflectSerializer::new_reflect_box( + reflect_value, + type_registry, + )) + } +} + fn get_serializable<'a, E: Error>( reflect_value: &'a dyn PartialReflect, - type_registry: &TypeRegistry, + type_registry: &'a TypeRegistry, ) -> Result, E> { + let path = reflect_value.reflect_type_path(); + if !IS_BOXED.get() && path.starts_with("bevy_reflect::boxed::ReflectBox<") { + // The value is a `ReflectBox` but it has not been flagged yet. + // Flag it and serialize the full type map. + IS_BOXED.set(true); + return Ok(Serializable::Owned(Box::new(ReflectSerializer::new( + reflect_value, + type_registry, + )))); + } + let Some(reflect_value) = reflect_value.try_as_reflect() else { return Err(Error::custom(format_args!( "Type '{}' does not implement `Reflect`", @@ -183,8 +231,19 @@ pub struct TypedReflectSerializer<'a> { impl<'a> TypedReflectSerializer<'a> { pub fn new(value: &'a dyn PartialReflect, registry: &'a TypeRegistry) -> Self { + // Check if `value` is a `ReflectBox` type. + IS_BOXED.set( + value + .reflect_type_path() + .starts_with("bevy_reflect::boxed::ReflectBox<"), + ); TypedReflectSerializer { value, registry } } + + fn new_reflect_box(value: &'a dyn PartialReflect, registry: &'a TypeRegistry) -> Self { + IS_BOXED.set(false); + Self { value, registry } + } } impl<'a> Serialize for TypedReflectSerializer<'a> { @@ -307,7 +366,7 @@ impl<'a> Serialize for StructSerializer<'a> { continue; } let key = struct_info.field_at(index).unwrap().name(); - state.serialize_field(key, &TypedReflectSerializer::new(value, self.registry))?; + state.serialize_field(key, &get_serializer(value, self.registry))?; } state.end() } @@ -359,7 +418,7 @@ impl<'a> Serialize for TupleStructSerializer<'a> { { continue; } - state.serialize_field(&TypedReflectSerializer::new(value, self.registry))?; + state.serialize_field(&get_serializer(value, self.registry))?; } state.end() } From e62be7e5d60705302a19abf7f4767e7e5286fc6f Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Thu, 15 Aug 2024 15:29:24 -0700 Subject: [PATCH 3/9] Serialize concrete ReflectBox as typed --- crates/bevy_reflect/src/boxed.rs | 10 +++++----- crates/bevy_reflect/src/serde/de.rs | 18 +++++++++++++++++- crates/bevy_reflect/src/serde/ser.rs | 6 +++--- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/bevy_reflect/src/boxed.rs b/crates/bevy_reflect/src/boxed.rs index 3c396e02697e5..30ef6088bbf12 100644 --- a/crates/bevy_reflect/src/boxed.rs +++ b/crates/bevy_reflect/src/boxed.rs @@ -684,7 +684,7 @@ mod tests { let serialized = ron::to_string(&reflect_serializer).unwrap(); assert_eq!( serialized, - r#"{"bevy_reflect::boxed::tests::MyStruct":(partial_reflect:{"i32":123},full_reflect:{"i32":456},concrete:{"i32":789})}"# + r#"{"bevy_reflect::boxed::tests::MyStruct":(partial_reflect:{"i32":123},full_reflect:{"i32":456},concrete:789)}"# ); let reflect_deserializer = ReflectDeserializer::new(®istry); @@ -719,7 +719,7 @@ mod tests { let serialized = ron::to_string(&reflect_serializer).unwrap(); assert_eq!( serialized, - r#"{"bevy_reflect::boxed::tests::MyTupleStruct":({"i32":123},{"i32":456},{"i32":789})}"# + r#"{"bevy_reflect::boxed::tests::MyTupleStruct":({"i32":123},{"i32":456},789)}"# ); let reflect_deserializer = ReflectDeserializer::new(®istry); @@ -751,8 +751,8 @@ mod tests { #[derive(Reflect)] #[reflect(from_reflect = false)] struct MyNestedStruct { - #[reflect(remote = ReflectBox)] - inner: Box, + #[reflect(remote = ReflectBox)] + inner: Box, } let input = MyNestedStruct { @@ -771,7 +771,7 @@ mod tests { let serialized = ron::to_string(&reflect_serializer).unwrap(); assert_eq!( serialized, - r#"{"bevy_reflect::boxed::tests::MyNestedStruct":(inner:{"bevy_reflect::boxed::tests::MyStruct":(partial_reflect:{"i32":123},full_reflect:{"i32":456},concrete:{"i32":789})})}"# + r#"{"bevy_reflect::boxed::tests::MyNestedStruct":(inner:{"bevy_reflect::boxed::tests::MyStruct":(partial_reflect:{"i32":123},full_reflect:{"i32":456},concrete:789)})}"# ); let reflect_deserializer = ReflectDeserializer::new(®istry); diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs index fae184dbd700a..58b2a9a04c440 100644 --- a/crates/bevy_reflect/src/serde/de.rs +++ b/crates/bevy_reflect/src/serde/de.rs @@ -1210,10 +1210,26 @@ fn get_deserializer<'a, E: Error>( type_path: &str, registry: &'a TypeRegistry, ) -> Result, E> { - if type_path.starts_with("bevy_reflect::boxed::ReflectBox<") { + if type_path.starts_with("bevy_reflect::boxed::ReflectBox") + .ok_or_else(|| E::custom(format_args!("invalid type path `{type_path}`")))?; + + let registration = registry.get_with_type_path(real_type_path).ok_or_else(|| { + Error::custom(format_args!( + "no registration found for type `{real_type_path}`" + )) + })?; + Ok(ReflectionDeserializer::Typed(TypedReflectDeserializer { + registration, + registry, + })) } else { Ok(ReflectionDeserializer::Typed(TypedReflectDeserializer { registration: get_registration::(type_id, type_path, registry).unwrap(), diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 10fd001a1dbee..50a621cb6a02a 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -55,7 +55,7 @@ fn get_serializer<'a>( type_registry: &'a TypeRegistry, ) -> Box { let path = reflect_value.reflect_type_path(); - if path.starts_with("bevy_reflect::boxed::ReflectBox<") { + if path.starts_with("bevy_reflect::boxed::ReflectBox( type_registry: &'a TypeRegistry, ) -> Result, E> { let path = reflect_value.reflect_type_path(); - if !IS_BOXED.get() && path.starts_with("bevy_reflect::boxed::ReflectBox<") { + if !IS_BOXED.get() && path.starts_with("bevy_reflect::boxed::ReflectBox TypedReflectSerializer<'a> { IS_BOXED.set( value .reflect_type_path() - .starts_with("bevy_reflect::boxed::ReflectBox<"), + .starts_with("bevy_reflect::boxed::ReflectBox Date: Thu, 15 Aug 2024 17:57:02 -0700 Subject: [PATCH 4/9] Make reflect serializer fields private --- crates/bevy_reflect/src/serde/ser.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 50a621cb6a02a..097e0b0dfa2fd 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -147,8 +147,8 @@ fn get_serializable<'a, E: Error>( /// [`ReflectDeserializer`]: crate::serde::ReflectDeserializer /// [type path]: crate::TypePath::type_path pub struct ReflectSerializer<'a> { - pub value: &'a dyn PartialReflect, - pub registry: &'a TypeRegistry, + value: &'a dyn PartialReflect, + registry: &'a TypeRegistry, } impl<'a> ReflectSerializer<'a> { @@ -225,8 +225,8 @@ impl<'a> Serialize for ReflectSerializer<'a> { /// [`TypedReflectDeserializer`]: crate::serde::TypedReflectDeserializer /// [type path]: crate::TypePath::type_path pub struct TypedReflectSerializer<'a> { - pub value: &'a dyn PartialReflect, - pub registry: &'a TypeRegistry, + value: &'a dyn PartialReflect, + registry: &'a TypeRegistry, } impl<'a> TypedReflectSerializer<'a> { @@ -237,7 +237,7 @@ impl<'a> TypedReflectSerializer<'a> { .reflect_type_path() .starts_with("bevy_reflect::boxed::ReflectBox Self { From b66dd963289959c1558f35f6836a14b3c4b8b7f6 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 16 Aug 2024 00:09:13 -0700 Subject: [PATCH 5/9] Remove thread local No longer needed due to fields being handled directly, and value types always serializing as a full type map --- crates/bevy_reflect/src/serde/ser.rs | 43 +--------------------------- 1 file changed, 1 insertion(+), 42 deletions(-) diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 097e0b0dfa2fd..22fe35852bfbd 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -10,7 +10,6 @@ use serde::{ ser::{SerializeMap, SerializeSeq}, Serialize, }; -use std::cell::Cell; use super::SerializationData; @@ -29,21 +28,6 @@ impl<'a> Serializable<'a> { } } -thread_local! { - /// Thread-local flag indicating whether we are currently - /// serializing a [`ReflectBox`] value. - /// - /// When a [`ReflectBox`] value is detected and this flag is `false`, - /// the serializer should attempt to serialize the full type map of the value. - /// It should then set the flag to `true` to indicate that the map has been - /// created and the inner value should be serialized as normal. - /// - /// This is necessary to prevent infinite recursion when serializing a [`ReflectBox`]. - /// - /// [`ReflectBox`]: crate::boxed::ReflectBox - static IS_BOXED: Cell = const { Cell::new(false) }; -} - /// Get the serializer for the given reflected value. /// /// This function should be used in place of constructing the serializer directly, @@ -58,10 +42,7 @@ fn get_serializer<'a>( if path.starts_with("bevy_reflect::boxed::ReflectBox( reflect_value: &'a dyn PartialReflect, type_registry: &'a TypeRegistry, ) -> Result, E> { - let path = reflect_value.reflect_type_path(); - if !IS_BOXED.get() && path.starts_with("bevy_reflect::boxed::ReflectBox { impl<'a> TypedReflectSerializer<'a> { pub fn new(value: &'a dyn PartialReflect, registry: &'a TypeRegistry) -> Self { - // Check if `value` is a `ReflectBox` type. - IS_BOXED.set( - value - .reflect_type_path() - .starts_with("bevy_reflect::boxed::ReflectBox Self { - IS_BOXED.set(false); Self { value, registry } } } From c72173ca7a0d7df2f473504c9e5e39c43cc9d97d Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 16 Aug 2024 00:14:32 -0700 Subject: [PATCH 6/9] Revert "Make reflect serializer fields private" This reverts commit 3b324adc34e6a1a107edc1b30f04c11cd01603a2. --- crates/bevy_reflect/src/serde/ser.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/bevy_reflect/src/serde/ser.rs b/crates/bevy_reflect/src/serde/ser.rs index 22fe35852bfbd..a21266f4294ce 100644 --- a/crates/bevy_reflect/src/serde/ser.rs +++ b/crates/bevy_reflect/src/serde/ser.rs @@ -117,8 +117,8 @@ fn get_serializable<'a, E: Error>( /// [`ReflectDeserializer`]: crate::serde::ReflectDeserializer /// [type path]: crate::TypePath::type_path pub struct ReflectSerializer<'a> { - value: &'a dyn PartialReflect, - registry: &'a TypeRegistry, + pub value: &'a dyn PartialReflect, + pub registry: &'a TypeRegistry, } impl<'a> ReflectSerializer<'a> { @@ -195,8 +195,8 @@ impl<'a> Serialize for ReflectSerializer<'a> { /// [`TypedReflectDeserializer`]: crate::serde::TypedReflectDeserializer /// [type path]: crate::TypePath::type_path pub struct TypedReflectSerializer<'a> { - value: &'a dyn PartialReflect, - registry: &'a TypeRegistry, + pub value: &'a dyn PartialReflect, + pub registry: &'a TypeRegistry, } impl<'a> TypedReflectSerializer<'a> { From 6dfdfb14844cd70c8004c2a59c3191e3e191a0cc Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Fri, 16 Aug 2024 16:00:28 -0700 Subject: [PATCH 7/9] Unhide ToPartialReflect and ToReflect Also moved these traits to a new `cast` module --- crates/bevy_reflect/src/boxed.rs | 114 ++------------------ crates/bevy_reflect/src/cast.rs | 180 +++++++++++++++++++++++++++++++ crates/bevy_reflect/src/lib.rs | 1 + 3 files changed, 187 insertions(+), 108 deletions(-) create mode 100644 crates/bevy_reflect/src/cast.rs diff --git a/crates/bevy_reflect/src/boxed.rs b/crates/bevy_reflect/src/boxed.rs index 30ef6088bbf12..ac1d8549af9bb 100644 --- a/crates/bevy_reflect/src/boxed.rs +++ b/crates/bevy_reflect/src/boxed.rs @@ -2,6 +2,7 @@ use core::fmt::Formatter; use std::any::Any; use crate::__macro_exports::RegisterForReflection; +use crate::cast::{ToPartialReflect, ToReflect}; use crate::serde::Serializable; use crate::utility::GenericTypePathCell; use crate::{ @@ -9,114 +10,6 @@ use crate::{ ReflectRef, ReflectRemote, TypeInfo, TypePath, }; -/// A trait used to access `Self` as a [`dyn PartialReflect`]. -/// -/// This is used to provide by [`ReflectBox`] in order to remotely reflect -/// the inner type, `T`. -/// -/// This trait can be implemented on custom trait objects to allow them to be remotely reflected -/// by [`ReflectBox`]. -/// -/// [`dyn PartialReflect`]: PartialReflect -#[doc(hidden)] -pub trait ToPartialReflect: Send + Sync + 'static { - /// Get a reference to `Self` as a [`&dyn PartialReflect`](PartialReflect). - fn to_partial_reflect_ref(&self) -> &dyn PartialReflect; - /// Get a mutable reference to `Self` as a [`&mut dyn PartialReflect`](PartialReflect). - fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect; - /// Take `Self` as a [`Box`](PartialReflect). - fn to_partial_reflect_box(self: Box) -> Box; -} - -impl ToPartialReflect for T { - fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { - self - } - - fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { - self - } - - fn to_partial_reflect_box(self: Box) -> Box { - self - } -} - -impl ToPartialReflect for dyn PartialReflect { - fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { - self - } - - fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { - self - } - - fn to_partial_reflect_box(self: Box) -> Box { - self - } -} - -/// A trait used to access `Self` as a [`dyn Reflect`]. -/// -/// This is used to provide by [`ReflectBox`] in order to remotely reflect -/// the inner type, `T`. -/// -/// This trait can be implemented on custom trait objects to allow them to be remotely reflected -/// by [`ReflectBox`]. -/// -/// [`dyn Reflect`]: Reflect -#[doc(hidden)] -pub trait ToReflect: ToPartialReflect { - /// Get a reference to `Self` as a [`&dyn Reflect`](Reflect). - fn to_reflect_ref(&self) -> &dyn Reflect; - /// Get a mutable reference to `Self` as a [`&mut dyn Reflect`](Reflect). - fn to_reflect_mut(&mut self) -> &mut dyn Reflect; - /// Take `Self` as a [`Box`](Reflect). - fn to_reflect_box(self: Box) -> Box; -} - -impl ToReflect for T { - fn to_reflect_ref(&self) -> &dyn Reflect { - self - } - - fn to_reflect_mut(&mut self) -> &mut dyn Reflect { - self - } - - fn to_reflect_box(self: Box) -> Box { - self - } -} - -impl ToPartialReflect for dyn Reflect { - fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { - self.as_partial_reflect() - } - - fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { - self.as_partial_reflect_mut() - } - - fn to_partial_reflect_box(self: Box) -> Box { - self.into_partial_reflect() - } -} - -impl ToReflect for dyn Reflect { - fn to_reflect_ref(&self) -> &dyn Reflect { - self - } - - fn to_reflect_mut(&mut self) -> &mut dyn Reflect { - self - } - - fn to_reflect_box(self: Box) -> Box { - self - } -} - /// A [remote wrapper] type around [`Box`] where `T` is either a type that implements [`PartialReflect`]/[`Reflect`] /// or a [`dyn PartialReflect`]/[`dyn Reflect`] trait object. /// @@ -126,6 +19,10 @@ impl ToReflect for dyn Reflect { /// [`ReflectBox`] should never be created or used directly outside remote reflection, /// in order to avoid double-boxing. /// +/// Along with supporting reflection for `Box`, `Box`, and `Box`, +/// [`ReflectBox`] can also be used to reflect any boxed trait object that is compatible with reflection. +/// This can be achieved by implementing [`ToPartialReflect`] and [`ToReflect`] for the custom trait object type. +/// /// # Examples /// /// ``` @@ -164,6 +61,7 @@ impl ToReflect for dyn Reflect { /// [remote wrapper]: ReflectRemote /// [`dyn PartialReflect`]: PartialReflect /// [`dyn Reflect`]: Reflect +/// [`cast`]: crate::cast #[repr(transparent)] pub struct ReflectBox(Box); diff --git a/crates/bevy_reflect/src/cast.rs b/crates/bevy_reflect/src/cast.rs new file mode 100644 index 0000000000000..3e9db0223f13e --- /dev/null +++ b/crates/bevy_reflect/src/cast.rs @@ -0,0 +1,180 @@ +//! Traits for casting types to [`dyn PartialReflect`] and [`dyn Reflect`] trait objects. +//! +//! These traits are primarily used by [`ReflectBox`] to cast the inner type to +//! the corresponding reflection trait object. +//! +//! # Example +//! +//! ``` +//! use bevy_reflect::{PartialReflect, Reflect, TypePath}; +//! use bevy_reflect::cast::{ToPartialReflect, ToReflect}; +//! use bevy_reflect::boxed::ReflectBox; +//! +//! // A custom trait used to represent equippable items +//! trait Equippable: Reflect {} +//! +//! impl TypePath for dyn Equippable { +//! fn type_path() -> &'static str { +//! "dyn my_crate::equipment::Equippable" +//! } +//! +//! fn short_type_path() -> &'static str { +//! "dyn Equippable" +//! } +//! } +//! +//! impl ToPartialReflect for dyn Equippable { +//! fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { +//! self.as_partial_reflect() +//! } +//! +//! fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { +//! self.as_partial_reflect_mut() +//! } +//! +//! fn to_partial_reflect_box(self: Box) -> Box { +//! self.into_partial_reflect() +//! } +//! } +//! +//! impl ToReflect for dyn Equippable { +//! fn to_reflect_ref(&self) -> &dyn Reflect { +//! self.as_reflect() +//! } +//! +//! fn to_reflect_mut(&mut self) -> &mut dyn Reflect { +//! self.as_reflect_mut() +//! } +//! +//! fn to_reflect_box(self: Box) -> Box { +//! self.into_reflect() +//! } +//! } +//! +//! #[derive(Reflect)] +//! #[reflect(from_reflect = false)] +//! struct Player { +//! // Now `dyn Equippable` can be used with `ReflectBox`: +//! #[reflect(remote = ReflectBox)] +//! weapon: Box, +//! } +//! ``` +//! +//! [`dyn PartialReflect`]: PartialReflect +//! [`dyn Reflect`]: Reflect + +use crate::{PartialReflect, Reflect}; + +/// A trait used to access `Self` as a [`dyn PartialReflect`]. +/// +/// This is used by [`ReflectBox`] in order to remotely reflect the inner type, `T`. +/// In most cases, [`PartialReflect`] should be used instead of this trait. +/// +/// This trait can be implemented on custom trait objects to allow them to be remotely reflected +/// by [`ReflectBox`]. +/// +/// See the [module-level documentation] for details. +/// +/// [`dyn PartialReflect`]: PartialReflect +/// [`ReflectBox`]: crate::boxed::ReflectBox +/// [module-level documentation]: crate::cast +pub trait ToPartialReflect: Send + Sync + 'static { + /// Get a reference to `Self` as a [`&dyn PartialReflect`](PartialReflect). + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect; + /// Get a mutable reference to `Self` as a [`&mut dyn PartialReflect`](PartialReflect). + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect; + /// Take `Self` as a [`Box`](PartialReflect). + fn to_partial_reflect_box(self: Box) -> Box; +} + +impl ToPartialReflect for T { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self + } + + fn to_partial_reflect_box(self: Box) -> Box { + self + } +} + +impl ToPartialReflect for dyn PartialReflect { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self + } + + fn to_partial_reflect_box(self: Box) -> Box { + self + } +} + +/// A trait used to access `Self` as a [`dyn Reflect`]. +/// +/// This is used by [`ReflectBox`] in order to remotely reflect the inner type, `T`. +/// In most cases, [`Reflect`] should be used instead of this trait. +/// +/// This trait can be implemented on custom trait objects to allow them to be remotely reflected +/// by [`ReflectBox`]. +/// +/// See the [module-level documentation] for details. +/// +/// [`dyn Reflect`]: Reflect +/// [`ReflectBox`]: crate::boxed::ReflectBox +/// [module-level documentation]: crate::cast +pub trait ToReflect: ToPartialReflect { + /// Get a reference to `Self` as a [`&dyn Reflect`](Reflect). + fn to_reflect_ref(&self) -> &dyn Reflect; + /// Get a mutable reference to `Self` as a [`&mut dyn Reflect`](Reflect). + fn to_reflect_mut(&mut self) -> &mut dyn Reflect; + /// Take `Self` as a [`Box`](Reflect). + fn to_reflect_box(self: Box) -> Box; +} + +impl ToReflect for T { + fn to_reflect_ref(&self) -> &dyn Reflect { + self + } + + fn to_reflect_mut(&mut self) -> &mut dyn Reflect { + self + } + + fn to_reflect_box(self: Box) -> Box { + self + } +} + +impl ToPartialReflect for dyn Reflect { + fn to_partial_reflect_ref(&self) -> &dyn PartialReflect { + self.as_partial_reflect() + } + + fn to_partial_reflect_mut(&mut self) -> &mut dyn PartialReflect { + self.as_partial_reflect_mut() + } + + fn to_partial_reflect_box(self: Box) -> Box { + self.into_partial_reflect() + } +} + +impl ToReflect for dyn Reflect { + fn to_reflect_ref(&self) -> &dyn Reflect { + self + } + + fn to_reflect_mut(&mut self) -> &mut dyn Reflect { + self + } + + fn to_reflect_box(self: Box) -> Box { + self + } +} diff --git a/crates/bevy_reflect/src/lib.rs b/crates/bevy_reflect/src/lib.rs index 327ac4cf1f930..99aed44f77c52 100644 --- a/crates/bevy_reflect/src/lib.rs +++ b/crates/bevy_reflect/src/lib.rs @@ -527,6 +527,7 @@ mod array; pub mod boxed; +pub mod cast; mod fields; mod from_reflect; #[cfg(feature = "functions")] From 350b3164b6c23ac4e213c8713cd01dea2110099e Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 19 Aug 2024 17:54:46 -0700 Subject: [PATCH 8/9] Update test for custom trait objects --- crates/bevy_reflect/src/boxed.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/bevy_reflect/src/boxed.rs b/crates/bevy_reflect/src/boxed.rs index ac1d8549af9bb..e60d4638955e7 100644 --- a/crates/bevy_reflect/src/boxed.rs +++ b/crates/bevy_reflect/src/boxed.rs @@ -525,12 +525,24 @@ mod tests { } } + #[derive(Reflect)] + struct Sword(u32); + + impl Equippable for Sword {} + #[derive(Reflect)] #[reflect(from_reflect = false)] struct Player { #[reflect(remote = ReflectBox)] weapon: Box, } + + let player: Box = Box::new(Player { + weapon: Box::new(Sword(123)), + }); + + let weapon = player.field("weapon").unwrap(); + assert!(weapon.reflect_partial_eq(&Sword(123)).unwrap_or_default()); } #[test] From dcef362ecd3f227e630cbd3bf0b1b0e597dcf919 Mon Sep 17 00:00:00 2001 From: Gino Valente Date: Mon, 19 Aug 2024 21:26:10 -0700 Subject: [PATCH 9/9] Add example --- Cargo.toml | 11 +++++ examples/README.md | 1 + examples/reflection/nested_reflection.rs | 51 ++++++++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 examples/reflection/nested_reflection.rs diff --git a/Cargo.toml b/Cargo.toml index 978c1b754f9fb..9772f74157967 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2195,6 +2195,17 @@ description = "Demonstrates how functions can be called dynamically using reflec category = "Reflection" wasm = false +[[example]] +name = "nested_reflection" +path = "examples/reflection/nested_reflection.rs" +doc-scrape-examples = true + +[package.metadata.example.nested_reflection] +name = "Nested Reflection" +description = "Demonstrates how reflection trait objects can be nested in other reflected types" +category = "Reflection" +wasm = false + [[example]] name = "generic_reflection" path = "examples/reflection/generic_reflection.rs" diff --git a/examples/README.md b/examples/README.md index b1fa8ac61d981..2a0792c605d3f 100644 --- a/examples/README.md +++ b/examples/README.md @@ -365,6 +365,7 @@ Example | Description [Custom Attributes](../examples/reflection/custom_attributes.rs) | Registering and accessing custom attributes on reflected types [Dynamic Types](../examples/reflection/dynamic_types.rs) | How dynamic types are used with reflection [Function Reflection](../examples/reflection/function_reflection.rs) | Demonstrates how functions can be called dynamically using reflection +[Nested Reflection](../examples/reflection/nested_reflection.rs) | Demonstrates how reflection trait objects can be nested in other reflected types [Generic Reflection](../examples/reflection/generic_reflection.rs) | Registers concrete instances of generic types that may be used with reflection [Reflection](../examples/reflection/reflection.rs) | Demonstrates how reflection in Bevy provides a way to dynamically interact with Rust types [Reflection Types](../examples/reflection/reflection_types.rs) | Illustrates the various reflection types available diff --git a/examples/reflection/nested_reflection.rs b/examples/reflection/nested_reflection.rs new file mode 100644 index 0000000000000..1b56b8a79ae1b --- /dev/null +++ b/examples/reflection/nested_reflection.rs @@ -0,0 +1,51 @@ +//! This example demonstrates how reflection trait objects can be nested in other reflected types +//! using remote reflection. + +use bevy::prelude::*; + +fn main() { + // Bevy's reflection crate relies heavily on the `dyn Reflect` trait object. + // This allows the compile-time name of a type to be "erased" and passed around at runtime, + // most often as a `Box`. + let _: Box = Box::new(String::from("Hello, World!")); + + // However, you'll notice that `Box` itself doesn't implement `Reflect`. + // This makes it impossible to use `Box` as a field in a struct that derives `Reflect`. + // ``` + // #[derive(Reflect)] + // struct MyType { + // field: Box, // <- Compile Error + // } + // ``` + // This is because it would be too easy to accidentally box a `Reflect` type, + // then accidentally box it again, and again, and so on. + // So instead, `bevy_reflect` exposes a `ReflectBox` type which can be used + // as a remote wrapper around a `Box` (or `Box`). + // + // For example, let's say we want to define some equipment for a player. + // We don't know what kind of equipment the player will have at compile time, + // so we want to store it as a `Box`. + // To do this, we first need to derive `Reflect` for our `Player`. + #[derive(Reflect)] + // Next, we need to opt out of deriving `FromReflect` since `Box` + // has no knowledge of `FromReflect`. + #[reflect(from_reflect = false)] + struct Player { + // Now we can use remote reflection to tell `Reflect` how to reflect our `Box`. + #[reflect(remote = bevy::reflect::boxed::ReflectBox)] + equipment: Box, + } + + // Now we can use any type that implements `Reflect` as equipment for our player. + let equipment: Box = Box::new(String::from("Sword")); + let mut player: Box = Box::new(Player { equipment }); + + // We can also use reflection to modify our player's equipment. + let equipment = player.field_mut("equipment").unwrap(); + equipment.try_apply(&String::from("Shield")).unwrap(); + assert!(player + .reflect_partial_eq(&Player { + equipment: Box::new(String::from("Shield")), + }) + .unwrap_or_default()); +}