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

✨πŸ”₯ Implement new converter API #191

Closed pleonex closed 9 months ago

pleonex commented 1 year ago

Description

This PR proposes a new API to use converters on IFormat variables and via the Node class. It tries to address the issues collected from users where the current API can be very verbose or confusing.

The new API is:

static object ConvertFormat.With(Type converterType, dynamic src, params object[] args);

Node Node.TransformWith<TConv>();
// Node Node.TransformWith<TConv>(params object[] args); won't be added, not type safe
Node Node.TransformWith(IConverter converter);
// Node Node.TransformWith<TSrc, TDst>(IConverter<TSrc, TDst> converter); // won't be added, generics can't be omitted
Node Node.TransformWith(Type converterType, params object[] args);

// Extension methods in IFormat
// generics can be omitted, they are inferred from the converter variable and source variable (if it's not object)
TDst ConvertWith<TSrc, TDst>(this TSrc, IConverter<TSrc, TDst> converter, bool disposeInput = false, bool disposeConverter = false);

Examples at the bottom

Main changes:

This new design does not require having dependencies in the core library (Yarhl) with an assembly scanning tech or DI (like MEF)

Cons

Options considered

Requirements analyzed

Description Old API New API
Core library (Yarhl) does not require assembly scanning ❌ βœ…
API does not depend on assemblies in the exe folder (ConvertTo) ❌ βœ…
Generic software (non type aware, like UI) can convert formats and nodes βœ… βœ…
Conversion of IFormat can be chained for multiple conversions ❌ βœ…
The format from a Node can be converted without affecting the node easily ❌ βœ…
Conversion does not require casting to work with the destination type ❌ βœ…
Converters can accept arguments βœ… βœ…
Converters can be made thread-safe ❌ βœ…
Type safety for input/source object ❌ βœ…
Type safety for converter (implements IConverter<,>) ❌ βœ…
Type safety for destination type with converter ❌ βœ…
Type safety for arguments βœ… βœ…
Compile error if missing arguments in converter ❌ βœ…

TODO

Example

// Vanilla API for formats with destination type returned (no need of ConvertFormat)
BinaryFormat binary = new Po2Binary().Convert(po);

// IFormat extension methods
BinaryFormat binary = po.ConvertWith(new Po2Binary());

StringFormatTest test2 = stringFormat
        .ConvertWith(new String2Int())
        .ConvertWith(new Int2String("thisIsAParam"));

// Node
poNode.TransformWith<Po, BinaryFormat>(new Po2Binary()); // source type is not known, so we can't omit generics
poNode.TransformWith(new Po2Binary()); // it uses the `IConverter` overload which is slower (use dynamic)
poNode.TransformWith<Po2Binary>(); // only when a converter has a parameterless constructor

// Arguments on a converter
var options = new Po2BinaryOptions();  // this class doesn't exist (yet?)
BinaryFormat binary = po.ConvertWith(new Po2Binary(options));
poNode.TransformWith<Po, BinaryFormat>(new Po2Binary(options));

// Converting the format of a Node without changing its format
Image<Rgba32> fontImage = overlay11.Format
    .ConvertWith(new Font2Binary(FontKind.Debug))
    .ConvertWith(new Font2Image(), disposeInput: true);

// APIs when we don't know the types at compile-time (generic tools)
Type converterType = ...; // selected via UI or file
object[] args = ...; // deserialized from file or UI

node.TransformWith(converterType);
node.TransformWith(converterType, args);

object result = ConvertFormat.With(converterType, input, args);

Performance results

TransformWith<TSrc, TDst>(IConverter<TSrc, TDst> converter) vs TransformWith(IConverter converter) (uses dynamic):

BenchmarkDotNet=v0.13.1, OS=Windows 10.0.22621
12th Gen Intel Core i7-1260P, 1 CPU, 16 logical and 12 physical cores
.NET SDK=7.0.400
  [Host]     : .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT
  DefaultJob : .NET 6.0.21 (6.0.2123.36311), X64 RyuJIT
Method Mean Error StdDev Gen 0 Allocated
TransformWithDynamic 204.9 ns 1.74 ns 1.54 ns 0.0229 216 B
TransformWithTyped 189.8 ns 2.53 ns 2.37 ns 0.0229 216 B

For now most of the old API is not removed but marked them as obsolete. It allows to do a smooth migration part by part.

Please feel free to provide feedback and try in your projects.

I have temporary modify the build pipeline so there is a NuGet version for this branch available in the preview feed. Version: 4.0.0-PullRequest0191.191 (the last digit may increase)

I'll leave this PR open some time while we test it in our projects, so we can validate it.

Internal validation:

This closes #124

pleonex commented 1 year ago

Good feedback from trying in some projects (PokΓ©mon Conquest and Ekona):