diff --git a/Source/controls/plrctrls.cpp b/Source/controls/plrctrls.cpp index 0e7614b04d5..f597de9247d 100644 --- a/Source/controls/plrctrls.cpp +++ b/Source/controls/plrctrls.cpp @@ -126,9 +126,9 @@ int GetDistance(Point destination, int maxDistance) return 0; } - int8_t walkpath[MaxPathLength]; + int8_t walkpath[MaxPathLengthPlayer]; Player &myPlayer = *MyPlayer; - int steps = FindPath(CanStep, [&myPlayer](Point position) { return PosOkPlayer(myPlayer, position); }, myPlayer.position.future, destination, walkpath); + int steps = FindPath(CanStep, [&myPlayer](Point position) { return PosOkPlayer(myPlayer, position); }, myPlayer.position.future, destination, walkpath, std::min(maxDistance, MaxPathLengthPlayer)); if (steps > maxDistance) return 0; diff --git a/Source/engine/path.cpp b/Source/engine/path.cpp index bfbf5258e4f..08e54607981 100644 --- a/Source/engine/path.cpp +++ b/Source/engine/path.cpp @@ -5,297 +5,172 @@ */ #include "engine/path.h" +#include +#include +#include #include #include -#include #include +#include #include #include "appfat.h" #include "crawl.hpp" -#include "engine/direction.hpp" +#include "engine/displacement.hpp" #include "engine/point.hpp" +#include "utils/algorithm/container.hpp" +#include "utils/static_vector.hpp" namespace devilution { -namespace { -constexpr size_t MaxPathNodes = 300; +// The frame times for axis-aligned and diagonal steps are actually the same. +// +// However, we set the diagonal step cost a bit higher to avoid +// excessive diagonal movement. For example, the frame times for these +// two paths are the same: ↑↑ and ↗↖. However, ↑↑ looks more natural. +const int PathAxisAlignedStepCost = 100; +const int PathDiagonalStepCost = 101; -struct PathNode { - static constexpr uint16_t InvalidIndex = std::numeric_limits::max(); - static constexpr size_t MaxChildren = 8; +namespace { - int16_t x = 0; - int16_t y = 0; - uint16_t parentIndex = InvalidIndex; - uint16_t childIndices[MaxChildren] = { InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex, InvalidIndex }; - uint16_t nextNodeIndex = InvalidIndex; - uint8_t f = 0; - uint8_t h = 0; - uint8_t g = 0; +constexpr size_t MaxPathNodes = 1024; - [[nodiscard]] Point position() const - { - return Point { x, y }; - } +using NodeIndexType = uint16_t; +using CoordType = uint8_t; +using CostType = uint16_t; +using PointT = PointOf; - void addChild(uint16_t childIndex) - { - size_t index = 0; - for (; index < MaxChildren; ++index) { - if (childIndices[index] == InvalidIndex) - break; - } - assert(index < MaxChildren); - childIndices[index] = childIndex; - } +struct FrontierNode { + PointT position; + + // Current best guess of the cost of the path to destination + // if it goes through this node. + CostType f; }; -PathNode PathNodes[MaxPathNodes]; +struct ExploredNode { + // Preceding node (needed to reconstruct the path at the end). + PointT prev; -/** A linked list of the A* frontier, sorted by distance */ -PathNode *Path2Nodes; + // The current lowest cost from start to this node (0 for the start node). + CostType g; +}; -/** - * @brief return a node for a position on the frontier, or `PathNode::InvalidIndex` if not found - */ -uint16_t GetNode1(Point targetPosition) -{ - uint16_t result = Path2Nodes->nextNodeIndex; - while (result != PathNode::InvalidIndex) { - if (PathNodes[result].position() == targetPosition) - return result; - result = PathNodes[result].nextNodeIndex; - } - return PathNode::InvalidIndex; -} +// A simple map with a fixed number of buckets and static storage. +class ExploredNodes { + static const size_t NumBuckets = 64; + static const size_t BucketCapacity = 3 * MaxPathNodes / NumBuckets; + using Entry = std::pair; + using Bucket = StaticVector; -/** - * @brief insert `front` node into the frontier (keeping the frontier sorted by total distance) - */ -void NextNode(uint16_t front) -{ - if (Path2Nodes->nextNodeIndex == PathNode::InvalidIndex) { - Path2Nodes->nextNodeIndex = front; - return; - } +public: + using value_type = Entry; + using iterator = value_type *; + using const_iterator = const value_type *; - PathNode *current = Path2Nodes; - uint16_t nextIndex = Path2Nodes->nextNodeIndex; - const uint8_t maxF = PathNodes[front].f; - while (nextIndex != PathNode::InvalidIndex && PathNodes[nextIndex].f < maxF) { - current = &PathNodes[nextIndex]; - nextIndex = current->nextNodeIndex; + [[nodiscard]] const_iterator find(const PointT &point) const + { + const Bucket &b = bucket(point); + const auto it = c_find_if(b, [r = repr(point)](const Entry &e) { return e.first == r; }); + if (it == b.end()) return nullptr; + return it; + } + [[nodiscard]] iterator find(const PointT &point) + { + Bucket &b = bucket(point); + auto it = c_find_if(b, [r = repr(point)](const Entry &e) { return e.first == r; }); + if (it == b.end()) return nullptr; + return it; } - PathNodes[front].nextNodeIndex = nextIndex; - current->nextNodeIndex = front; -} -/** A linked list of all visited nodes */ -PathNode *VisitedNodes; + // NOLINTNEXTLINE(readability-convert-member-functions-to-static) + [[nodiscard]] const_iterator end() const { return nullptr; } + // NOLINTNEXTLINE(readability-convert-member-functions-to-static) + [[nodiscard]] iterator end() { return nullptr; } -/** - * @brief return a node for this position if it was visited, or NULL if not found - */ -uint16_t GetNode2(Point targetPosition) -{ - uint16_t result = VisitedNodes->nextNodeIndex; - while (result != PathNode::InvalidIndex) { - if (PathNodes[result].position() == targetPosition) - return result; - result = PathNodes[result].nextNodeIndex; + void emplace(const PointT &point, const ExploredNode &exploredNode) + { + bucket(point).emplace_back(repr(point), exploredNode); } - return result; -} -/** - * @brief get the next node on the A* frontier to explore (estimated to be closest to the goal), mark it as visited, and return it - */ -uint16_t GetNextPath() -{ - uint16_t result = Path2Nodes->nextNodeIndex; - if (result == PathNode::InvalidIndex) { - return result; + [[nodiscard]] bool canInsert(const PointT &point) const + { + return bucket(point).size() < BucketCapacity; } - Path2Nodes->nextNodeIndex = PathNodes[result].nextNodeIndex; - PathNodes[result].nextNodeIndex = VisitedNodes->nextNodeIndex; - VisitedNodes->nextNodeIndex = result; - return result; -} - -/** the number of in-use nodes in path_nodes */ -uint32_t gdwCurNodes; -/** - * @brief zero one of the preallocated nodes and return a pointer to it, or NULL if none are available - */ -uint16_t NewStep() -{ - if (gdwCurNodes >= MaxPathNodes) - return PathNode::InvalidIndex; +private: + [[nodiscard]] const Bucket &bucket(const PointT &point) const { return buckets_[bucketIndex(point)]; } + [[nodiscard]] Bucket &bucket(const PointT &point) { return buckets_[bucketIndex(point)]; } + [[nodiscard]] static size_t bucketIndex(const PointT &point) + { + return ((point.x & 0b111) << 3) | (point.y & 0b111); + } - PathNodes[gdwCurNodes] = {}; - return gdwCurNodes++; -} + [[nodiscard]] static uint16_t repr(const PointT &point) + { + return (point.x << 8) | point.y; + } -/** A stack for recursively searching nodes */ -uint16_t pnode_tblptr[MaxPathNodes]; -/** size of the pnode_tblptr stack */ -uint32_t gdwCurPathStep; -/** - * @brief push pPath onto the pnode_tblptr stack - */ -void PushActiveStep(uint16_t pPath) -{ - assert(gdwCurPathStep < MaxPathNodes); - pnode_tblptr[gdwCurPathStep] = pPath; - gdwCurPathStep++; -} + std::array buckets_; +}; -/** - * @brief pop and return a node from the pnode_tblptr stack - */ -uint16_t PopActiveStep() +bool IsDiagonalStep(const Point &a, const Point &b) { - gdwCurPathStep--; - return pnode_tblptr[gdwCurPathStep]; + return a.x != b.x && a.y != b.y; } /** * @brief Returns the distance between 2 adjacent nodes. - * - * The distance is 2 for nodes in the same row or column, - * and 3 for diagonally adjacent nodes. - * - * This approximates that diagonal movement on a square grid should have a cost - * of sqrt(2). That's approximately 1.5, so they multiply all step costs by 2, - * except diagonal steps which are times 3 */ -int GetDistance(Point startPosition, Point destinationPosition) +CostType GetDistance(PointT startPosition, PointT destinationPosition) { - if (startPosition.x == destinationPosition.x || startPosition.y == destinationPosition.y) - return 2; - - return 3; + return IsDiagonalStep(startPosition, destinationPosition) + ? PathDiagonalStepCost + : PathAxisAlignedStepCost; } /** * @brief heuristic, estimated cost from startPosition to destinationPosition. */ -int GetHeuristicCost(Point startPosition, Point destinationPosition) -{ - // see GetDistance for why this is times 2 - return 2 * startPosition.ManhattanDistance(destinationPosition); -} - -/** - * @brief update all path costs using depth-first search starting at pPath - */ -void SetCoords(tl::function_ref canStep, uint16_t pPath) +CostType GetHeuristicCost(PointT startPosition, PointT destinationPosition) { - PushActiveStep(pPath); - // while there are path nodes to check - while (gdwCurPathStep > 0) { - uint16_t pathOldIndex = PopActiveStep(); - const PathNode &pathOld = PathNodes[pathOldIndex]; - for (uint16_t childIndex : pathOld.childIndices) { - if (childIndex == PathNode::InvalidIndex) - break; - PathNode &pathAct = PathNodes[childIndex]; - - if (pathOld.g + GetDistance(pathOld.position(), pathAct.position()) < pathAct.g) { - if (canStep(pathOld.position(), pathAct.position())) { - pathAct.parentIndex = pathOldIndex; - pathAct.g = pathOld.g + GetDistance(pathOld.position(), pathAct.position()); - pathAct.f = pathAct.g + pathAct.h; - PushActiveStep(childIndex); - } - } - } - } + // This function needs to be admissible, i.e. it should never over-estimate + // the distance. + // + // This calculation assumes we can take diagonal steps until we reach + // the same row or column and then take the remaining axis-aligned steps. + const int dx = std::abs(static_cast(startPosition.x) - static_cast(destinationPosition.x)); + const int dy = std::abs(static_cast(startPosition.y) - static_cast(destinationPosition.y)); + const int diagSteps = std::min(dx, dy); + + // After we've taken `diagSteps`, the remaining steps in one coordinate + // will be zero, and in the other coordinate it will be reduced by `diagSteps`. + // We then still need to take the remaining steps: + // max(dx, dy) - diagSteps = max(dx, dy) - min(dx, dy) = abs(dx - dy) + const int axisAlignedSteps = std::abs(dx - dy); + return diagSteps * PathDiagonalStepCost + axisAlignedSteps * PathAxisAlignedStepCost; } -/** - * @brief add a step from pPath to destination, return 1 if successful, and update the frontier/visited nodes accordingly - * - * @param canStep specifies whether a step between two adjacent points is allowed - * @param pathIndex index of the current path node - * @param candidatePosition expected to be a neighbour of the current path node position - * @param destinationPosition where we hope to end up - * @return true if step successfully added, false if we ran out of nodes to use - */ -bool ExploreFrontier(tl::function_ref canStep, uint16_t pathIndex, Point candidatePosition, Point destinationPosition) +int ReconstructPath(const ExploredNodes &explored, PointT dest, int8_t *path, size_t maxPathLength) { - PathNode &path = PathNodes[pathIndex]; - int nextG = path.g + GetDistance(path.position(), candidatePosition); - - // 3 cases to consider - // case 1: (dx,dy) is already on the frontier - uint16_t dxdyIndex = GetNode1(candidatePosition); - if (dxdyIndex != PathNode::InvalidIndex) { - path.addChild(dxdyIndex); - PathNode &dxdy = PathNodes[dxdyIndex]; - if (nextG < dxdy.g) { - if (canStep(path.position(), candidatePosition)) { - // we'll explore it later, just update - dxdy.parentIndex = pathIndex; - dxdy.g = nextG; - dxdy.f = nextG + dxdy.h; - } - } - } else { - // case 2: (dx,dy) was already visited - dxdyIndex = GetNode2(candidatePosition); - if (dxdyIndex != PathNode::InvalidIndex) { - path.addChild(dxdyIndex); - PathNode &dxdy = PathNodes[dxdyIndex]; - if (nextG < dxdy.g && canStep(path.position(), candidatePosition)) { - // update the node - dxdy.parentIndex = pathIndex; - dxdy.g = nextG; - dxdy.f = nextG + dxdy.h; - // already explored, so re-update others starting from that node - SetCoords(canStep, dxdyIndex); - } - } else { - // case 3: (dx,dy) is totally new - dxdyIndex = NewStep(); - if (dxdyIndex == PathNode::InvalidIndex) - return false; - PathNode &dxdy = PathNodes[dxdyIndex]; - dxdy.parentIndex = pathIndex; - dxdy.g = nextG; - dxdy.h = GetHeuristicCost(candidatePosition, destinationPosition); - dxdy.f = nextG + dxdy.h; - dxdy.x = static_cast(candidatePosition.x); - dxdy.y = static_cast(candidatePosition.y); - // add it to the frontier - NextNode(dxdyIndex); - path.addChild(dxdyIndex); + size_t len = 0; + PointT cur = dest; + while (true) { + const auto it = explored.find(cur); + if (it == explored.end()) app_fatal("Failed to reconstruct path"); + if (it->second.g == 0) break; // reached start + path[len++] = GetPathDirection(it->second.prev, cur); + cur = it->second.prev; + if (len == maxPathLength) { + // Path too long. + len = 0; + break; } } - return true; -} - -/** - * @brief perform a single step of A* bread-first search by trying to step in every possible direction from pPath with goal (x,y). Check each step with PosOk - * - * @return false if we ran out of preallocated nodes to use, else true - */ -bool GetPath(tl::function_ref canStep, tl::function_ref posOk, uint16_t pathIndex, Point destination) -{ - for (Displacement dir : PathDirs) { - const PathNode &path = PathNodes[pathIndex]; - const Point tile = path.position() + dir; - const bool ok = posOk(tile); - if ((ok && canStep(path.position(), tile)) || (!ok && tile == destination)) { - if (!ExploreFrontier(canStep, pathIndex, tile, destination)) - return false; - } - } - - return true; + std::reverse(path, path + len); + std::fill(path + len, path + maxPathLength, -1); + return len; } } // namespace @@ -306,54 +181,105 @@ int8_t GetPathDirection(Point startPosition, Point destinationPosition) return PathDirections[3 * (destinationPosition.y - startPosition.y) + 4 + destinationPosition.x - startPosition.x]; } -int FindPath(tl::function_ref canStep, tl::function_ref posOk, Point startPosition, Point destinationPosition, int8_t path[MaxPathLength]) +int FindPath(tl::function_ref canStep, tl::function_ref posOk, Point startPosition, Point destinationPosition, int8_t *path, size_t maxPathLength) { - /** - * for reconstructing the path after the A* search is done. The longest - * possible path is actually 24 steps, even though we can fit 25 - */ - static int8_t pnodeVals[MaxPathLength]; - - // clear all nodes, create root nodes for the visited/frontier linked lists - gdwCurNodes = 0; - Path2Nodes = &PathNodes[NewStep()]; - VisitedNodes = &PathNodes[NewStep()]; - gdwCurPathStep = 0; - const uint16_t pathStartIndex = NewStep(); - PathNode &pathStart = PathNodes[pathStartIndex]; - pathStart.x = static_cast(startPosition.x); - pathStart.y = static_cast(startPosition.y); - pathStart.f = pathStart.h + pathStart.g; - pathStart.h = GetHeuristicCost(startPosition, destinationPosition); - pathStart.g = 0; - Path2Nodes->nextNodeIndex = pathStartIndex; - // A* search until we find (dx,dy) or fail - uint16_t nextNodeIndex; - while ((nextNodeIndex = GetNextPath()) != PathNode::InvalidIndex) { - // reached the end, success! - if (PathNodes[nextNodeIndex].position() == destinationPosition) { - const PathNode *current = &PathNodes[nextNodeIndex]; - size_t pathLength = 0; - while (current->parentIndex != PathNode::InvalidIndex) { - if (pathLength >= MaxPathLength) - break; - pnodeVals[pathLength++] = GetPathDirection(PathNodes[current->parentIndex].position(), current->position()); - current = &PathNodes[current->parentIndex]; + const PointT start { startPosition }; + const PointT dest { destinationPosition }; + + const CostType initialHeuristicCost = GetHeuristicCost(start, dest); + if (initialHeuristicCost > PathDiagonalStepCost * maxPathLength) { + // Heuristic cost never underestimates the true cost, so we can give up early. + return 0; + } + + StaticVector frontier; + ExploredNodes explored; + { + frontier.emplace_back(FrontierNode { .position = start, .f = initialHeuristicCost }); + explored.emplace(start, ExploredNode { .prev = {}, .g = 0 }); + } + + const auto frontierComparator = [&explored, &dest](const FrontierNode &a, const FrontierNode &b) { + // We use heap functions from which form a max-heap. + // We reverse the comparison sign here to get a min-heap. + if (a.f != b.f) return a.f > b.f; + + // For nodes with the same f-score, prefer the ones with lower + // heuristic cost (likely to be closer to the goal). + const CostType hA = GetHeuristicCost(a.position, dest); + const CostType hB = GetHeuristicCost(b.position, dest); + if (hA != hB) return hA > hB; + + // Prefer diagonal steps first. + const ExploredNode &aInfo = explored.find(a.position)->second; + const ExploredNode &bInfo = explored.find(b.position)->second; + const bool isDiagonalA = IsDiagonalStep(aInfo.prev, a.position); + const bool isDiagonalB = IsDiagonalStep(bInfo.prev, b.position); + if (isDiagonalA != isDiagonalB) return isDiagonalB; + + // Finally, disambiguate by coordinate: + if (a.position.x != b.position.x) return a.position.x > b.position.x; + return a.position.y > b.position.y; + }; + + while (!frontier.empty()) { + const FrontierNode cur = frontier.front(); // argmin(node.f) for node in openSet + + if (cur.position == destinationPosition) { + return ReconstructPath(explored, cur.position, path, maxPathLength); + } + + std::pop_heap(frontier.begin(), frontier.end(), frontierComparator); + frontier.pop_back(); + const CostType curG = explored.find(cur.position)->second.g; + + // Discard invalid nodes. + + // If this node is already at the maximum number of steps, we can skip processing it. + // We don't keep track of the maximum number of steps, so we approximate it. + if (curG >= PathDiagonalStepCost * maxPathLength) continue; + + // When we discover a better path to a node, we push the node to the heap + // with the new `f` value even if the node is already in the heap. + if (curG + GetHeuristicCost(cur.position, dest) > cur.f) continue; + + for (const DisplacementOf d : PathDirs) { + // We're using `uint8_t` for coordinates. Avoid underflow: + if ((cur.position.x == 0 && d.deltaX < 0) || (cur.position.y == 0 && d.deltaY < 0)) continue; + const PointT neighborPos = cur.position + d; + const bool ok = posOk(neighborPos); + if (ok) { + if (!canStep(cur.position, neighborPos)) continue; + } else { + // We allow targeting a non-walkable node if it is the destination. + if (neighborPos != dest) continue; } - if (pathLength != MaxPathLength) { - size_t i; - for (i = 0; i < pathLength; i++) - path[i] = pnodeVals[pathLength - i - 1]; - return static_cast(i); + const CostType g = curG + GetDistance(cur.position, neighborPos); + if (curG >= PathDiagonalStepCost * maxPathLength) continue; + bool improved = false; + if (auto it = explored.find(neighborPos); it == explored.end()) { + if (explored.canInsert(neighborPos)) { + explored.emplace(neighborPos, ExploredNode { .prev = cur.position, .g = g }); + improved = true; + } + } else if (it->second.g > g) { + it->second.prev = cur.position; + it->second.g = g; + improved = true; + } + if (improved) { + const CostType f = g + GetHeuristicCost(neighborPos, dest); + if (frontier.size() < MaxPathNodes) { + // We always push the node to the heap, even if the same position already exists in it. + // When popping from the heap, we discard invalid nodes by checking that `g + h <= f`. + frontier.emplace_back(FrontierNode { .position = neighborPos, .f = f }); + std::push_heap(frontier.begin(), frontier.end(), frontierComparator); + } } - return 0; } - // ran out of nodes, abort! - if (!GetPath(canStep, posOk, nextNodeIndex, destinationPosition)) - return 0; } - // frontier is empty, no path! - return 0; + + return 0; // no path } std::optional FindClosestValidPosition(tl::function_ref posOk, Point startingPosition, unsigned int minimumRadius, unsigned int maximumRadius) diff --git a/Source/engine/path.h b/Source/engine/path.h index 27b9cc16783..e85f0a821ce 100644 --- a/Source/engine/path.h +++ b/Source/engine/path.h @@ -5,6 +5,7 @@ */ #pragma once +#include #include #include @@ -15,7 +16,14 @@ namespace devilution { -constexpr size_t MaxPathLength = 25; +constexpr size_t MaxPathLengthMonsters = 25; +constexpr size_t MaxPathLengthPlayer = 100; + +// Cost for an axis-aligned step (up/down/left/right). Visible for testing. +extern const int PathAxisAlignedStepCost; + +// Cost for a diagonal step. Visible for testing. +extern const int PathDiagonalStepCost; /** * @brief Find the shortest path from `startPosition` to `destinationPosition`. @@ -24,10 +32,11 @@ constexpr size_t MaxPathLength = 25; * @param posOk specifies whether a position can be stepped on. * @param startPosition * @param destinationPosition - * @param path Resulting path represented as the step directions, which are indices in `PathDirs`. Must have room for `MaxPathLength` steps. + * @param path Resulting path represented as the step directions, which are indices in `PathDirs`. Must have room for `maxPathLength` steps. + * @param maxPathLength The maximum allowed length of the resulting path. * @return The length of the resulting path, or 0 if there is no valid path. */ -int FindPath(tl::function_ref canStep, tl::function_ref posOk, Point startPosition, Point destinationPosition, int8_t path[MaxPathLength]); +int FindPath(tl::function_ref canStep, tl::function_ref posOk, Point startPosition, Point destinationPosition, int8_t *path, size_t maxPathLength); /** For iterating over the 8 possible movement directions */ const Displacement PathDirs[8] = { diff --git a/Source/loadsave.cpp b/Source/loadsave.cpp index b485e2468ff..2a43a42c125 100644 --- a/Source/loadsave.cpp +++ b/Source/loadsave.cpp @@ -6,6 +6,7 @@ #include "loadsave.h" #include +#include #include #include #include @@ -50,6 +51,7 @@ uint8_t giNumberOfLevels; namespace { constexpr size_t MaxMissilesForSaveGame = 125; +constexpr size_t PlayerWalkPathSizeForSaveGame = 25; uint8_t giNumberQuests; uint8_t giNumberOfSmithPremiumItems; @@ -345,9 +347,11 @@ void LoadPlayer(LoadHelper &file, Player &player) { player._pmode = static_cast(file.NextLE()); - for (int8_t &step : player.walkpath) { - step = file.NextLE(); + for (size_t i = 0; i < PlayerWalkPathSizeForSaveGame; ++i) { + player.walkpath[i] = file.NextLE(); } + player.walkpath[PlayerWalkPathSizeForSaveGame] = WALK_NONE; + player.plractive = file.NextBool8(); file.Skip(2); // Alignment player.destAction = static_cast(file.NextLE()); @@ -1147,8 +1151,9 @@ void SaveItem(SaveHelper &file, const Item &item) void SavePlayer(SaveHelper &file, const Player &player) { file.WriteLE(player._pmode); - for (int8_t step : player.walkpath) - file.WriteLE(step); + for (size_t i = 0; i < PlayerWalkPathSizeForSaveGame; ++i) { + file.WriteLE(player.walkpath[i]); + } file.WriteLE(player.plractive ? 1 : 0); file.Skip(2); // Alignment file.WriteLE(player.destAction); diff --git a/Source/monster.cpp b/Source/monster.cpp index fcf4c560096..c6adb1dfed9 100644 --- a/Source/monster.cpp +++ b/Source/monster.cpp @@ -1690,12 +1690,12 @@ bool IsTileAccessible(const Monster &monster, Point position) bool AiPlanWalk(Monster &monster) { - int8_t path[MaxPathLength]; + int8_t path[MaxPathLengthMonsters]; /** Maps from walking path step to facing direction. */ const Direction plr2monst[9] = { Direction::South, Direction::NorthEast, Direction::NorthWest, Direction::SouthEast, Direction::SouthWest, Direction::North, Direction::East, Direction::South, Direction::West }; - if (FindPath(CanStep, [&monster](Point position) { return IsTileAccessible(monster, position); }, monster.position.tile, monster.enemyPosition, path) == 0) { + if (FindPath(CanStep, [&monster](Point position) { return IsTileAccessible(monster, position); }, monster.position.tile, monster.enemyPosition, path, MaxPathLengthMonsters) == 0) { return false; } diff --git a/Source/player.cpp b/Source/player.cpp index 8325f90a07a..e5c3dcc12c4 100644 --- a/Source/player.cpp +++ b/Source/player.cpp @@ -1182,11 +1182,11 @@ void CheckNewPath(Player &player, bool pmWillBeCalled) break; } - for (size_t j = 1; j < MaxPathLength; j++) { + for (size_t j = 1; j < MaxPathLengthPlayer; j++) { player.walkpath[j - 1] = player.walkpath[j]; } - player.walkpath[MaxPathLength - 1] = WALK_NONE; + player.walkpath[MaxPathLengthPlayer - 1] = WALK_NONE; if (player._pmode == PM_STAND) { StartStand(player, player._pdir); @@ -1923,8 +1923,8 @@ void Player::UpdatePreviewCelSprite(_cmd_id cmdId, Point point, uint16_t wParam1 } if (minimalWalkDistance >= 0 && position.future != point) { - int8_t testWalkPath[MaxPathLength]; - int steps = FindPath(CanStep, [this](Point position) { return PosOkPlayer(*this, position); }, position.future, point, testWalkPath); + int8_t testWalkPath[MaxPathLengthPlayer]; + int steps = FindPath(CanStep, [this](Point position) { return PosOkPlayer(*this, position); }, position.future, point, testWalkPath, MaxPathLengthPlayer); if (steps == 0) { // Can't walk to desired location => stand still return; @@ -3057,7 +3057,7 @@ void MakePlrPath(Player &player, Point targetPosition, bool endspace) return; } - int path = FindPath(CanStep, [&player](Point position) { return PosOkPlayer(player, position); }, player.position.future, targetPosition, player.walkpath); + int path = FindPath(CanStep, [&player](Point position) { return PosOkPlayer(player, position); }, player.position.future, targetPosition, player.walkpath, MaxPathLengthPlayer); if (path == 0) { return; } diff --git a/Source/player.h b/Source/player.h index 8f9e838f06b..29f7f8ccd3a 100644 --- a/Source/player.h +++ b/Source/player.h @@ -265,7 +265,7 @@ struct Player { int _pILMaxDam; uint32_t _pExperience; PLR_MODE _pmode; - int8_t walkpath[MaxPathLength]; + int8_t walkpath[MaxPathLengthPlayer]; bool plractive; action_id destAction; int destParam1; diff --git a/Source/utils/static_vector.hpp b/Source/utils/static_vector.hpp index 5e894dabbab..b57fcf8afe9 100644 --- a/Source/utils/static_vector.hpp +++ b/Source/utils/static_vector.hpp @@ -22,7 +22,12 @@ class StaticVector { using value_type = T; using reference = T &; using const_reference = const T &; + using pointer = T *; + using const_pointer = const T *; using size_type = size_t; + using iterator = T *; + using const_iterator = const T *; + using difference_type = std::ptrdiff_t; StaticVector() = default; diff --git a/test/fixtures/timedemo/WarriorLevel1to2/demo_0.dmo b/test/fixtures/timedemo/WarriorLevel1to2/demo_0.dmo index 252b10bc15b..33c8812f2d8 100644 Binary files a/test/fixtures/timedemo/WarriorLevel1to2/demo_0.dmo and b/test/fixtures/timedemo/WarriorLevel1to2/demo_0.dmo differ diff --git a/test/fixtures/timedemo/WarriorLevel1to2/demo_0_reference_spawn_0.sv b/test/fixtures/timedemo/WarriorLevel1to2/demo_0_reference_spawn_0.sv index cc856ef856f..8aaa3163309 100644 Binary files a/test/fixtures/timedemo/WarriorLevel1to2/demo_0_reference_spawn_0.sv and b/test/fixtures/timedemo/WarriorLevel1to2/demo_0_reference_spawn_0.sv differ diff --git a/test/fixtures/timedemo/WarriorLevel1to2/spawn_0.sv b/test/fixtures/timedemo/WarriorLevel1to2/spawn_0.sv index d9c9c78c28d..db7d4c4e179 100644 Binary files a/test/fixtures/timedemo/WarriorLevel1to2/spawn_0.sv and b/test/fixtures/timedemo/WarriorLevel1to2/spawn_0.sv differ diff --git a/test/path_benchmark.cpp b/test/path_benchmark.cpp index 0db673e2330..e9e3caa7201 100644 --- a/test/path_benchmark.cpp +++ b/test/path_benchmark.cpp @@ -1,3 +1,4 @@ +#include #include #include @@ -37,10 +38,11 @@ void BenchmarkMap(const Map &map, benchmark::State &state) { const auto [start, dest] = FindStartDest(map); const auto posOk = /*posOk=*/[&map](Point p) { return map[p] != '#'; }; + constexpr size_t MaxPathLength = 25; for (auto _ : state) { int8_t path[MaxPathLength]; int result = FindPath(/*canStep=*/[](Point, Point) { return true; }, - posOk, start, dest, path); + posOk, start, dest, path, MaxPathLength); benchmark::DoNotOptimize(result); } } @@ -114,9 +116,48 @@ void BM_NoPath(benchmark::State &state) state); } +void BM_NoPathBig(benchmark::State &state) +{ + BenchmarkMap( + Map { + Size { 30, 30 }, + "##############################" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#............................#" + "#.....S......................#" + "##############################" + "#.....E......................#" + "##############################" }, + state); +} + BENCHMARK(BM_SinglePath); BENCHMARK(BM_Bridges); BENCHMARK(BM_NoPath); +BENCHMARK(BM_NoPathBig); } // namespace } // namespace devilution diff --git a/test/path_test.cpp b/test/path_test.cpp index 447ab7a8408..dd457d3d935 100644 --- a/test/path_test.cpp +++ b/test/path_test.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -25,29 +26,29 @@ TEST(PathTest, Heuristics) EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 0) << "Wrong cost for travelling to the same tile"; destination = source + Direction::NorthEast; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 2) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathAxisAlignedStepCost) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; destination = source + Direction::SouthEast; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 2) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathAxisAlignedStepCost) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; destination = source + Direction::SouthWest; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 2) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathAxisAlignedStepCost) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; destination = source + Direction::NorthWest; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 2) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathAxisAlignedStepCost) << "Wrong cost for travelling to horizontal/vertical adjacent tile"; destination = source + Direction::North; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 4) << "Wrong cost for travelling to diagonally adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathDiagonalStepCost) << "Wrong cost for travelling to diagonally adjacent tile"; destination = source + Direction::East; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 4) << "Wrong cost for travelling to diagonally adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathDiagonalStepCost) << "Wrong cost for travelling to diagonally adjacent tile"; destination = source + Direction::South; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 4) << "Wrong cost for travelling to diagonally adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathDiagonalStepCost) << "Wrong cost for travelling to diagonally adjacent tile"; destination = source + Direction::West; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 4) << "Wrong cost for travelling to diagonally adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathDiagonalStepCost) << "Wrong cost for travelling to diagonally adjacent tile"; destination = source + Direction::SouthWest + Direction::SouthEast; // Effectively the same as Direction::South - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 4) << "Wrong cost for travelling to diagonally adjacent tile"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathDiagonalStepCost) << "Wrong cost for travelling to diagonally adjacent tile"; destination = source + Direction::NorthEast + Direction::North; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 6) << "Wrong cost for travelling to a { 2, 1 } offset"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), PathAxisAlignedStepCost + PathDiagonalStepCost) << "Wrong cost for travelling to a { 2, 1 } offset"; destination = source + Direction::SouthEast + Direction::SouthEast; - EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 4) << "Wrong cost for travelling to a { 2, 0 } offset"; + EXPECT_EQ(TestPathGetHeuristicCost(source, destination), 2 * PathAxisAlignedStepCost) << "Wrong cost for travelling to a { 2, 0 } offset"; } // These symbols are in terms of coordinates (not in terms of on-screen direction). @@ -90,11 +91,12 @@ std::vector ToSyms(std::span indices) void CheckPath(Point startPosition, Point destinationPosition, std::vector expectedSteps) { + constexpr size_t MaxPathLength = 25; int8_t pathSteps[MaxPathLength]; auto pathLength = FindPath( /*canStep=*/[](Point, Point) { return true; }, /*posOk=*/[](Point) { return true; }, - startPosition, destinationPosition, pathSteps); + startPosition, destinationPosition, pathSteps, MaxPathLength); EXPECT_THAT(ToSyms(std::span(pathSteps, pathLength)), ElementsAreArray(ToSyms(expectedSteps))) << "Path steps differs from expectation for a path from " << startPosition << " to " << destinationPosition; diff --git a/test/player_test.cpp b/test/player_test.cpp index 80d538b11e2..9730a6a8b4c 100644 --- a/test/player_test.cpp +++ b/test/player_test.cpp @@ -136,7 +136,7 @@ static void AssertPlayer(devilution::Player &player) ASSERT_EQ(player.pDamAcFlags, ItemSpecialEffectHf::None); ASSERT_EQ(player._pmode, 0); - ASSERT_EQ(Count8(player.walkpath, MaxPathLength), 0); + ASSERT_EQ(Count8(player.walkpath, MaxPathLengthPlayer), 0); ASSERT_EQ(player.queuedSpell.spellId, SpellID::Null); ASSERT_EQ(player.queuedSpell.spellType, SpellType::Skill); ASSERT_EQ(player.queuedSpell.spellFrom, 0); diff --git a/test/writehero_test.cpp b/test/writehero_test.cpp index fb3922ec6b2..6d63066747c 100644 --- a/test/writehero_test.cpp +++ b/test/writehero_test.cpp @@ -303,7 +303,7 @@ void AssertPlayer(Player &player) ASSERT_EQ(player.pDamAcFlags, ItemSpecialEffectHf::None); ASSERT_EQ(player._pmode, 0); - ASSERT_EQ(Count8(player.walkpath, MaxPathLength), 25); + ASSERT_EQ(Count8(player.walkpath, MaxPathLengthPlayer), MaxPathLengthPlayer); ASSERT_EQ(player._pgfxnum, 36); ASSERT_EQ(player.AnimInfo.ticksPerFrame, 4); ASSERT_EQ(player.AnimInfo.tickCounterOfCurrentFrame, 1);