Issue #799 : Make movement aware of tile owners #817
Conversation
| } | ||
|
|
||
| public static float getMovementCost(Tile from, TileDirection dir, Tile newLocation) { | ||
| public static float GetMovementCost(MapUnit unit, Tile from, TileDirection dir, Tile newLocation) { |
There was a problem hiding this comment.
It looks like the only thing needed from the mapunit here is the owner - could we follow the law of Demeter and just pass the player in?
There was a problem hiding this comment.
I get your point, but I will have to disagree on that one. How would we implement something like the Privateer as a land unit (like a spy or something)? Or even the Privateer if we decide to make enemy ocean tiles cost more (and this wouldn't apply to the Privateer)?
And as I am writing this, I am thinking of the Marine unit, maybe it has some implications there as well (or similar unit in the future)?
There was a problem hiding this comment.
We would pass some bit of information about the unit if/when we implement something like marines.
I don't feel super strongly about this, just pointing out that it tends to result in less coupled code.
C7Engine/C7GameData/Player.cs
Outdated
| Player targetTileOwner = targetTile.OwningPlayer(); | ||
| Player sourceTileOwner = sourceTile.OwningPlayer(); | ||
|
|
||
| // any to any && any to own && own to any && own to own |
There was a problem hiding this comment.
I'd personally find a comment like this a bit easier to understand:
// We are free to move if:
// - the tile we are on is unowned, or we own the tile
// - and the tile we are moving to is unowned, or we own the tile
There was a problem hiding this comment.
Yeah, that's better, I just wrote it like this cause it made sense to me in the heat of the moment, I will change it
… move freely on a tile. Clarified a comment
3 participants
Besides the tile ownership, I implemented the Engineering effect of bridging rivers with roads. It seemed logical to include it in there since it affects tile movement cost.