Skip to content

Commit

Permalink
bevy_reflect: Add ReflectFromReflect (v2) (#6245)
Browse files Browse the repository at this point in the history
# Objective

Resolves #4597 (based on the work from #6056 and a refresh of #4147)

When using reflection, we may often end up in a scenario where we have a Dynamic representing a certain type. Unfortunately, we can't just call `MyType::from_reflect` as we do not have knowledge of the concrete type (`MyType`) at runtime.

Such scenarios happen when we call `Reflect::clone_value`, use the reflection deserializers, or create the Dynamic type ourselves.

## Solution

Add a `ReflectFromReflect` type data struct.

This struct allows us to easily convert Dynamic representations of our types into their respective concrete instances.

```rust
#[derive(Reflect, FromReflect)]
#[reflect(FromReflect)] // <- Register `ReflectFromReflect`
struct MyStruct(String);

let type_id = TypeId::of::<MyStruct>();

// Register our type
let mut registry = TypeRegistry::default();
registry.register::<MyStruct>();

// Create a concrete instance
let my_struct = MyStruct("Hello world".to_string());

// `Reflect::clone_value` will generate a `DynamicTupleStruct` for tuple struct types
let dynamic_value: Box<dyn Reflect> = my_struct.clone_value();
assert!(!dynamic_value.is::<MyStruct>());

// Get the `ReflectFromReflect` type data from the registry
let rfr: &ReflectFromReflect = registry
  .get_type_data::<ReflectFromReflect>(type_id)
  .unwrap();

// Call `FromReflect::from_reflect` on our Dynamic value
let concrete_value: Box<dyn Reflect> = rfr.from_reflect(&dynamic_value);
assert!(concrete_value.is::<MyStruct>());
```

### Why this PR?

###### Why now?

The three main reasons I closed #4147 were that:

1. Registering `ReflectFromReflect` is clunky (deriving `FromReflect` *and* registering `ReflectFromReflect`)
2. The ecosystem and Bevy itself didn't seem to pay much attention to deriving `FromReflect`
3. I didn't see a lot of desire from the community for such a feature

However, as time has passed it seems 2 and 3 are not really true anymore. Bevy is internally adding lots more `FromReflect` derives, which should make this feature all the more useful. Additionally, I have seen a growing number of people look for something like `ReflectFromReflect`.

I think 1 is still an issue, but not a horrible one. Plus it could be made much, much better using #6056. And I think splitting this feature out of #6056 could lead to #6056 being adopted sooner (or at least make the need more clear to users).

###### Why not just re-open #4147?

The main reason is so that this PR can garner more attention than simply re-opening the old one. This helps bring fresh eyes to the PR for potentially more perspectives/reviews.

---

## Changelog

* Added `ReflectFromReflect`

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
  • Loading branch information
MrGVSV and MrGVSV committed Dec 11, 2022
1 parent 6308041 commit 63f1a9d
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 15 deletions.
92 changes: 92 additions & 0 deletions crates/bevy_reflect/src/from_reflect.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
use crate::{FromType, Reflect};

/// A trait for types which can be constructed from a reflected type.
///
/// This trait can be derived on types which implement [`Reflect`]. Some complex
/// types (such as `Vec<T>`) may only be reflected if their element types
/// implement this trait.
///
/// For structs and tuple structs, fields marked with the `#[reflect(ignore)]`
/// attribute will be constructed using the `Default` implementation of the
/// field type, rather than the corresponding field value (if any) of the
/// reflected value.
pub trait FromReflect: Reflect + Sized {
/// Constructs a concrete instance of `Self` from a reflected value.
fn from_reflect(reflect: &dyn Reflect) -> Option<Self>;
}

/// Type data that represents the [`FromReflect`] trait and allows it to be used dynamically.
///
/// `FromReflect` allows dynamic types (e.g. [`DynamicStruct`], [`DynamicEnum`], etc.) to be converted
/// to their full, concrete types. This is most important when it comes to deserialization where it isn't
/// guaranteed that every field exists when trying to construct the final output.
///
/// However, to do this, you normally need to specify the exact concrete type:
///
/// ```
/// # use bevy_reflect::{DynamicTupleStruct, FromReflect, Reflect};
/// #[derive(Reflect, FromReflect, PartialEq, Eq, Debug)]
/// struct Foo(#[reflect(default = "default_value")] usize);
///
/// fn default_value() -> usize { 123 }
///
/// let reflected = DynamicTupleStruct::default();
///
/// let concrete: Foo = <Foo as FromReflect>::from_reflect(&reflected).unwrap();
///
/// assert_eq!(Foo(123), concrete);
/// ```
///
/// In a dynamic context where the type might not be known at compile-time, this is nearly impossible to do.
/// That is why this type data struct exists— it allows us to construct the full type without knowing
/// what the actual type is.
///
/// # Example
///
/// ```
/// # use bevy_reflect::{DynamicTupleStruct, FromReflect, Reflect, ReflectFromReflect, TypeRegistry};
/// # #[derive(Reflect, FromReflect, PartialEq, Eq, Debug)]
/// # #[reflect(FromReflect)]
/// # struct Foo(#[reflect(default = "default_value")] usize);
/// # fn default_value() -> usize { 123 }
/// # let mut registry = TypeRegistry::new();
/// # registry.register::<Foo>();
///
/// let mut reflected = DynamicTupleStruct::default();
/// reflected.set_name(std::any::type_name::<Foo>().to_string());
///
/// let registration = registry.get_with_name(reflected.type_name()).unwrap();
/// let rfr = registration.data::<ReflectFromReflect>().unwrap();
///
/// let concrete: Box<dyn Reflect> = rfr.from_reflect(&reflected).unwrap();
///
/// assert_eq!(Foo(123), concrete.take::<Foo>().unwrap());
/// ```
///
/// [`DynamicStruct`]: crate::DynamicStruct
/// [`DynamicEnum`]: crate::DynamicEnum
#[derive(Clone)]
pub struct ReflectFromReflect {
from_reflect: fn(&dyn Reflect) -> Option<Box<dyn Reflect>>,
}

impl ReflectFromReflect {
/// Perform a [`FromReflect::from_reflect`] conversion on the given reflection object.
///
/// This will convert the object to a concrete type if it wasn't already, and return
/// the value as `Box<dyn Reflect>`.
#[allow(clippy::wrong_self_convention)]
pub fn from_reflect(&self, reflect_value: &dyn Reflect) -> Option<Box<dyn Reflect>> {
(self.from_reflect)(reflect_value)
}
}

impl<T: FromReflect> FromType<T> for ReflectFromReflect {
fn from_type() -> Self {
Self {
from_reflect: |reflect_value| {
T::from_reflect(reflect_value).map(|value| Box::new(value) as Box<dyn Reflect>)
},
}
}
}
39 changes: 39 additions & 0 deletions crates/bevy_reflect/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

mod array;
mod fields;
mod from_reflect;
mod list;
mod map;
mod path;
Expand Down Expand Up @@ -47,6 +48,7 @@ pub mod prelude {
pub use array::*;
pub use enums::*;
pub use fields::*;
pub use from_reflect::*;
pub use impls::*;
pub use list::*;
pub use map::*;
Expand Down Expand Up @@ -103,6 +105,7 @@ mod tests {
ser::{to_string_pretty, PrettyConfig},
Deserializer,
};
use std::any::TypeId;
use std::fmt::{Debug, Formatter};

use super::prelude::*;
Expand Down Expand Up @@ -244,6 +247,42 @@ mod tests {
assert_eq!(values, vec![1]);
}

#[test]
fn should_call_from_reflect_dynamically() {
#[derive(Reflect, FromReflect)]
#[reflect(FromReflect)]
struct MyStruct {
foo: usize,
}

// Register
let mut registry = TypeRegistry::default();
registry.register::<MyStruct>();

// Get type data
let type_id = TypeId::of::<MyStruct>();
let rfr = registry
.get_type_data::<ReflectFromReflect>(type_id)
.expect("the FromReflect trait should be registered");

// Call from_reflect
let mut dynamic_struct = DynamicStruct::default();
dynamic_struct.insert("foo", 123usize);
let reflected = rfr
.from_reflect(&dynamic_struct)
.expect("the type should be properly reflected");

// Assert
let expected = MyStruct { foo: 123 };
assert!(expected
.reflect_partial_eq(reflected.as_ref())
.unwrap_or_default());
let not_expected = MyStruct { foo: 321 };
assert!(!not_expected
.reflect_partial_eq(reflected.as_ref())
.unwrap_or_default());
}

#[test]
fn from_reflect_should_use_default_field_attributes() {
#[derive(Reflect, FromReflect, Eq, PartialEq, Debug)]
Expand Down
17 changes: 2 additions & 15 deletions crates/bevy_reflect/src/reflect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,21 +215,6 @@ pub trait Reflect: Any + Send + Sync {
}
}

/// A trait for types which can be constructed from a reflected type.
///
/// This trait can be derived on types which implement [`Reflect`]. Some complex
/// types (such as `Vec<T>`) may only be reflected if their element types
/// implement this trait.
///
/// For structs and tuple structs, fields marked with the `#[reflect(ignore)]`
/// attribute will be constructed using the `Default` implementation of the
/// field type, rather than the corresponding field value (if any) of the
/// reflected value.
pub trait FromReflect: Reflect + Sized {
/// Constructs a concrete instance of `Self` from a reflected value.
fn from_reflect(reflect: &dyn Reflect) -> Option<Self>;
}

impl Debug for dyn Reflect {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.debug(f)
Expand Down Expand Up @@ -280,6 +265,8 @@ impl dyn Reflect {
/// a different type, like the Dynamic\*\*\* types do, you can call `represents`
/// to determine what type they represent. Represented types cannot be downcasted
/// to, but you can use [`FromReflect`] to create a value of the represented type from them.
///
/// [`FromReflect`]: crate::FromReflect
#[inline]
pub fn is<T: Reflect>(&self) -> bool {
self.type_id() == TypeId::of::<T>()
Expand Down

0 comments on commit 63f1a9d

Please sign in to comment.