GothicKit / ZenKitCS

C#-bindings for ZenKit, the ZenGin asset parser.
https://zk.gothickit.dev/
MIT License
4 stars 1 forks source link

WayNet Edges as Arrays - Migrated to List? #1

Closed JaXt0r closed 9 months ago

JaXt0r commented 9 months ago

ZenKitCS moved away from Arrays[] towards List. Is it intended, that WayNet.WayEdges are an array instead of List? I'd suggest to uniform it as WayPoints are also of type List.

[Serializable]
public struct WayNet
{
    public WayEdge[] Edges;
    public List<WayPoint> Points;
}
lmichaelis commented 9 months ago

This is something I am not quite sure about. The difference comes from the way the data is exposed from C so it basically exposes an implementation detail:

https://github.com/GothicKit/ZenKitCS/blob/8016dbc007486791689aa8a19b3268bb6ad59291/ZenKit/WayNet.cs#L78-L95

Do you have a preference either way?

JaXt0r commented 9 months ago

I checked a few blogs. As Arrays are fixed in size, they should never be resized. As your data is fixed, this is ok. From getting an element via index, Arrays seems slightly faster. e.g.

Let's stick with List<> as the overhead is neglectable.

JaXt0r commented 9 months ago

But thinking about it... Why not using the performance boost. Even if it's small. As the arrays are always fixed in size from ZenKit.

Hopefully the change to Arrays is done in less than 1h for you?

JaXt0r commented 9 months ago

https://stackoverflow.com/a/42753399

image

JaXt0r commented 9 months ago

Any suggestions how you want to progress? Is List the way to go for you based on smaller performance possibilities?

lmichaelis commented 9 months ago

I've switched all raw arrays to Lists for now. Maybe we want to change it later but that's the easiest currently :)

JaXt0r commented 9 months ago

Works. Thanks.