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

DynamicScene - Malformed enum when reserializing #12462

Closed
mrchantey opened this issue Mar 13, 2024 · 3 comments
Closed

DynamicScene - Malformed enum when reserializing #12462

mrchantey opened this issue Mar 13, 2024 · 3 comments
Labels
A-Reflection Runtime information about types A-Scenes Serialized ECS data stored on the disk C-Bug An unexpected or incorrect behavior D-Trivial Nice and easy! A great choice to get started with Bevy

Comments

@mrchantey
Copy link
Contributor

mrchantey commented Mar 13, 2024

Bevy version

0.14.0-dev 0.13.1 0.12.1 0.10.0

Relevant system information

  • Confirmed on windows and ubuntu wsl

What you did

  1. Serialize a scene containing an enum that has a tuple variant
  2. Deserialize it
  3. Serialize it again

What went wrong

Enums use the name of the first variant with the value of another:

    #[derive(Reflect, Default)]
    enum MyEnum {
        #[default]
        Unit,
        Tuple(String),
        Struct {
            value: u32,
        },
    }

Will serialize MyEnum::Tuple("Hello World") as Unit("Hello World").

Steps to reproduce

Add the following to the should_roundtrip_bincode test crates\bevy_scene\src\serde.rs:868

        let scene_serializer2 = SceneSerializer::new(&deserialized_scene, &registry.0);
        let serialized_scene2 = bincode::serialize(&scene_serializer2).unwrap();

        println!(
            "malformed enum: {}",
            deserialized_scene.serialize_ron(&registry.0).unwrap()
        );
        assert_eq!(serialized_scene, serialized_scene2);

Will output:

malformed enum: (
  resources: {},
  entities: {
    4294967296: (
      components: {
        "bevy_scene::serde::tests::MyComponent": (
          foo: (1, 2, 3),
          bar: (1.3, 3.7),
          baz: Unit("Hello World!"),
        ),
      },
    ),
  },
)

Additional information

  • Demonstrated using bincode but same behavior for ron, serde_json etc
  • I feel like im missing something obvious because it goes at least to version 0.10, is this intended behavior?
@mrchantey mrchantey added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 13, 2024
@mrchantey mrchantey changed the title Reflect Scene - Malformed tuple reserializing DynamicScene - Malformed tuple reserializing Mar 13, 2024
@alice-i-cecile alice-i-cecile added A-Reflection Runtime information about types A-Scenes Serialized ECS data stored on the disk D-Trivial Nice and easy! A great choice to get started with Bevy and removed S-Needs-Triage This issue needs to be labelled labels Mar 13, 2024
@alice-i-cecile
Copy link
Member

This seems like a pretty straightforward reflection serde bug. If you submit a patch with a test I'll happily review it.

@MrGVSV
Copy link
Member

MrGVSV commented Mar 13, 2024

Just tested it and can confirm this is a bug.

Full repro

Code:

#[derive(Reflect, Default)]
enum MyEnum {
    #[default]
    Unit,
    Tuple(String),
    Struct {
        value: u32,
    },
}

let mut registry = TypeRegistry::default();
registry.register::<MyEnum>();

let value = MyEnum::Tuple("Hello world".to_string());

let serializer = ReflectSerializer::new(&value, &registry);
let serialized = ron::ser::to_string(&serializer).unwrap();
println!("{}", serialized);

let mut deserializer = ron::de::Deserializer::from_str(&serialized).unwrap();
let reflect_deserializer = UntypedReflectDeserializer::new(&registry);
let value = reflect_deserializer.deserialize(&mut deserializer).unwrap();
println!("{:?}", value);

let serializer = ReflectSerializer::new(&*value, &registry);
let serialized = ron::ser::to_string(&serializer).unwrap();
println!("{}", serialized);

Output:

{"bevy_reflect::serde::tests::MyEnum":Tuple("Hello world")}
DynamicEnum(Tuple("Hello world"))
{"bevy_reflect::serde::tests::MyEnum":Unit("Hello world")}

I think this is happening because we don't pass the variant information into the deserialized dynamic output (in this case DynamicEnum).

Edit: Actually this is probably happening in the serializer. We're likely not handling variant names for dynamic types properly

Edit 2: Actually actually this is happening in the deserializer. We're using DynamicEnum::set_variant when we should be using DynamicEnum::set_variant_with_index. Applying this change fixes the issue.

@mrchantey mrchantey changed the title DynamicScene - Malformed tuple reserializing DynamicScene - Malformed enum when reserializing Mar 14, 2024
github-merge-queue bot pushed a commit that referenced this issue Mar 14, 2024
# Objective

- Addresses #12462
- When we serialize an enum, deserialize it, then reserialize it, the
correct variant should be selected.

## Solution

- Change `dynamic_enum.set_variant` to
`dynamic_enum.set_variant_with_index` in `EnumVisitor`
@MrGVSV
Copy link
Member

MrGVSV commented Mar 14, 2024

Closed by #12464

@MrGVSV MrGVSV closed this as completed Mar 14, 2024
mockersf pushed a commit that referenced this issue Mar 15, 2024
# Objective

- Addresses #12462
- When we serialize an enum, deserialize it, then reserialize it, the
correct variant should be selected.

## Solution

- Change `dynamic_enum.set_variant` to
`dynamic_enum.set_variant_with_index` in `EnumVisitor`
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 D-Trivial Nice and easy! A great choice to get started with Bevy
Projects
None yet
Development

No branches or pull requests

3 participants