anders429 / msd Goto Github PK

There are a few composite serde types that are unencodable in the msd format. It would be good to have a list in the documentation somewhere.

Currently, units are serialized as their own empty parameter (and deserialized the same way). This is redundant, as the data will always be the exact same. It also means that certain MSD files are difficult to represent (see .dwi files' #END tag, which has no parameters).

claim seems to be unmaintained, so claims is a fork that fixes a bugs and adds new features. claims should be used in this crate for testing.

Currently, de::Error doesn't handle custom errors well at all. It also doesn't do well even with the serde "canonical" errors. The whole thing could probably use a bit of an overhaul to express these errors more verbosely (currently it just sets the kind to de::error::Kind::Custom, which is not super useful).

Types in this library should implement Debug.

Seems that the clippy CI workflow action has not been configured properly. It runs clippy, but it doesn't actually emit any failures when clippy finds warnings.

In the initial 0.1.0 version, a lot of code for both serialization and deserialization ended up being very similar. It would clean things up significantly to reduce code duplication, possibly by declarative macros and combining some Serializers or Deserializers together (for example, I believe there are a couple Deserializers that are just for identifiers, which could be combined into a single Deserializer).

When deserializing a struct at root, all field names should be emitted, not just field names that are provided by the Deserialize implementation. This will allow ignoring of unknown fields, which currently will always cause errors.

This should only affect root structs. Structs within sequences should continue to work as they are, because the unknown tag name may be part of a field for a struct upstream or something.

Enums with struct variants are serialized, but currently deserialization is not implemented for them at all. The deserialization may need to be restructured to differentiate the different valid serializations.

struct variants are allowed in serialization when serialized at any tag level, essentially. Specifically, they're currently allowed at root level and at root sequence level. This could be expanded in the future to include being allowed in struct fields.

There are a lot of things stored in the various parsing structs that are not necessary. For example, we store starting_position, but every time a new value is requested the value is overwritten with the current value, and is never used outside of that context. The same goes for the various parsing state enums.

The parsing structs could be simplified down to a very bare-bones struct containing just bytes and position, with maybe one or two other fields if needed. A lot of the fat can be cut off here.

Nested structs could be serialized and deserialized, although it would be difficult to differentiate when an old struct ends and a new one begins when there are optional fields involved, due to there being no concept of start and end for sections in MSD. However, I think ambiguity in those situations can be worked with, given that it will increase flexibility of the data format.