Skip to content

Commit

Permalink
reflect: treat proxy types correctly when serializing (bevyengine#12024)
Browse files Browse the repository at this point in the history
# Objective

- Fixes bevyengine#12001.
- Note this PR doesn't change any feature flags, however flaky the issue
revealed they are.

## Solution

- Use `FromReflect` to convert proxy types to concrete ones in
`ReflectSerialize::get_serializable`.
- Use `get_represented_type_info() -> type_id()` to get the correct type
id to interact with the registry in
`bevy_reflect::serde::ser::get_serializable`.

---

## Changelog

- Registering `ReflectSerialize` now imposes additional `FromReflect`
and `TypePath` bounds.

## Migration Guide

- If `ReflectSerialize` is registered on a type, but `TypePath` or
`FromReflect` implementations are omitted (perhaps by
`#[reflect(type_path = false)` or `#[reflect(from_reflect = false)]`),
the traits must now be implemented.

---------

Co-authored-by: Alice Cecile <[email protected]>
Co-authored-by: Gino Valente <[email protected]>
  • Loading branch information
3 people authored and msvbg committed Feb 26, 2024
1 parent 339100e commit 9344e42
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
11 changes: 9 additions & 2 deletions crates/bevy_reflect/src/serde/ser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,19 @@ fn get_serializable<'a, E: Error>(
reflect_value: &'a dyn Reflect,
type_registry: &TypeRegistry,
) -> Result<Serializable<'a>, E> {
let info = reflect_value.get_represented_type_info().ok_or_else(|| {
Error::custom(format_args!(
"Type '{}' does not represent any type",
reflect_value.reflect_type_path(),
))
})?;

let reflect_serialize = type_registry
.get_type_data::<ReflectSerialize>(reflect_value.type_id())
.get_type_data::<ReflectSerialize>(info.type_id())
.ok_or_else(|| {
Error::custom(format_args!(
"Type '{}' did not register ReflectSerialize",
reflect_value.reflect_type_path()
info.type_path(),
))
})?;
Ok(reflect_serialize.get_serializable(reflect_value))
Expand Down
18 changes: 12 additions & 6 deletions crates/bevy_reflect/src/type_registry.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::{serde::Serializable, Reflect, TypeInfo, TypePath, Typed};
use crate::{serde::Serializable, FromReflect, Reflect, TypeInfo, TypePath, Typed};
use bevy_ptr::{Ptr, PtrMut};
use bevy_utils::{HashMap, HashSet, TypeIdMap};
use downcast_rs::{impl_downcast, Downcast};
Expand Down Expand Up @@ -461,14 +461,20 @@ pub struct ReflectSerialize {
get_serializable: for<'a> fn(value: &'a dyn Reflect) -> Serializable,
}

impl<T: Reflect + erased_serde::Serialize> FromType<T> for ReflectSerialize {
impl<T: TypePath + FromReflect + erased_serde::Serialize> FromType<T> for ReflectSerialize {
fn from_type() -> Self {
ReflectSerialize {
get_serializable: |value| {
let value = value.downcast_ref::<T>().unwrap_or_else(|| {
panic!("ReflectSerialize::get_serialize called with type `{}`, even though it was created for `{}`", value.reflect_type_path(), std::any::type_name::<T>())
});
Serializable::Borrowed(value)
value
.downcast_ref::<T>()
.map(|value| Serializable::Borrowed(value))
.or_else(|| T::from_reflect(value).map(|value| Serializable::Owned(Box::new(value))))
.unwrap_or_else(|| {
panic!(
"FromReflect::from_reflect failed when called on type `{}` with this value: {value:?}",
T::type_path(),
);
})
},
}
}
Expand Down
1 change: 1 addition & 0 deletions examples/scene/scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ fn save_scene_system(world: &mut World) {
component_b,
ComponentA { x: 1.0, y: 2.0 },
Transform::IDENTITY,
Name::new("joe"),
));
scene_world.spawn(ComponentA { x: 3.0, y: 4.0 });
scene_world.insert_resource(ResourceA { score: 1 });
Expand Down

0 comments on commit 9344e42

Please sign in to comment.