FaronBracy / RogueSharp

A .NET Standard class library providing map generation, path-finding, and field-of-view utilities frequently used in roguelikes or 2D tile based games. Inspired by libtcod
https://roguesharp.wordpress.com/
MIT License
574 stars 58 forks source link

Consider unsealing the Point class #30

Open dustinandrews opened 5 years ago

dustinandrews commented 5 years ago

Edit: I guess I should say "convert from struct to class". My editor said it was "sealed" but I checked the code and it's a struct. Maybe you have compelling perf reasons to keep it a struct.

I was very surprised that I couldn't subclass the Point class. I know I can use composition instead. I'm using an ECS pattern where components implement an IComponent interface (for serialization). Some of the components are just points with a different name. LocationComponent and DestinationComponent for example. It would be a lot cleaner for them to inherit point so I can do things like subtract them.

var delta = location - destination entity.AddComponent(delta);

V.S.

var delta = location.point - destination.point var deltaComponent = new DeltaComponent(){point = delta}; entity.AddComponent(deltaComponent)

FaronBracy commented 5 years ago

Thank you for the feedback. It was originally a class but got converted to a struct in this commit:

https://github.com/FaronBracy/RogueSharp/commit/87b1568c09005580cb4be9bbbd67f380b11e0822#diff-336d9b5ef45c6e307febd568f0845ed0

People generally prefer it a struct for "performance" reasons, but in any of my performance profiling I haven't seen a big difference either way. I feel like it's a situation where it won't ever please everyone. I'll need to think about and research it some more before changing it back.

In the meantime could a potential workaround be to override the - operator on your component classes that are composed of points?

dustinandrews commented 5 years ago

If it's not a problem in the profiler, it's not a problem! Anyone who says differently is guilty of premature optimization IMO. In the mean time I wrote a wrapper class so I can make things points and hand them to RogueSharp and back easily. It's not so much that I needed the - operator as I wanted to be able to interchange the points easily.

henrikweimenhog commented 4 years ago

I would much rather prefer an interface which has X and Y. This can then be used by Point, Actor, Cell, Behaviour, Item, etc, etc. It would also be the input to all these methods that takes int x, int y. It greatly improves the extensibility and also gets rid of a lot of code.

ekolis commented 3 years ago

I suppose a record or a ValueTuple could be used, too?