Skip to content

Commit

Permalink
Use FromReflect when extracting entities in dynamic scenes (bevyeng…
Browse files Browse the repository at this point in the history
…ine#15174)

# Objective

Fix bevyengine#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
(bevyengine#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 bevyengine#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::camera::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::camera::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 <[email protected]>
  • Loading branch information
yrns and MrGVSV authored Sep 15, 2024
1 parent 21e3936 commit 2ea51fc
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 84 deletions.
78 changes: 39 additions & 39 deletions crates/bevy_reflect/src/impls/glam.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
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,
y: i32,
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct IVec3 {
x: i32,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -62,15 +62,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct UVec2 {
x: u32,
y: u32,
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct UVec3 {
x: u32,
Expand All @@ -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,
Expand All @@ -90,15 +90,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct U64Vec2 {
x: u64,
y: u64,
}
);
impl_reflect!(
#[reflect(Debug, Hash, PartialEq, Default)]
#[reflect(Debug, Hash, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct U64Vec3 {
x: u64,
Expand All @@ -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,
Expand All @@ -118,15 +118,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Vec2 {
x: f32,
y: f32,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Vec3 {
x: f32,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -155,15 +155,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct BVec2 {
x: bool,
y: bool,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct BVec3 {
x: bool,
Expand All @@ -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,
Expand All @@ -183,15 +183,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DVec2 {
x: f64,
y: f64,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DVec3 {
x: f64,
Expand All @@ -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,
Expand All @@ -211,15 +211,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Mat2 {
x_axis: Vec2,
y_axis: Vec2,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Mat3 {
x_axis: Vec3,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -248,15 +248,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DMat2 {
x_axis: DVec2,
y_axis: DVec2,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DMat3 {
x_axis: DVec3,
Expand All @@ -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,
Expand All @@ -276,15 +276,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Affine2 {
matrix2: Mat2,
translation: Vec2,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct Affine3A {
matrix3: Mat3A,
Expand All @@ -293,15 +293,15 @@ impl_reflect!(
);

impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DAffine2 {
matrix2: DMat2,
translation: DVec2,
}
);
impl_reflect!(
#[reflect(Debug, PartialEq, Default)]
#[reflect(Debug, PartialEq, Default, Deserialize, Serialize)]
#[type_path = "glam"]
struct DAffine3 {
matrix3: DMat3,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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 changes: 4 additions & 22 deletions crates/bevy_reflect/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"));
Expand All @@ -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();
Expand Down Expand Up @@ -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}"));
Expand All @@ -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();
Expand Down
Loading

0 comments on commit 2ea51fc

Please sign in to comment.