palette icon indicating copy to clipboard operation
palette copied to clipboard

Serialization not compatible with many formats because of flatten

Open Frizi opened this issue 6 years ago • 1 comments

There is a long-standing serde bug without an intention to fix, that causes all structs that use #[serde(flatten)] attribute to be interpreted as maps during serialization. Unfortunately, that means that all formats that differentiate between structs and maps cannot properly round-trip palette types through serde. It works on JSON, because it has no distinction between structs and maps.

An example format to test against would be RON (and here is it's related issue). For binary format that suffers from the same issue, check bincode.

In order to fix this, palette needs to implement serialization manually, and use serialize_struct with all the expected fields being serialized directly. That way serde will always treat those types as structs uniformly. This is not fully trivial due to generics, but should be possible with additional crate-private traits that operate on struct serializers adding all necessary fields.

Example RON behaviour of serializing Srgba with current setup:

{"red": 0.5, "green": 1.0, "blue": 0.0, "alpha": 1.0}

Deserialization of above fails, because generated deserializer expects identifiers in place of map keys, but ron always passes strings. In order to deserialze above struct properly, the representation must be manually changed to

{red: 0.5, green: 1.0, blue: 0.0, alpha: 1.0}

With manual serialization implementation, the ideal supported serialized form would be one of those, or ideally both:

Srgba(red: 0.5, green: 1.0, blue: 0.0, alpha: 1.0)

Srgba(0.5, 1.0, 0.0, 1.0)

For reference, this is how amethyst implements transform deserialization as seq and as map to accept both formats as valid in a vector field.

Frizi avatar May 10 '19 14:05 Frizi

Thank you for highlighting this! It should definitely be fixed. I'm all for supporting both formats if it can be done in a nice way (I haven't tried to do a manual implementation in quite a while, so I don't have it fresh in mind).

Ogeon avatar May 12 '19 10:05 Ogeon

A fix has been released in 0.7.1.

Ogeon avatar Apr 16 '23 13:04 Ogeon