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.

#[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:

Registering ReflectFromReflect is clunky (deriving FromReflect and registering ReflectFromReflect)
The ecosystem and Bevy itself didn't seem to pay much attention to deriving FromReflect
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

Oct 12 '22 21:10 MrGVSV

I like the idea but can't say I'm a fan of this name, "ReflectFromReflect" doesn't seem very intuitive to me. Maybe ConvertFromReflect, CastFromReflect, ValueFromReflect, TypeFromReflect?

Oct 13 '22 14:10 sarkahn

I like the idea but can't say I'm a fan of this name, "ReflectFromReflect" doesn't seem very intuitive to me. Maybe ConvertFromReflect, CastFromReflect, ValueFromReflect, TypeFromReflect?

Yeah, I don't love the name. Unfortunately, there's not much we can do about this as the #[reflect(FromReflect, Default, ...)] attribute automatically prepends each identifier with Reflect (e.g. ReflectFromReflect, ReflectDefault, etc.).

It'd be great if there was a way to get the types directly from the trait (e.g. <Default as GetTypeData>::type_data()), but I don't think that's possible as you can't implement a trait for a trait (and besides, not all type data are based on traits).

So I think it's best to just keep this as is for now. You could maybe argue that ReflectFromReflect works fine since you're converting from one dyn Reflect to another haha.

Oct 13 '22 15:10 MrGVSV

Pull request successfully merged into main.

Build succeeded:

Dec 11 '22 18:12 bors[bot]

bevy_reflect: Add `ReflectFromReflect` (v2)

Objective

Solution

Why this PR?

Why now?

Why not just re-open #4147?

Changelog