bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Reflection is not symmetrical

Open UkoeHB opened this issue 1 year ago • 3 comments

Bevy version

0.14.2

What you did

  1. Define unit struct with reflection and ser/de.
  2. Use TypedReflectDeserializer to deserialize raw JSON into a reflection box.
  3. Use T::from_reflect to get a T out of the box.
  4. Re-serialize T directly to JSON.
  5. Expect the reserialized JSON to equal the starting JSON.

What went wrong

The TypedReflectDeserializer expects unit structs to be JSON arrays. However, JSON serializes unit structs to null. If you try to use null in the original raw JSON, then you get Error("invalid type: null, expected reflected struct value", ...).

Additional information

Minimum reproducible:

bevy = { version = "0.14", default-features = true, features = ["serialize"] }
serde = { version = "1.0" }
serde_json = { version = "1.0", features = ["preserve_order"] }
use std::str::FromStr;
use std::fmt::Debug;

use bevy::{prelude::*, reflect::serde::TypedReflectDeserializer};
use serde::de::DeserializeSeed;

use serde::{Deserialize, Serialize};

#[derive(Reflect, Default, Debug, PartialEq, Serialize, Deserialize)]
pub struct UnitStruct;

fn test_equivalence<T>(w: &World, json_raw: &str, val: T)
where
    T: Serialize + for<'de> Deserialize<'de> + Debug + Reflect + FromReflect + PartialEq
{
    // Json raw to json value
    let json_val = serde_json::Value::from_str(json_raw).unwrap();

    // Json value to reflect
    let type_registry = w.resource::<AppTypeRegistry>().read();
    let registration = type_registry.get(std::any::TypeId::of::<T>()).unwrap();
    let deserializer = TypedReflectDeserializer::new(registration, &type_registry);
    let reflected = deserializer.deserialize(json_val.clone()).unwrap();

    // Reflect to val
    let extracted = T::from_reflect(reflected.as_reflect()).unwrap();
    assert_eq!(val, extracted);

    // Val to json value
    let json_val_deser = serde_json::to_value(&val).unwrap();
    assert_eq!(json_val, json_val_deser);
}

#[test]
fn bug()
{
    let mut app = App::new();
    app.register_type::<UnitStruct>();
    test_equivalence(app.world(), "[]", UnitStruct);
}

UkoeHB avatar Oct 07 '24 21:10 UkoeHB

Looks like we're not using the unit_struct methods on the serializer/deserializer. We can probably do that pretty easy with a simple field length check (being sure to exclude skip_serializing fields).

MrGVSV avatar Oct 07 '24 23:10 MrGVSV

I fixed this bug for bevy 0.15 (currently main) the fix for 0.14.2 is similar but occurs in a different file. (initially I fixed it on my local files and noticed the refacror only after forking main) should I create a seperate PR to fix 0.14.2? is this branch even active?

Thanks, and sorry for the confusion, this is my first open-source PR

pikerpoler avatar Nov 25 '24 16:11 pikerpoler

I fixed this bug for bevy 0.15 (currently main) the fix for 0.14.2 is similar but occurs in a different file. (initially I fixed it on my local files and noticed the refacror only after forking main) should I create a seperate PR to fix 0.14.2? is this branch even active?

Thanks, and sorry for the confusion, this is my first open-source PR

What you did is correct. 0.14.2 is not active. We are working on releasing 0.15 now.

BenjaminBrienen avatar Nov 25 '24 16:11 BenjaminBrienen