Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializing and Deserializing a DynamicScene with a custom serializer fails #6891

Closed
hankjordan opened this issue Dec 8, 2022 · 1 comment
Labels
A-Reflection Runtime information about types A-Scenes Serialized ECS data stored on the disk C-Bug An unexpected or incorrect behavior

Comments

@hankjordan
Copy link
Contributor

hankjordan commented Dec 8, 2022

Here is how I am currently serializing my game state:

pub fn save_state(world: &mut World) {
    let type_registry = world.resource::<AppTypeRegistry>();
    let scene = DynamicScene::from_world(world, type_registry);

    let ser = SceneSerializer::new(&scene, type_registry);

    let mut buf = Vec::new();

    ser.serialize(&mut Serializer::new(&mut buf)).expect("Error serializing scene");

    IoTaskPool::get()
        .spawn(async move {
            File::create(format!("save/gamestate.mp"))
                .and_then(|mut file| file.write(buf.as_slice()))
                .expect("Error writing scene to file");
        })
        .detach();
}

It serializes to the messagepack format perfectly fine, but when I try to deserialize:

fn load_state(registry: &Res<AppTypeRegistry>) -> Result<DynamicScene, ()> {
    let path = "save/gamestate.mp";

    if !Path::new(path).exists() {
        info!("Could not find path");
        return Err(());
    }

    let file = File::open(path).expect("Could not open save file");

    let mut reader = BufReader::new(file);

    let de = SceneDeserializer {
        type_registry: &registry.read(),
    };

    de.deserialize(&mut Deserializer::new(&mut reader)).map_err(|_| ())
}

It doesn't work, returning an Err at de.deserialize.

The error: Value Syntax("invalid type: string \"bevy_transform::components::transform::Transform\", expected a borrowed string")

Here are my imports:

use bevy::{
    prelude::*,
    scene::serde::{SceneSerializer, SceneDeserializer}, tasks::IoTaskPool,
};

use rmp_serde::{Serializer, Deserializer};
use serde::{Serialize, de::DeserializeSeed};

fn main() {
  App::new()
    .add_plugins(DefaultPlugins)
    .add_startup_system(setup_state);
}

fn setup_state(
  registry: Res<AppTypeRegistry>,
) {
  load_state(&registry);
}

Version: 0.9.1
Platform: Linux x86-64

@MrGVSV MrGVSV added C-Bug An unexpected or incorrect behavior A-Reflection Runtime information about types A-Scenes Serialized ECS data stored on the disk labels Dec 8, 2022
@MrGVSV
Copy link
Member

MrGVSV commented Dec 8, 2022

This seems to also affect reflection's deserialization as well. I have a fix and will put up a PR shortly!

bors bot pushed a commit that referenced this issue Jan 4, 2023
# Objective

Fixes #6891

## Solution

Replaces deserializing map keys as `&str` with deserializing them as `String`.

This bug seems to occur when using something like `File` or `BufReader` rather than bytes or a string directly (I only tested `File` and `BufReader` for `rmp-serde` and `serde_json`). This might be an issue with other `Read` impls as well (except `&[u8]` it seems).

We already had passing tests for Message Pack but none that use a `File` or `BufReader`. This PR also adds or modifies tests to check for this in the future.

This change was also based on [feedback](#4561 (comment)) I received in a previous PR.

---

## Changelog

- Fix bug where scene deserialization using certain readers could fail (e.g. `BufReader`, `File`, etc.)
@bors bors bot closed this as completed in 717def2 Jan 4, 2023
alradish pushed a commit to alradish/bevy that referenced this issue Jan 22, 2023
# Objective

Fixes bevyengine#6891

## Solution

Replaces deserializing map keys as `&str` with deserializing them as `String`.

This bug seems to occur when using something like `File` or `BufReader` rather than bytes or a string directly (I only tested `File` and `BufReader` for `rmp-serde` and `serde_json`). This might be an issue with other `Read` impls as well (except `&[u8]` it seems).

We already had passing tests for Message Pack but none that use a `File` or `BufReader`. This PR also adds or modifies tests to check for this in the future.

This change was also based on [feedback](bevyengine#4561 (comment)) I received in a previous PR.

---

## Changelog

- Fix bug where scene deserialization using certain readers could fail (e.g. `BufReader`, `File`, etc.)
ItsDoot pushed a commit to ItsDoot/bevy that referenced this issue Feb 1, 2023
# Objective

Fixes bevyengine#6891

## Solution

Replaces deserializing map keys as `&str` with deserializing them as `String`.

This bug seems to occur when using something like `File` or `BufReader` rather than bytes or a string directly (I only tested `File` and `BufReader` for `rmp-serde` and `serde_json`). This might be an issue with other `Read` impls as well (except `&[u8]` it seems).

We already had passing tests for Message Pack but none that use a `File` or `BufReader`. This PR also adds or modifies tests to check for this in the future.

This change was also based on [feedback](bevyengine#4561 (comment)) I received in a previous PR.

---

## Changelog

- Fix bug where scene deserialization using certain readers could fail (e.g. `BufReader`, `File`, etc.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Reflection Runtime information about types A-Scenes Serialized ECS data stored on the disk C-Bug An unexpected or incorrect behavior
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants