Merge branch 'pathgridgraph' into 'master'

Refactor pathgrid indices to use size_t instead of int

See merge request OpenMW/openmw!2900

apps/esmtool/record.cpp

@@ -1144,7 +1144,7 @@ namespace EsmTool
         std::cout << "  Cell: " << mData.mCell << std::endl;
         std::cout << "  Coordinates: (" << mData.mData.mX << "," << mData.mData.mY << ")" << std::endl;
         std::cout << "  Unknown S1: " << mData.mData.mS1 << std::endl;
-        if ((unsigned int)mData.mData.mS2 != mData.mPoints.size())
+        if (static_cast<size_t>(mData.mData.mS2) != mData.mPoints.size())
             std::cout << "  Reported Point Count: " << mData.mData.mS2 << std::endl;
         std::cout << "  Point Count: " << mData.mPoints.size() << std::endl;
         std::cout << "  Edge Count: " << mData.mEdges.size() << std::endl;
@@ -1164,7 +1164,7 @@ namespace EsmTool
         for (const ESM::Pathgrid::Edge& edge : mData.mEdges)
         {
             std::cout << "  Edge[" << i << "]: " << edge.mV0 << " -> " << edge.mV1 << std::endl;
-            if (edge.mV0 >= mData.mData.mS2 || edge.mV1 >= mData.mData.mS2)
+            if (edge.mV0 >= static_cast<size_t>(mData.mData.mS2) || edge.mV1 >= static_cast<size_t>(mData.mData.mS2))
                 std::cout << "  BAD POINT IN EDGE!" << std::endl;
             i++;
         }

apps/opencs/model/tools/pathgridcheck.cpp

