serde support for core types & VariantDispatch by Waridley · Pull Request #743 · godot-rust/gdnative (original) (raw)

Just tried. Vector3 is serialized as a string containing (x, y, z). Arrays are interpreted as arrays no matter what the element count is:

Ah, right, but then Quat, Color, and Rect2 are all stored as "(a, b, c, d)" for some reason. Don't know why they wouldn't then encode Rect2 as "((x, y), (x, y))". It all seems very kludgey, but I guess there's no "best" way to encode complex types in JSON.

I think it's fine to match how the JSON singleton works for the Variant impl, but should you want to go that route, it should be a 1:1 match with no further magic added. imo.

This seems logical, but also, the JSON singleton works fine from Rust anyway, so if you want to encode Variants the way it does, why not just use it?

I think the bigger argument for deserializing sequences as structs may be that it already works to deserialize sequences as concrete structs. The derived impls provide an implementation of deserialize_seq that assumes every field is in order. Therefore, it would be possible to deserialize [a, b, c] as either Vector3::new(a, b, c) or Variant::from_vector3(...). I'm also assuming the derive macros do this perhaps because some formats actually store structs this way? If so, for those formats, MyStruct::serialize -> Variant::deserialize -> MyStruct::from_variant would always fail if we didn't treat sequences as structs, while it could break in a few situations for formats that only treat structs as maps if we don't do that.

Another option is to leave the Variant implementation out for now, and limit the scope of this PR to VariantDispatch.

JSON

[...] I don't see how extra effort is required here?

The problem is that VariantDispatch is not working at all for JSON right now because of the use of serialize_newtype_variant, so strings serialize as GodotString("string"), which JSON fails to serialize as a key.

We could just serialize all GodotString variants directly as strings, but that's likely to break other formats when deserializing.

The other option I see is to use an adjacently tagged representation for VariantDispatch, which then loses the ability to do VariantDispatch::serialize -> Variant::deserialize and get the same value. Although, it's possible that very few formats would have worked with this, anyway. In Ron it only works because deserialize_any on a serialized newtype enum variant can end up treating it as a newtype struct and thus forwarding to the code that differentiates untagged variants. I don't know how many other formats would behave that way. Although, another option to maintain this ability is to check in VariantVisitor::visit_map to see if the map could be a VariantDispatch, and then deserialize the value and throw away the tag. I guess then the only risk is a dictionary that happens to have 2 entries with the names type and value (or whatever keys we choose), which would then serialize in a way that looked like a VariantDispatch and lose the type entry when deserialized.