Chris3606 / GoRogue

.NET Standard roguelike library in C#. Features many algorithms and data structures pertinent to roguelike/2D game developers, specifically designed to be minimally intrusive upon the developer's architecture.
MIT License
492 stars 30 forks source link

Split TranslationMap and LambdaTranslationMap #26

Closed Chris3606 closed 6 years ago

Chris3606 commented 6 years ago

Currently, TranslationMap and LambdaTranslationMap require specifying both "get" and "set" methods. In many GoRogue algorithms, including pathing, fov, and sense-mapping, "set" functionality is not required, rather only "get" functionality. In these cases it would be convenient to specify translation methods as lambda methods, but only specify the needed "get" functionality, rather than writing unnecessary set functionality (particularly since set functionality may be difficult or lengthy to implement in certain map architectures).

Thus, I propose that it may be beneficial to split TranslationMap into two classes, TranslationMap and SettableTranslationMap. TranslationMap would implement IMapView and require only the "get" function to be specified, whereas SettableTranslationMap would extend that to implement ISettableMapView and require the "set" functionality. Similarly, split LambdaTranslationMap into LambdaTranslationMap and SettableLambdaTranslationMap. These would more closely mirror the IMapView vs. ISettableMapView distinction made in GoRogue, and prevent the need to specify both "get" and "set" functionality to take advantage of the convenience of lambda functions.

Any feedback is welcome! :)

masonwheeler commented 6 years ago

Go ahead if you want to do this. I wrote it the way I did because it's what I needed specifically to create random maps while keeping corridors and rooms separate. Just watch out if you try to do a read-only version of LambdaTranslationMap, because C# will require that the SettableLambdaTranslationMap version have a matching constructor which will be invalid.

Chris3606 commented 6 years ago

I'll look into it. The hope is that the change won't break any code on your end.