@@ -50,48 +50,48 @@ void CSMTools::PathgridCheckStage::perform(int stage, CSMDoc::Messages& messages
         messages.add(id, "More points than expected", "", CSMDoc::Message::Severity_Error);
 
     std::vector<CSMTools::Point> pointList(pathgrid.mPoints.size());
-    std::vector<int> duplList;
+    std::vector<size_t> duplList;
 
-    for (unsigned int i = 0; i < pathgrid.mEdges.size(); ++i)
+    for (const auto& edge : pathgrid.mEdges)
     {
-        if (pathgrid.mEdges[i].mV0 < static_cast<int>(pathgrid.mPoints.size()) && pathgrid.mEdges[i].mV0 >= 0)
+        if (edge.mV0 < pathgrid.mPoints.size())
         {
-            pointList[pathgrid.mEdges[i].mV0].mConnectionNum++;
+            auto& point = pointList[edge.mV0];
+            point.mConnectionNum++;
             // first check for duplicate edges
-            unsigned int j = 0;
+            size_t j = 0;
-            for (; j < pointList[pathgrid.mEdges[i].mV0].mOtherIndex.size(); ++j)
+            for (; j < point.mOtherIndex.size(); ++j)
             {
-                if (pointList[pathgrid.mEdges[i].mV0].mOtherIndex[j] == pathgrid.mEdges[i].mV1)
+                if (point.mOtherIndex[j] == edge.mV1)
                 {
                     std::ostringstream ss;
-                    ss << "Duplicate edge between points #" << pathgrid.mEdges[i].mV0 << " and #"
+                    ss << "Duplicate edge between points #" << edge.mV0 << " and #" << edge.mV1;
-                       << pathgrid.mEdges[i].mV1;
+                    messages.add(id, ss.str(), {}, CSMDoc::Message::Severity_Error);
-                    messages.add(id, ss.str(), "", CSMDoc::Message::Severity_Error);
                     break;
                 }
             }
 
             // only add if not a duplicate
-            if (j == pointList[pathgrid.mEdges[i].mV0].mOtherIndex.size())
+            if (j == point.mOtherIndex.size())
-                pointList[pathgrid.mEdges[i].mV0].mOtherIndex.push_back(pathgrid.mEdges[i].mV1);
+                point.mOtherIndex.push_back(edge.mV1);
         }
         else
         {
             std::ostringstream ss;
-            ss << "An edge is connected to a non-existent point #" << pathgrid.mEdges[i].mV0;
+            ss << "An edge is connected to a non-existent point #" << edge.mV0;
-            messages.add(id, ss.str(), "", CSMDoc::Message::Severity_Error);
+            messages.add(id, ss.str(), {}, CSMDoc::Message::Severity_Error);
         }
     }
 
-    for (unsigned int i = 0; i < pathgrid.mPoints.size(); ++i)
+    for (size_t i = 0; i < pathgrid.mPoints.size(); ++i)
     {
         // check that edges are bidirectional
         bool foundReverse = false;
-        for (unsigned int j = 0; j < pointList[i].mOtherIndex.size(); ++j)
+        for (const auto& otherIndex : pointList[i].mOtherIndex)
         {
-            for (unsigned int k = 0; k < pointList[pointList[i].mOtherIndex[j]].mOtherIndex.size(); ++k)
+            for (const auto& other : pointList[otherIndex].mOtherIndex)
             {
-                if (pointList[pointList[i].mOtherIndex[j]].mOtherIndex[k] == static_cast<int>(i))
+                if (other == i)
                 {
                     foundReverse = true;
                     break;
@@ -101,25 +101,25 @@ void CSMTools::PathgridCheckStage::perform(int stage, CSMDoc::Messages& messages
             if (!foundReverse)
             {
                 std::ostringstream ss;
-                ss << "Missing edge between points #" << i << " and #" << pointList[i].mOtherIndex[j];
+                ss << "Missing edge between points #" << i << " and #" << otherIndex;
-                messages.add(id, ss.str(), "", CSMDoc::Message::Severity_Error);
+                messages.add(id, ss.str(), {}, CSMDoc::Message::Severity_Error);
             }
         }
 
         // check duplicate points
         // FIXME: how to do this efficiently?
-        for (unsigned int j = 0; j != i; ++j)
+        for (size_t j = 0; j != i; ++j)
         {
             if (pathgrid.mPoints[i].mX == pathgrid.mPoints[j].mX && pathgrid.mPoints[i].mY == pathgrid.mPoints[j].mY
                 && pathgrid.mPoints[i].mZ == pathgrid.mPoints[j].mZ)
             {
-                std::vector<int>::const_iterator it = find(duplList.begin(), duplList.end(), static_cast<int>(i));
+                auto it = std::find(duplList.begin(), duplList.end(), i);
                 if (it == duplList.end())
                 {
                     std::ostringstream ss;
                     ss << "Point #" << i << " duplicates point #" << j << " (" << pathgrid.mPoints[i].mX << ", "
                        << pathgrid.mPoints[i].mY << ", " << pathgrid.mPoints[i].mZ << ")";
-                    messages.add(id, ss.str(), "", CSMDoc::Message::Severity_Warning);
+                    messages.add(id, ss.str(), {}, CSMDoc::Message::Severity_Warning);
 
                     duplList.push_back(i);
                     break;
@@ -129,14 +129,14 @@ void CSMTools::PathgridCheckStage::perform(int stage, CSMDoc::Messages& messages
     }
 
     // check pathgrid points that are not connected to anything
-    for (unsigned int i = 0; i < pointList.size(); ++i)
+    for (size_t i = 0; i < pointList.size(); ++i)
     {
         if (pointList[i].mConnectionNum == 0)
         {
             std::ostringstream ss;
             ss << "Point #" << i << " (" << pathgrid.mPoints[i].mX << ", " << pathgrid.mPoints[i].mY << ", "
                << pathgrid.mPoints[i].mZ << ") is disconnected from other points";
-            messages.add(id, ss.str(), "", CSMDoc::Message::Severity_Warning);
+            messages.add(id, ss.str(), {}, CSMDoc::Message::Severity_Warning);
         }
     }
 

apps/opencs/model/tools/pathgridcheck.hpp

@@ -23,7 +23,7 @@ namespace CSMTools
     struct Point
     {
         unsigned char mConnectionNum;
-        std::vector<int> mOtherIndex;
+        std::vector<size_t> mOtherIndex;
         Point()
             : mConnectionNum(0)
             , mOtherIndex(0)

apps/opencs/model/world/nestedcoladapterimp.cpp

@@ -200,9 +200,9 @@ namespace CSMWorld
             case 0:
                 return subRowIndex;
             case 1:
-                return edge.mV0;
+                return static_cast<uint>(edge.mV0);
             case 2:
-                return edge.mV1;
+                return static_cast<uint>(edge.mV1);
             default:
                 throw std::runtime_error("Pathgrid edge subcolumn index out of range");
         }

apps/opencs/view/render/pathgrid.cpp

@@ -424,9 +424,9 @@ namespace CSVRender
                 int adjustment1 = 0;
 
                 // Determine necessary adjustment
-                for (std::vector<unsigned short>::iterator point = mSelected.begin(); point != mSelected.end(); ++point)
+                for (const auto point : mSelected)
                 {
-                    if (source->mEdges[edge].mV0 == *point || source->mEdges[edge].mV1 == *point)
+                    if (source->mEdges[edge].mV0 == point || source->mEdges[edge].mV1 == point)
                     {
                         edgeRowsToRemove.insert(static_cast<int>(edge));
 
@@ -435,10 +435,10 @@ namespace CSVRender
                         break;
                     }
 
-                    if (source->mEdges[edge].mV0 > *point)
+                    if (source->mEdges[edge].mV0 > point)
                         --adjustment0;
 
-                    if (source->mEdges[edge].mV1 > *point)
+                    if (source->mEdges[edge].mV1 > point)
                         --adjustment1;
                 }
 
@@ -457,10 +457,9 @@ namespace CSVRender
                 }
             }
 
-            std::set<int, std::greater<int>>::iterator row;
+            for (const auto row : edgeRowsToRemove)
-            for (row = edgeRowsToRemove.begin(); row != edgeRowsToRemove.end(); ++row)
             {
-                commands.push(new CSMWorld::DeleteNestedCommand(*model, mId.getRefIdString(), *row, parentColumn));
+                commands.push(new CSMWorld::DeleteNestedCommand(*model, mId.getRefIdString(), row, parentColumn));
             }
         }
 

apps/openmw/mwmechanics/aipackage.cpp

@@ -13,6 +13,7 @@
 #include "../mwworld/action.hpp"
 #include "../mwworld/cellstore.hpp"
 #include "../mwworld/class.hpp"
+#include "../mwworld/esmstore.hpp"
 #include "../mwworld/inventorystore.hpp"
 
 #include "../mwphysics/raycasting.hpp"
@@ -328,7 +329,7 @@ void MWMechanics::AiPackage::openDoors(const MWWorld::Ptr& actor)
     }
 }
 
-const MWMechanics::PathgridGraph& MWMechanics::AiPackage::getPathGridGraph(const MWWorld::CellStore* cell)
+const MWMechanics::PathgridGraph& MWMechanics::AiPackage::getPathGridGraph(const MWWorld::CellStore* cell) const
 {
     const ESM::RefId id = cell->getCell()->getId();
     // static cache is OK for now, pathgrids can never change during runtime
@@ -337,10 +338,17 @@ const MWMechanics::PathgridGraph& MWMechanics::AiPackage::getPathGridGraph(const
     CacheMap::iterator found = cache.find(id);
     if (found == cache.end())
     {
-        cache.insert(
+        const ESM::Pathgrid* pathgrid
-            std::make_pair(id, std::make_unique<MWMechanics::PathgridGraph>(MWMechanics::PathgridGraph(cell))));
+            = MWBase::Environment::get().getWorld()->getStore().get<ESM::Pathgrid>().search(*cell->getCell());
+        std::unique_ptr<MWMechanics::PathgridGraph> ptr;
+        if (pathgrid)
+            ptr = std::make_unique<MWMechanics::PathgridGraph>(MWMechanics::PathgridGraph(*pathgrid));
+        found = cache.emplace(id, std::move(ptr)).first;
     }
-    return *cache[id].get();
+    const MWMechanics::PathgridGraph* graph = found->second.get();
+    if (!graph)
+        return MWMechanics::PathgridGraph::sEmpty;
+    return *graph;
 }
 
 bool MWMechanics::AiPackage::shortcutPath(const osg::Vec3f& startPoint, const osg::Vec3f& endPoint,

apps/openmw/mwmechanics/aipackage.hpp

@@ -145,7 +145,7 @@ namespace MWMechanics
 
         void openDoors(const MWWorld::Ptr& actor);
 
-        const PathgridGraph& getPathGridGraph(const MWWorld::CellStore* cell);
+        const PathgridGraph& getPathGridGraph(const MWWorld::CellStore* cell) const;
 
         DetourNavigator::Flags getNavigatorFlags(const MWWorld::Ptr& actor) const;
 

apps/openmw/mwmechanics/aiwander.cpp

@@ -273,7 +273,7 @@ namespace MWMechanics
         // Initialization to discover & store allowed node points for this actor.
         if (storage.mPopulateAvailableNodes)
         {
-            getAllowedNodes(actor, actor.getCell()->getCell(), storage);
+            getAllowedNodes(actor, storage);
         }
 
         auto& prng = MWBase::Environment::get().getWorld()->getPrng();
@@ -722,7 +722,7 @@ namespace MWMechanics
 
         AiWanderStorage& storage = state.get<AiWanderStorage>();
         if (storage.mPopulateAvailableNodes)
-            getAllowedNodes(actor, actor.getCell()->getCell(), storage);
+            getAllowedNodes(actor, storage);
 
         if (storage.mAllowedNodes.empty())
             return;
@@ -811,12 +811,12 @@ namespace MWMechanics
         getPathGridGraph(currentCell).getNeighbouringPoints(index, points);
     }
 
-    void AiWander::getAllowedNodes(const MWWorld::Ptr& actor, const MWWorld::Cell* cell, AiWanderStorage& storage)
+    void AiWander::getAllowedNodes(const MWWorld::Ptr& actor, AiWanderStorage& storage)
     {
         // infrequently used, therefore no benefit in caching it as a member
-        const ESM::Pathgrid* pathgrid
-            = MWBase::Environment::get().getWorld()->getStore().get<ESM::Pathgrid>().search(*cell);
         const MWWorld::CellStore* cellStore = actor.getCell();
+        const ESM::Pathgrid* pathgrid
+            = MWBase::Environment::get().getWorld()->getStore().get<ESM::Pathgrid>().search(*cellStore->getCell());
 
         storage.mAllowedNodes.clear();
 
@@ -835,7 +835,7 @@ namespace MWMechanics
         if (mDistance && storage.mCanWanderAlongPathGrid && !actor.getClass().isPureWaterCreature(actor))
         {
             // get NPC's position in local (i.e. cell) coordinates
-            auto converter = Misc::CoordinateConverter(*cell);
+            auto converter = Misc::CoordinateConverter(*cellStore->getCell());
             const osg::Vec3f npcPos = converter.toLocalVec3(mInitialActorPosition);
 
             // Find closest pathgrid point
@@ -844,8 +844,8 @@ namespace MWMechanics
             // mAllowedNodes for this actor with pathgrid point indexes based on mDistance
             // and if the point is connected to the closest current point
             // NOTE: mPoints is in local coordinates
-            int pointIndex = 0;
+            size_t pointIndex = 0;
-            for (unsigned int counter = 0; counter < pathgrid->mPoints.size(); counter++)
+            for (size_t counter = 0; counter < pathgrid->mPoints.size(); counter++)
             {
                 osg::Vec3f nodePos(PathFinder::makeOsgVec3(pathgrid->mPoints[counter]));
                 if ((npcPos - nodePos).length2() <= mDistance * mDistance
@@ -873,10 +873,10 @@ namespace MWMechanics
     // additional points for NPC to wander to are:
     // 1. NPC's initial location
     // 2. Partway along the path between the point and its connected points.
-    void AiWander::addNonPathGridAllowedPoints(const ESM::Pathgrid* pathGrid, int pointIndex, AiWanderStorage& storage,
+    void AiWander::addNonPathGridAllowedPoints(const ESM::Pathgrid* pathGrid, size_t pointIndex,
-        const Misc::CoordinateConverter& converter)
+        AiWanderStorage& storage, const Misc::CoordinateConverter& converter)
     {
-        for (auto& edge : pathGrid->mEdges)
+        for (const auto& edge : pathGrid->mEdges)
         {
             if (edge.mV0 == pointIndex)
             {

apps/openmw/mwmechanics/aiwander.hpp

@@ -151,7 +151,7 @@ namespace MWMechanics
         void getNeighbouringNodes(
             ESM::Pathgrid::Point dest, const MWWorld::CellStore* currentCell, ESM::Pathgrid::PointList& points);
 
-        void getAllowedNodes(const MWWorld::Ptr& actor, const MWWorld::Cell* cell, AiWanderStorage& storage);
+        void getAllowedNodes(const MWWorld::Ptr& actor, AiWanderStorage& storage);
 
         void trimAllowedNodes(std::vector<ESM::Pathgrid::Point>& nodes, const PathFinder& pathfinder);
 
@@ -164,7 +164,7 @@ namespace MWMechanics
 
         void setCurrentNodeToClosestAllowedNode(AiWanderStorage& storage);
 
-        void addNonPathGridAllowedPoints(const ESM::Pathgrid* pathGrid, int pointIndex, AiWanderStorage& storage,
+        void addNonPathGridAllowedPoints(const ESM::Pathgrid* pathGrid, size_t pointIndex, AiWanderStorage& storage,
             const Misc::CoordinateConverter& converter);
 
         void AddPointBetweenPathGridPoints(

apps/openmw/mwmechanics/pathgrid.cpp

@@ -1,10 +1,7 @@
 #include "pathgrid.hpp"
 
-#include "../mwbase/environment.hpp"
+#include <list>
-#include "../mwbase/world.hpp"
+#include <set>
-
-#include "../mwworld/cellstore.hpp"
-#include "../mwworld/esmstore.hpp"
 
 namespace
 {
@@ -45,20 +42,97 @@ namespace
         // return distance(a, b);
         return manhattan(a, b);
     }
+
+    constexpr size_t NoIndex = static_cast<size_t>(-1);
 }
 
 namespace MWMechanics
 {
-    PathgridGraph::PathgridGraph(const MWWorld::CellStore* cell)
+
-        : mCell(nullptr)
+    class PathgridGraph::Builder
-        , mPathgrid(nullptr)
-        , mGraph(0)
-        , mIsGraphConstructed(false)
-        , mSCCId(0)
-        , mSCCIndex(0)
     {
-        load(cell);
+        std::vector<Node>& mGraph;
+
+        // variables used to calculate connected components
+        int mSCCId = 0;
+        size_t mSCCIndex = 0;
+        std::vector<size_t> mSCCStack;
+        std::vector<std::pair<size_t, size_t>> mSCCPoint; // first is index, second is lowlink
+
+        // v is the pathgrid point index (some call them vertices)
+        void recursiveStrongConnect(const size_t v)
+        {
+            mSCCPoint[v].first = mSCCIndex; // index
+            mSCCPoint[v].second = mSCCIndex; // lowlink
+            mSCCIndex++;
+            mSCCStack.push_back(v);
+            size_t w;
+
+            for (const auto& edge : mGraph[v].edges)
+            {
+                w = edge.index;
+                if (mSCCPoint[w].first == NoIndex) // not visited
+                {
+                    recursiveStrongConnect(w); // recurse
+                    mSCCPoint[v].second = std::min(mSCCPoint[v].second, mSCCPoint[w].second);
                 }
+                else if (std::find(mSCCStack.begin(), mSCCStack.end(), w) != mSCCStack.end())
+                    mSCCPoint[v].second = std::min(mSCCPoint[v].second, mSCCPoint[w].first);
+            }
+
+            if (mSCCPoint[v].second == mSCCPoint[v].first)
+            { // new component
+                do
+                {
+                    w = mSCCStack.back();
+                    mSCCStack.pop_back();
+                    mGraph[w].componentId = mSCCId;
+                } while (w != v);
+                mSCCId++;
+            }
+        }
+
+    public:
+        /*
+         * mGraph contains the strongly connected component group id's along
+         * with pre-calculated edge costs.
+         *
+         * A cell can have disjointed pathgrids, e.g. Seyda Neen has 3
+         *
+         * mGraph for Seyda Neen will therefore have 3 different values.  When
+         * selecting a random pathgrid point for AiWander, mGraph can be checked
+         * for quickly finding whether the destination is reachable.
+         *
+         * Otherwise, buildPath can automatically select a closest reachable end
+         * pathgrid point (reachable from the closest start point).
+         *
+         * Using Tarjan's algorithm:
+         *
+         *  mGraph                   | graph G   |
+         *  mSCCPoint                | V         | derived from mPoints
+         *  mGraph[v].edges          | E (for v) |
+         *  mSCCIndex                | index     | tracking smallest unused index
+         *  mSCCStack                | S         |
+         *  mGraph[v].edges[i].index | w         |
+         *
+         */
+        explicit Builder(PathgridGraph& graph)
+            : mGraph(graph.mGraph)
+        {
+            // both of these are set to zero in the constructor
+            // mSCCId = 0; // how many strongly connected components in this cell
+            // mSCCIndex = 0;
+            size_t pointsSize = graph.mPathgrid->mPoints.size();
+            mSCCPoint.resize(pointsSize, std::pair<size_t, size_t>(NoIndex, NoIndex));
+            mSCCStack.reserve(pointsSize);
+
+            for (size_t v = 0; v < pointsSize; ++v)
+            {
+                if (mSCCPoint[v].first == NoIndex) // undefined (haven't visited)
+                    recursiveStrongConnect(v);
+            }
+        }
+    };
 
     /*
      * mGraph is populated with the cost of each allowed edge.
@@ -96,132 +170,38 @@ namespace MWMechanics
      *    +---------------->
      *      high cost
      */
-    bool PathgridGraph::load(const MWWorld::CellStore* cell)
+    PathgridGraph::PathgridGraph(const ESM::Pathgrid& pathgrid)
+        : mPathgrid(&pathgrid)
     {
-        if (!cell)
-            return false;
-
-        if (mIsGraphConstructed)
-            return true;
-
-        mCell = cell->getCell();
-
-        mPathgrid = MWBase::Environment::get().getWorld()->getStore().get<ESM::Pathgrid>().search(*mCell);
-        if (!mPathgrid)
-            return false;
-
         mGraph.resize(mPathgrid->mPoints.size());
-        for (int i = 0; i < static_cast<int>(mPathgrid->mEdges.size()); i++)
+        for (const auto& edge : mPathgrid->mEdges)
         {
             ConnectedPoint neighbour;
-            neighbour.cost
+            neighbour.cost = costAStar(mPathgrid->mPoints[edge.mV0], mPathgrid->mPoints[edge.mV1]);
-                = costAStar(mPathgrid->mPoints[mPathgrid->mEdges[i].mV0], mPathgrid->mPoints[mPathgrid->mEdges[i].mV1]);
             // forward path of the edge
-            neighbour.index = mPathgrid->mEdges[i].mV1;
+            neighbour.index = edge.mV1;
-            mGraph[mPathgrid->mEdges[i].mV0].edges.push_back(neighbour);
+            mGraph[edge.mV0].edges.push_back(neighbour);
             // reverse path of the edge
             // NOTE: These are redundant, ESM already contains the required reverse paths
-            // neighbour.index = mPathgrid->mEdges[i].mV0;
+            // neighbour.index = edge.mV0;
-            // mGraph[mPathgrid->mEdges[i].mV1].edges.push_back(neighbour);
+            // mGraph[edge.mV1].edges.push_back(neighbour);
         }
-        buildConnectedPoints();
+        Builder(*this);
-        mIsGraphConstructed = true;
-        return true;
     }
 
-    const ESM::Pathgrid* PathgridGraph::getPathgrid() const
+    const PathgridGraph PathgridGraph::sEmpty = {};
-    {
-        return mPathgrid;
-    }
 
-    // v is the pathgrid point index (some call them vertices)
+    bool PathgridGraph::isPointConnected(const size_t start, const size_t end) const
-    void PathgridGraph::recursiveStrongConnect(int v)
-    {
-        mSCCPoint[v].first = mSCCIndex; // index
-        mSCCPoint[v].second = mSCCIndex; // lowlink
-        mSCCIndex++;
-        mSCCStack.push_back(v);
-        int w;
-
-        for (int i = 0; i < static_cast<int>(mGraph[v].edges.size()); i++)
-        {
-            w = mGraph[v].edges[i].index;
-            if (mSCCPoint[w].first == -1) // not visited
-            {
-                recursiveStrongConnect(w); // recurse
-                mSCCPoint[v].second = std::min(mSCCPoint[v].second, mSCCPoint[w].second);
-            }
-            else
-            {
-                if (find(mSCCStack.begin(), mSCCStack.end(), w) != mSCCStack.end())
-                    mSCCPoint[v].second = std::min(mSCCPoint[v].second, mSCCPoint[w].first);
-            }
-        }
-
-        if (mSCCPoint[v].second == mSCCPoint[v].first)
-        { // new component
-            do
-            {
-                w = mSCCStack.back();
-                mSCCStack.pop_back();
-                mGraph[w].componentId = mSCCId;
-            } while (w != v);
-            mSCCId++;
-        }
-        return;
-    }
-
-    /*
-     * mGraph contains the strongly connected component group id's along
-     * with pre-calculated edge costs.
-     *
-     * A cell can have disjointed pathgrids, e.g. Seyda Neen has 3
-     *
-     * mGraph for Seyda Neen will therefore have 3 different values.  When
-     * selecting a random pathgrid point for AiWander, mGraph can be checked
-     * for quickly finding whether the destination is reachable.
-     *
-     * Otherwise, buildPath can automatically select a closest reachable end
-     * pathgrid point (reachable from the closest start point).
-     *
-     * Using Tarjan's algorithm:
-     *
-     *  mGraph                   | graph G   |
-     *  mSCCPoint                | V         | derived from mPoints
-     *  mGraph[v].edges          | E (for v) |
-     *  mSCCIndex                | index     | tracking smallest unused index
-     *  mSCCStack                | S         |
-     *  mGraph[v].edges[i].index | w         |
-     *
-     */
-    void PathgridGraph::buildConnectedPoints()
-    {
-        // both of these are set to zero in the constructor
-        // mSCCId = 0; // how many strongly connected components in this cell
-        // mSCCIndex = 0;
-        int pointsSize = static_cast<int>(mPathgrid->mPoints.size());
-        mSCCPoint.resize(pointsSize, std::pair<int, int>(-1, -1));
-        mSCCStack.reserve(pointsSize);
-
-        for (int v = 0; v < pointsSize; v++)
-        {
-            if (mSCCPoint[v].first == -1) // undefined (haven't visited)
-                recursiveStrongConnect(v);
-        }
-    }
-
-    bool PathgridGraph::isPointConnected(const int start, const int end) const
     {
         return (mGraph[start].componentId == mGraph[end].componentId);
     }
 
-    void PathgridGraph::getNeighbouringPoints(const int index, ESM::Pathgrid::PointList& nodes) const
+    void PathgridGraph::getNeighbouringPoints(const size_t index, ESM::Pathgrid::PointList& nodes) const
     {
-        for (int i = 0; i < static_cast<int>(mGraph[index].edges.size()); i++)
+        for (const auto& edge : mGraph[index].edges)
         {
-            int neighbourIndex = mGraph[index].edges[i].index;
+            if (edge.index != index)
-            if (neighbourIndex != index)
+                nodes.push_back(mPathgrid->mPoints[edge.index]);
-                nodes.push_back(mPathgrid->mPoints[neighbourIndex]);
         }
     }
 
@@ -252,7 +232,7 @@ namespace MWMechanics
      *       pathgrid points form (currently they are converted to world
      *       coordinates).  Essentially trading speed w/ memory.
      */
-    std::deque<ESM::Pathgrid::Point> PathgridGraph::aStarSearch(const int start, const int goal) const
+    std::deque<ESM::Pathgrid::Point> PathgridGraph::aStarSearch(const size_t start, const size_t goal) const
     {
         std::deque<ESM::Pathgrid::Point> path;
         if (!isPointConnected(start, goal))
@@ -260,20 +240,20 @@ namespace MWMechanics
             return path; // there is no path, return an empty path
         }
 
-        int graphSize = static_cast<int>(mGraph.size());
+        size_t graphSize = mGraph.size();
         std::vector<float> gScore(graphSize, -1);
         std::vector<float> fScore(graphSize, -1);
-        std::vector<int> graphParent(graphSize, -1);
+        std::vector<size_t> graphParent(graphSize, NoIndex);
 
         // gScore & fScore keep costs for each pathgrid point in mPoints
         gScore[start] = 0;
         fScore[start] = costAStar(mPathgrid->mPoints[start], mPathgrid->mPoints[goal]);
 
-        std::list<int> openset;
+        std::list<size_t> openset;
-        std::list<int> closedset;
+        std::set<size_t> closedset;
         openset.push_back(start);
 
-        int current = -1;
+        size_t current = start;
 
         while (!openset.empty())
         {
@@ -283,16 +263,16 @@ namespace MWMechanics
             if (current == goal)
                 break;
 
-            closedset.push_back(current); // remember we've been here
+            closedset.insert(current); // remember we've been here
 
             // check all edges for the current point index
-            for (int j = 0; j < static_cast<int>(mGraph[current].edges.size()); j++)
+            for (const auto& edge : mGraph[current].edges)
             {
-                if (std::find(closedset.begin(), closedset.end(), mGraph[current].edges[j].index) == closedset.end())
+                if (!closedset.contains(edge.index))
                 {
                     // not in closedset - i.e. have not traversed this edge destination
-                    int dest = mGraph[current].edges[j].index;
+                    size_t dest = edge.index;
-                    float tentative_g = gScore[current] + mGraph[current].edges[j].cost;
+                    float tentative_g = gScore[current] + edge.cost;
                     bool isInOpenSet = std::find(openset.begin(), openset.end(), dest) != openset.end();
                     if (!isInOpenSet || tentative_g < gScore[dest])
                     {
@@ -303,8 +283,8 @@ namespace MWMechanics
                         {
                             // add this edge to openset, lowest cost goes to the front
                             // TODO: if this causes performance problems a hash table may help
-                            std::list<int>::iterator it = openset.begin();
+                            auto it = openset.begin();
-                            for (it = openset.begin(); it != openset.end(); ++it)
+                            for (; it != openset.end(); ++it)
                             {
                                 if (fScore[*it] > fScore[dest])
                                     break;
@@ -320,7 +300,7 @@ namespace MWMechanics
             return path; // for some reason couldn't build a path
 
         // reconstruct path to return, using local coordinates
-        while (graphParent[current] != -1)
+        while (graphParent[current] != NoIndex)
         {
             path.push_front(mPathgrid->mPoints[current]);
             current = graphParent[current];

apps/openmw/mwmechanics/pathgrid.hpp

@@ -5,49 +5,44 @@
 
 #include <components/esm3/loadpgrd.hpp>
 
-namespace ESM
-{
-    struct Cell;
-}
-
-namespace MWWorld
-{
-    class CellStore;
-    class Cell;
-}
-
 namespace MWMechanics
 {
     class PathgridGraph
     {
+        PathgridGraph()
+            : mPathgrid(nullptr)
+        {
+        }
+
     public:
-        PathgridGraph(const MWWorld::CellStore* cell);
+        explicit PathgridGraph(const ESM::Pathgrid& pathGrid);
 
-        bool load(const MWWorld::CellStore* cell);
+        const ESM::Pathgrid* getPathgrid() const { return mPathgrid; }
-
-        const ESM::Pathgrid* getPathgrid() const;
 
         // returns true if end point is strongly connected (i.e. reachable
         // from start point) both start and end are pathgrid point indexes
-        bool isPointConnected(const int start, const int end) const;
+        bool isPointConnected(const size_t start, const size_t end) const;
 
         // get neighbouring nodes for index node and put them to "nodes" vector
-        void getNeighbouringPoints(const int index, ESM::Pathgrid::PointList& nodes) const;
+        void getNeighbouringPoints(const size_t index, ESM::Pathgrid::PointList& nodes) const;
 
         // the input parameters are pathgrid point indexes
         // the output list is in local (internal cells) or world (external
         // cells) coordinates
         //
         // NOTE: if start equals end an empty path is returned
-        std::deque<ESM::Pathgrid::Point> aStarSearch(const int start, const int end) const;
+        std::deque<ESM::Pathgrid::Point> aStarSearch(const size_t start, const size_t end) const;
+
+        static const PathgridGraph sEmpty;
 
     private:
-        const MWWorld::Cell* mCell;
         const ESM::Pathgrid* mPathgrid;
 
+        class Builder;
+
         struct ConnectedPoint // edge
         {
-            int index; // pathgrid point index of neighbour
+            size_t index; // pathgrid point index of neighbour
             float cost;
         };
 
@@ -67,17 +62,6 @@ namespace MWMechanics
         //   all other pathgrid points are the third set
         //
         std::vector<Node> mGraph;
-        bool mIsGraphConstructed;
-
-        // variables used to calculate connected components
-        int mSCCId;
-        int mSCCIndex;
-        std::vector<int> mSCCStack;
-        typedef std::pair<int, int> VPair; // first is index, second is lowlink
-        std::vector<VPair> mSCCPoint;
-        // methods used to calculate connected components
-        void recursiveStrongConnect(int v);
-        void buildConnectedPoints();
     };
 }
 

components/esm3/loadpgrd.cpp

@@ -87,21 +87,22 @@ namespace ESM
                     else
                     {
                         int rawConnNum = size / sizeof(int);
-                        std::vector<int> rawConnections;
+                        std::vector<size_t> rawConnections;
                         rawConnections.reserve(rawConnNum);
                         for (int i = 0; i < rawConnNum; ++i)
                         {
                             int currentValue;
                             esm.getT(currentValue);
-                            rawConnections.push_back(currentValue);
+                            assert(currentValue >= 0);
+                            rawConnections.push_back(static_cast<size_t>(currentValue));
                         }
 
-                        std::vector<int>::const_iterator rawIt = rawConnections.begin();
+                        auto rawIt = rawConnections.begin();
-                        int pointIndex = 0;
+                        size_t pointIndex = 0;
                         mEdges.reserve(edgeCount);
-                        for (PointList::const_iterator it = mPoints.begin(); it != mPoints.end(); ++it, ++pointIndex)
+                        for (const auto& point : mPoints)
                         {
-                            unsigned char connectionNum = (*it).mConnectionNum;
+                            unsigned char connectionNum = point.mConnectionNum;
                             if (rawConnections.end() - rawIt < connectionNum)
                                 esm.fail("Not enough connections");
                             for (int i = 0; i < connectionNum; ++i)
@@ -112,6 +113,7 @@ namespace ESM
                                 ++rawIt;
                                 mEdges.push_back(edge);
                             }
+                            ++pointIndex;
                         }
                     }
                     break;
@@ -143,11 +145,11 @@ namespace ESM
         {
             correctedPoints[point].mConnectionNum = 0;
 
-            for (EdgeList::const_iterator it = mEdges.begin(); it != mEdges.end(); ++it)
+            for (const auto& edge : mEdges)
             {
-                if (static_cast<size_t>(it->mV0) == point)
+                if (edge.mV0 == point)
                 {
-                    sortedEdges.push_back(it->mV1);
+                    sortedEdges.push_back(static_cast<int>(edge.mV1));
                     ++correctedPoints[point].mConnectionNum;
                 }
             }

components/esm3/loadpgrd.hpp

@@ -53,8 +53,8 @@ namespace ESM
 
         struct Edge // path grid edge
         {
-            int mV0, mV1; // index of points connected with this edge
+            size_t mV0, mV1; // index of points connected with this edge
-        }; // 8 bytes
+        };
 
         ESM::RefId mCell; // Cell name
         DATAstruct mData;

components/sceneutil/pathgridutil.cpp

@@ -96,8 +96,7 @@ namespace SceneUtil
             for (ESM::Pathgrid::EdgeList::const_iterator edge = pathgrid.mEdges.begin(); edge != pathgrid.mEdges.end();
                  ++edge)
             {
-                if (edge->mV0 == edge->mV1 || edge->mV0 < 0 || edge->mV0 >= PointCount || edge->mV1 < 0
+                if (edge->mV0 == edge->mV1 || edge->mV0 >= PointCount || edge->mV1 >= PointCount)
-                    || edge->mV1 >= PointCount)
                     continue;
 
                 const ESM::Pathgrid::Point& from = pathgrid.mPoints[edge->mV0];