Closed masonwheeler closed 6 years ago
I must ask, is the breaking not possible to easily fix? Unless there's a bug, it should just be a matter of, instead of passing a translation function to GoalMap at construction, pass a LambdaTranslationMap<T, GoalState> with that same translation function.
If for some reason this isn't viable, let me know and I can provide in another release a second class called GoalMap T or some such that does what I described above, in order to make an upgrade easier. In fact, it might be possible to provide such template versions of many classes, that use LambdaTranslationMap<T, GoalState> in this manner, for convenience.
The logic behind the change, and the reason I don't want to completely revert it, is that GoRogue types, by design, try to avoid having any knowledge of game-side data design and data types they don't need, working with only the minimal amount of information necessary. Just as AStar needs nothing but a boolean for each cell, GoalMap needs nothing but GoalState. This was the point of the IMapView system, so algorithms could be integrated into complex map types, while imposing 0 general limitations on what the arrangement of that data is - not even that there must exist some single type T that represents each coordinate (which actually has indeed not been entirely the case in some projects I've worked on). Thus, GoalMap becomes more versatile by not having a type parameter, and instead relying on IMapView, and allowing LambdaTranslationMap to handle that simple translation case.
I must ask, is the breaking not possible to easily fix? Unless there's a bug, it should just be a matter of, instead of passing a translation function to GoalMap at construction, pass a
LambdaTranslationMap<T, GoalState>
with that same translation function.
That won't work, because I need two input types to determine the goal state: the tile type (to determine barrier/passable status) and the coordinate (to determine goal status, as most goals are either a character or an item.) LambdaTranslationMap
only gives you one input parameter; my original implementation had both.
Agh, I see -- I forgot the original took a Coord, and I generally have my Coord's bundled into type T so didn't notice.
A custom IMapView implementation that checks with a Func(T, Coord) in the this
indexer would definitely work, however since this seems like it could be a common case, would creating LambdaPositionalTranslationMap or some such that is like LambdaTranslationMap but takes Func<T1, Coord> as its argument work in your case? This would preserve the flexibility added by the non-generic type, and hopefully make your situation much easier to implement?
This may also break. Let me do a quick check. I'll get back to you.
EDIT: The above translation map/IMapView implementation should be possible, it would be something like:
public MyMapView<T1, T2> : IMapView<T2>
{
public IMapView<T1> _baseMap { get; }
public Func<T1, Coord> _evaluator { get; }
public T2 this[Coord pos] { get => _evaluator(_baseMap[pos], pos); }
... (Boilerplate)
public MyMapView(IMapView<T1> baseMap, Func<T1, Coord> evaluator) { ... }
}
... where this situation would pass new MyMapView<T, GoalState>(baseMap, evaluator)
to GoalMap.
I'm not entirely sure what to name the class (suggestions are welcome), and at first glance, it might be getting close to what I would consider to be a game-specific situation. But if something like that would work effectively to solve your situation without requiring the (Coord, T) setup of all GoalMap users, I can include it in the IMapView implementations of GoRogue?
Looking into this a bit further, it looks like the LambdaTranslationMap
doesn't provide a coord, but LambdaMapView
does. With that, I can work around this.
Still not particularly happy about it, but I can do it.
I do apologize for any inconvenience or issues it caused. Would you like me to leave the issue open in case further issues with this arise?
No, I think I can work with this.
In this changeset
GoalMap<T>
was changed to a non-genericGoalMap
. I was using it, and this breaks my build. Please revert this change.