Mirrors/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy synced 2024-11-10 07:04:33 +00:00
Code Issues Projects Releases Packages Wiki Activity

Use FromReflect when extracting entities in dynamic scenes (#15174)

Browse source 
# Objective

Fix #10284.

## Solution

When `DynamicSceneBuilder` extracts entities, they are cloned via
`PartialReflect::clone_value`, making them into dynamic versions of the
original components. This loses any custom `ReflectSerialize` type data.
Dynamic scenes are deserialized with the original types, not the dynamic
versions, and so any component with a custom serialize may fail. In this
case `Rect` and `Vec2`. The dynamic version includes the field names 'x'
and 'y' but the `Serialize` impl doesn't, hence the "expect float"
error.

The solution here: Instead of using `clone_value` to clone the
components, `FromReflect` clones and retains the original information
needed to serialize with any custom `Serialize` impls. I think using
something like `reflect_clone` from
(https://github.com/bevyengine/bevy/pull/13432) might make this more
efficient.

I also did the same when deserializing dynamic scenes to appease some of
the round-trip tests which use `ReflectPartialEq`, which requires the
types be the same and not a unique/proxy pair. I'm not sure it's
otherwise necessary. Maybe this would also be more efficient when
spawning dynamic scenes with `reflect_clone` instead of `FromReflect`
again?

An alternative solution would be to fall back to the dynamic version
when deserializing `DynamicScene`s if the custom version fails. I think
that's possible. Or maybe simply always deserializing via the dynamic
route for dynamic scenes?

## Testing

This example is similar to the original test case in #10284:

``` rust
#![allow(missing_docs)]

use bevy::{prelude::*, scene::SceneInstanceReady};

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, (save, load).chain())
        .observe(check)
        .run();
}

static SAVEGAME_SAVE_PATH: &str = "savegame.scn.ron";

fn save(world: &mut World) {
    let entity = world.spawn(OrthographicProjection::default()).id();

    let scene = DynamicSceneBuilder::from_world(world)
        .extract_entity(entity)
        .build();

    if let Some(registry) = world.get_resource::<AppTypeRegistry>() {
        let registry = registry.read();
        let serialized_scene = scene.serialize(&registry).unwrap();
        // println!("{}", serialized_scene);
        std::fs::write(format!("assets/{SAVEGAME_SAVE_PATH}"), serialized_scene).unwrap();
    }

    world.entity_mut(entity).despawn_recursive();
}

fn load(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(DynamicSceneBundle {
        scene: asset_server.load(SAVEGAME_SAVE_PATH),
        ..default()
    });
}

fn check(_trigger: Trigger<SceneInstanceReady>, query: Query<&OrthographicProjection>) {
    dbg!(query.single());
}
```


## Migration Guide

The `DynamicScene` format is changed to use custom serialize impls so
old scene files will need updating:

Old: 

```ron
(
  resources: {},
  entities: {
    4294967299: (
      components: {
        "bevy_render:📷:projection::OrthographicProjection": (
          near: 0.0,
          far: 1000.0,
          viewport_origin: (
            x: 0.5,
            y: 0.5,
          ),
          scaling_mode: WindowSize(1.0),
          scale: 1.0,
          area: (
            min: (
              x: -1.0,
              y: -1.0,
            ),
            max: (
              x: 1.0,
              y: 1.0,
            ),
          ),
        ),
      },
    ),
  },
)
```

New:

```ron
(
  resources: {},
  entities: {
    4294967299: (
      components: {
        "bevy_render:📷:projection::OrthographicProjection": (
          near: 0.0,
          far: 1000.0,
          viewport_origin: (0.5, 0.5),
          scaling_mode: WindowSize(1.0),
          scale: 1.0,
          area: (
            min: (-1.0, -1.0),
            max: (1.0, 1.0),
          ),
        ),
      },
    ),
  },
)
```

---------

Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
This commit is contained in:
Al M. 2024-09-15 07:33:39 -07:00 committed by GitHub
parent 21e39360f7
commit 2ea51fc60f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 130 additions and 84 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

78
crates/bevy_reflect/src/impls/glam.rs
View file

@ -1,10 +1,10 @@
use crate as bevy_reflect;
use crate::prelude::ReflectDefault;
use crate::{std_traits::ReflectDefault, ReflectDeserialize, ReflectSerialize};
use bevy_reflect_derive::{impl_reflect, impl_reflect_value};
use glam::*;
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct IVec2 {
x: i32,
@ -12,7 +12,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct IVec3 {
x: i32,
@ -21,7 +21,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct IVec4 {
x: i32,
@ -32,7 +32,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct I64Vec2 {
x: i64,
@ -41,7 +41,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct I64Vec3 {
x: i64,
@ -51,7 +51,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct I64Vec4 {
x: i64,
@ -62,7 +62,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct UVec2 {
x: u32,
@ -70,7 +70,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct UVec3 {
x: u32,
@ -79,7 +79,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct UVec4 {
x: u32,
@ -90,7 +90,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct U64Vec2 {
x: u64,
@ -98,7 +98,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct U64Vec3 {
x: u64,
@ -107,7 +107,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct U64Vec4 {
x: u64,
@ -118,7 +118,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Vec2 {
x: f32,
@ -126,7 +126,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Vec3 {
x: f32,
@ -135,7 +135,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Vec3A {
x: f32,
@ -144,7 +144,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Vec4 {
x: f32,
@ -155,7 +155,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct BVec2 {
x: bool,
@ -163,7 +163,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct BVec3 {
x: bool,
@ -172,7 +172,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct BVec4 {
x: bool,
@ -183,7 +183,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DVec2 {
x: f64,
@ -191,7 +191,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DVec3 {
x: f64,
@ -200,7 +200,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DVec4 {
x: f64,
@ -211,7 +211,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Mat2 {
x_axis: Vec2,
@ -219,7 +219,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Mat3 {
x_axis: Vec3,
@ -228,7 +228,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Mat3A {
x_axis: Vec3A,
@ -237,7 +237,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Mat4 {
x_axis: Vec4,
@ -248,7 +248,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DMat2 {
x_axis: DVec2,
@ -256,7 +256,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DMat3 {
x_axis: DVec3,
@ -265,7 +265,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DMat4 {
x_axis: DVec4,
@ -276,7 +276,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Affine2 {
matrix2: Mat2,
@ -284,7 +284,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Affine3A {
matrix3: Mat3A,
@ -293,7 +293,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DAffine2 {
matrix2: DMat2,
@ -301,7 +301,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DAffine3 {
matrix3: DMat3,
@ -310,7 +310,7 @@ impl_reflect!(
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Quat {
x: f32,
@ -320,7 +320,7 @@ impl_reflect!(
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DQuat {
x: f64,
@ -330,6 +330,6 @@ impl_reflect!(
}
);
impl_reflect_value!(::glam::EulerRot(Debug, Default));
impl_reflect_value!(::glam::BVec3A(Debug, Default));
impl_reflect_value!(::glam::BVec4A(Debug, Default));
impl_reflect_value!(::glam::EulerRot(Debug, Default, Deserialize, Serialize));
impl_reflect_value!(::glam::BVec3A(Debug, Default, Deserialize, Serialize));
impl_reflect_value!(::glam::BVec4A(Debug, Default, Deserialize, Serialize));

26
crates/bevy_reflect/src/lib.rs
View file

@ -2961,12 +2961,7 @@ bevy_reflect::tests::Test {
let output = to_string_pretty(&ser, config).unwrap();
let expected = r#"
{
"glam::Quat": (
x: 1.0,
y: 2.0,
z: 3.0,
w: 4.0,
),
"glam::Quat": (1.0, 2.0, 3.0, 4.0),
}"#;
assert_eq!(expected, format!("\n{output}"));
@ -2976,12 +2971,7 @@ bevy_reflect::tests::Test {
fn quat_deserialization() {
let data = r#"
{
"glam::Quat": (
x: 1.0,
y: 2.0,
z: 3.0,
w: 4.0,
),
"glam::Quat": (1.0, 2.0, 3.0, 4.0),
}"#;
let mut registry = TypeRegistry::default();
@ -3020,11 +3010,7 @@ bevy_reflect::tests::Test {
let output = to_string_pretty(&ser, config).unwrap();
let expected = r#"
{
"glam::Vec3": (
x: 12.0,
y: 3.0,
z: -6.9,
),
"glam::Vec3": (12.0, 3.0, -6.9),
}"#;
assert_eq!(expected, format!("\n{output}"));
@ -3034,11 +3020,7 @@ bevy_reflect::tests::Test {
fn vec3_deserialization() {
let data = r#"
{
"glam::Vec3": (
x: 12.0,
y: 3.0,
z: -6.9,
),
"glam::Vec3": (12.0, 3.0, -6.9),
}"#;
let mut registry = TypeRegistry::default();

19
crates/bevy_scene/src/dynamic_scene_builder.rs
View file

@ -6,7 +6,7 @@ use bevy_ecs::{
reflect::{AppTypeRegistry, ReflectComponent, ReflectResource},
world::World,
};
use bevy_reflect::PartialReflect;
use bevy_reflect::{PartialReflect, ReflectFromReflect};
use bevy_utils::default;
use std::collections::BTreeMap;
@ -274,11 +274,22 @@ impl<'w> DynamicSceneBuilder<'w> {
return None;
}
let component = type_registry
.get(type_id)?
let type_registration = type_registry.get(type_id)?;
let component = type_registration
.data::<ReflectComponent>()?
.reflect(original_entity)?;
entry.components.push(component.clone_value());
// Clone via `FromReflect`. Unlike `PartialReflect::clone_value` this
// retains the original type and `ReflectSerialize` type data which is needed to
// deserialize.
let component = type_registration
.data::<ReflectFromReflect>()
.and_then(|fr| fr.from_reflect(component.as_partial_reflect()))
.map(PartialReflect::into_partial_reflect)
.unwrap_or_else(|| component.clone_value());
entry.components.push(component);
Some(())
};
extract_and_push();

91
crates/bevy_scene/src/serde.rs
View file

@ -3,11 +3,11 @@
use crate::{DynamicEntity, DynamicScene};
use bevy_ecs::entity::Entity;
use bevy_reflect::serde::{TypedReflectDeserializer, TypedReflectSerializer};
use bevy_reflect::PartialReflect;
use bevy_reflect::{
serde::{ReflectDeserializer, TypeRegistrationDeserializer},
TypeRegistry,
};
use bevy_reflect::{PartialReflect, ReflectFromReflect};
use bevy_utils::HashSet;
use serde::ser::SerializeMap;
use serde::{
@ -488,9 +488,19 @@ impl<'a, 'de> Visitor<'de> for SceneMapVisitor<'a> {
)));
}
entries.push(
map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?,
);
let value =
map.next_value_seed(TypedReflectDeserializer::new(registration, self.registry))?;
// Attempt to convert using FromReflect.
let value = self
.registry
.get(registration.type_id())
.and_then(|tr| tr.data::<ReflectFromReflect>())
.and_then(|fr| fr.from_reflect(value.as_partial_reflect()))
.map(PartialReflect::into_partial_reflect)
.unwrap_or(value);
entries.push(value);
}
Ok(entries)
@ -508,10 +518,10 @@ mod tests {
use bevy_ecs::query::{With, Without};
use bevy_ecs::reflect::{AppTypeRegistry, ReflectMapEntities};
use bevy_ecs::world::FromWorld;
use bevy_reflect::{Reflect, ReflectSerialize};
use bevy_reflect::{Reflect, ReflectDeserialize, ReflectSerialize};
use bincode::Options;
use serde::de::DeserializeSeed;
use serde::Serialize;
use serde::{Deserialize, Serialize};
use std::io::BufReader;
#[derive(Component, Reflect, Default)]
@ -524,6 +534,30 @@ mod tests {
#[reflect(Component)]
struct Baz(i32);
// De/serialize as hex.
mod qux {
use serde::{de::Error, Deserialize, Deserializer, Serializer};
pub fn serialize<S>(value: &u32, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_str(&format!("{:X}", value))
}
pub fn deserialize<'de, D>(deserializer: D) -> Result<u32, D::Error>
where
D: Deserializer<'de>,
{
u32::from_str_radix(<&str as Deserialize>::deserialize(deserializer)?, 16)
.map_err(Error::custom)
}
}
#[derive(Component, Copy, Clone, Reflect, Debug, PartialEq, Serialize, Deserialize)]
#[reflect(Component, Serialize, Deserialize)]
struct Qux(#[serde(with = "qux")] u32);
#[derive(Component, Reflect, Default)]
#[reflect(Component)]
struct MyComponent {
@ -572,6 +606,7 @@ mod tests {
registry.register::<Foo>();
registry.register::<Bar>();
registry.register::<Baz>();
registry.register::<Qux>();
registry.register::<MyComponent>();
registry.register::<MyEnum>();
registry.register::<String>();
@ -696,6 +731,18 @@ mod tests {
assert_eq!(1, dst_world.query::<&Baz>().iter(&dst_world).count());
}
fn roundtrip_ron(world: &World) -> (DynamicScene, DynamicScene) {
let scene = DynamicScene::from_world(world);
let registry = world.resource::<AppTypeRegistry>().read();
let serialized = scene.serialize(&registry).unwrap();
let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap();
let scene_deserializer = SceneDeserializer {
type_registry: &registry,
};
let deserialized_scene = scene_deserializer.deserialize(&mut deserializer).unwrap();
(scene, deserialized_scene)
}
#[test]
fn should_roundtrip_with_later_generations_and_obsolete_references() {
let mut world = create_world();
@ -707,19 +754,7 @@ mod tests {
world.despawn(a);
world.spawn(MyEntityRef(foo)).insert(Bar(123));
let registry = world.resource::<AppTypeRegistry>();
let scene = DynamicScene::from_world(&world);
let serialized = scene
.serialize(&world.resource::<AppTypeRegistry>().read())
.unwrap();
let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap();
let scene_deserializer = SceneDeserializer {
type_registry: &registry.0.read(),
};
let deserialized_scene = scene_deserializer.deserialize(&mut deserializer).unwrap();
let (scene, deserialized_scene) = roundtrip_ron(&world);
let mut map = EntityHashMap::default();
let mut dst_world = create_world();
@ -747,6 +782,24 @@ mod tests {
.all(|r| world.get_entity(r.0).is_none()));
}
#[test]
fn should_roundtrip_with_custom_serialization() {
let mut world = create_world();
let qux = Qux(42);
world.spawn(qux);
let (scene, deserialized_scene) = roundtrip_ron(&world);
assert_eq!(1, deserialized_scene.entities.len());
assert_scene_eq(&scene, &deserialized_scene);
let mut world = create_world();
deserialized_scene
.write_to_world(&mut world, &mut EntityHashMap::default())
.unwrap();
assert_eq!(&qux, world.query::<&Qux>().single(&world));
}
#[test]
fn should_roundtrip_postcard() {
let mut world = create_world();