SceneGate / Yarhl

Framework for the implementation of format converters like game assets or media files
https://scenegate.github.io/Yarhl/
MIT License
60 stars 10 forks source link

API enhancements for reflection-based object binary serialization and deserialization #207

Closed pleonex closed 7 months ago

pleonex commented 9 months ago

Goal

Reduce the responsibility of DataReader and DataWriter by extracting the code that (de)serialize objects into a separate class.

Description

The classes DataReader and DataWriter are big and contain a lot of logic. I propose to clarify their responsibility to only handle primitive types and string. This includes byte[] and helper methods like writing padding as it's still integers / bytes.

The logic to serialize / deserialize models / objects (WriteUsingReflection, ReadUsingReflection) based on attributes on properties should be moved into a separate class. That code is similar to what a JsonSerializer or BinaryFormatter would do, so it should be different to the responsibility of DataWriter.

Additionally, we can take this opportunity to improve the API so that:

Proposed solution

Some ideas to improve the design of the API, reduces the complexity, clarify implementation and fixes some of its issues:

The concept is similar to the IConverter<,> concept. We could also consider being an implementation of IConverter<IBinary, object> and IConverter<object, IBinary> but we would need the actual types as a parameter.

Example

Deserializer:

Stream input = ...
MyHeader header = BinaryDeserializer.Deserialize<MyHeader>(input);
// or
var deserializer = new BinaryDeserializer(input);
MyHeader header = deserializer.Deserialize<MyHeader>();
Section section = deserializer.Deserialize<Section>();

Serializer:

Stream output = ...
MyHeader header = ...
BinarySerializer.Serialize(header);
// or
var serializer = new BinarySerializer(output);
serializer.Serialize(header);
serializer.Serialize(section);

Acceptance criteria

Status

pleonex commented 9 months ago

Starting .NET 7.0 (so .NET 8.0) the order of the properties is deterministic (see this PR). However we still support .NET 6.0, so the issue would be there. However, the declared order could be problematic when using base types. Base type property always are returned last, which could not be desirable.

We could keep the BinaryIgnore attribute and create an attribute like BinaryFieldOrder to optionally define the order of the properties. If one property in a type specify it, every property should as well, so the order is clear. The order doesn't need to be incremental, for instance there could be a difference of 10 between on property and another, allowing for base types to define the property in the middle. For .NET 6.0 this attribute would be mandatory.