Your patch is about 700 lines long with lots of chunks and it is difficult to review.
Code: Select all
@@ -961,12 +966,13 @@
* multiple targets that are spread around, we should perform a breadth first
* search by specifiying CalcZero as our heuristic.
*/
-static NPFFoundTargetData NPFRouteInternal(AyStarNode *start1, bool ignore_start_tile1, AyStarNode *start2, bool ignore_start_tile2, NPFFindStationOrTileData *target, AyStar_EndNodeCheck target_proc, AyStar_CalculateH heuristic_proc, AyStarUserData *user, uint reverse_penalty)
+static NPFFoundTargetData NPFRouteInternal(AyStarNode *start1, bool ignore_start_tile1, AyStarNode *start2, bool ignore_start_tile2, NPFFindStationOrTileData *target, AyStar_EndNodeCheck target_proc, AyStar_CalculateH heuristic_proc, AyStarUserData *user, uint reverse_penalty, uint max_penalty)
{
int r;
NPFFoundTargetData result;
/* Initialize procs */
+ _npf_aystar.max_path_cost = max_penalty;
_npf_aystar.CalculateH = heuristic_proc;
_npf_aystar.EndNodeCheck = target_proc;
_npf_aystar.FoundEndNode = NPFSaveTargetData;
Adding a default value for max_penalty (uint max_penalty = 0) will make things easier. Also, you are adding a max_penalty, which should be a separated patch. I already suggested
using the max_penalty some months ago.
Code: Select all
--- src/pathfinder/npf/npf.cpp (revision 27840)
+++ src/pathfinder/npf/npf.cpp (working copy)
@@ -536,8 +536,13 @@
AyStarUserData *user = (AyStarUserData *)as->user_data;
/* It's not worth caching the result with NPF_FLAG_IS_TARGET here as below,
* since checking the cache not that much faster than the actual check */
- return IsDepotTypeTile(current->path.node.tile, user->type) ?
- AYSTAR_FOUND_END_NODE : AYSTAR_DONE;
+ if (user->type != TRANSPORT_WATER) {
+ return IsDepotTypeTile(current->path.node.tile, user->type) ?
+ AYSTAR_FOUND_END_NODE : AYSTAR_DONE;
+ } else {
+ return IsShipDepotTile(current->path.node.tile) && GetShipDepotPart(current->path.node.tile) == DEPOT_PART_NORTH && IsTileOwner(current->path.node.tile, user->owner) ?
+ AYSTAR_FOUND_END_NODE : AYSTAR_DONE;
+ }
}
/** Find any safe and free tile. */
Is this a bug in the original code? Or is it a change you need for your patch?
Code: Select all
@@ -187,7 +194,7 @@
* reverse. The tile given is the tile we are about to enter, enterdir is the
* direction in which we are entering the tile
*/
-Track OPFShipChooseTrack(const Ship *v, TileIndex tile, DiagDirection enterdir, TrackBits tracks, bool &path_found)
+Track OPFShipChooseTrack(const Ship *v, TileIndex tile, DiagDirection enterdir, TrackBits tracks, bool &path_found, TileIndex depottile = INVALID_TILE)
{
assert(IsValidDiagDirection(enterdir));
I don't see why you need that. I don't see depottile used in that function.
I haven't reviewed the rest. The patch is too long but it looks ok for what you are trying to solve.
Last year I tried implementing this same feature and I ended up with something very similar to what you have. Have you tested whether it can cause heavy lag in a game with lots of ships with breakdowns enabled?