Moved logic for building a Sync'ed path from AiCombat to PathFinding.

Is now used by AiFollow, which should fix "running in circles" bug caused when recalc a path and closest way point is the one NPC has just passed.

apps/openmw/mwmechanics/aicombat.cpp

@@ -677,44 +677,32 @@ namespace MWMechanics
         return false;
     }
 
+    bool AiCombat::doesPathNeedRecalc(ESM::Pathgrid::Point dest, const ESM::Cell *cell)
+    {
+        if (!mPathFinder.getPath().empty())
+        {
+            Ogre::Vector3 currPathTarget(PathFinder::MakeOgreVector3(mPathFinder.getPath().back()));
+            Ogre::Vector3 newPathTarget = PathFinder::MakeOgreVector3(dest);
+            float dist = (newPathTarget - currPathTarget).length();
+            float targetPosThreshold = (cell->isExterior()) ? 300.0f : 100.0f;
+            return dist > targetPosThreshold;
+        }
+        else
+        {
+            // necessarily construct a new path
+            return true;
+        }
+    }
+
     void AiCombat::buildNewPath(const MWWorld::Ptr& actor, const MWWorld::Ptr& target)
     {
-        Ogre::Vector3 newPathTarget = Ogre::Vector3(target.getRefData().getPosition().pos);
+        ESM::Pathgrid::Point newPathTarget = PathFinder::MakePathgridPoint(target.getRefData().getPosition());
-
-        float dist;
-
-        if(!mPathFinder.getPath().empty())
-        {
-            ESM::Pathgrid::Point lastPt = mPathFinder.getPath().back();
-            Ogre::Vector3 currPathTarget(PathFinder::MakeOgreVector3(lastPt));
-            dist = (newPathTarget - currPathTarget).length();
-        }
-        else dist = 1e+38F; // necessarily construct a new path
-
-        float targetPosThreshold = (actor.getCell()->getCell()->isExterior())? 300.0f : 100.0f;
 
         //construct new path only if target has moved away more than on [targetPosThreshold]
-        if(dist > targetPosThreshold)
+        if (doesPathNeedRecalc(newPathTarget, actor.getCell()->getCell()))
         {
-            ESM::Position pos = actor.getRefData().getPosition();
+            ESM::Pathgrid::Point start(PathFinder::MakePathgridPoint(actor.getRefData().getPosition()));
-
+            mPathFinder.buildSyncedPath(start, newPathTarget, actor.getCell(), false);
-            ESM::Pathgrid::Point start(PathFinder::MakePathgridPoint(pos));
-
-            ESM::Pathgrid::Point dest(PathFinder::MakePathgridPoint(newPathTarget));
-
-            if(!mPathFinder.isPathConstructed())
-                mPathFinder.buildPath(start, dest, actor.getCell(), false);
-            else
-            {
-                PathFinder newPathFinder;
-                newPathFinder.buildPath(start, dest, actor.getCell(), false);
-
-                if(!mPathFinder.getPath().empty())
-                {
-                    newPathFinder.syncStart(mPathFinder.getPath());
-                    mPathFinder = newPathFinder;
-                }
-            }
         }
     }
 

apps/openmw/mwmechanics/aicombat.hpp

@@ -53,6 +53,9 @@ namespace MWMechanics
 
             virtual void writeState(ESM::AiSequence::AiSequence &sequence) const;
 
+        protected:
+            virtual bool doesPathNeedRecalc(ESM::Pathgrid::Point dest, const ESM::Cell *cell);
+
         private:
 
             int mTargetActorId;

apps/openmw/mwmechanics/aipackage.cpp

@@ -68,7 +68,7 @@ bool MWMechanics::AiPackage::pathTo(const MWWorld::Ptr& actor, ESM::Pathgrid::Po
     if(mTimer > 0.25)
     {
         if (doesPathNeedRecalc(dest, cell)) { //Only rebuild path if it's moved
-            mPathFinder.buildPath(start, dest, actor.getCell(), true); //Rebuild path, in case the target has moved
+            mPathFinder.buildSyncedPath(start, dest, actor.getCell(), true); //Rebuild path, in case the target has moved
             mPrevDest = dest;
         }
 

apps/openmw/mwmechanics/pathfinding.cpp

@@ -301,23 +301,35 @@ namespace MWMechanics
         return false;
     }
 
-    // used by AiCombat, see header for the rationale
+    // see header for the rationale
-    bool PathFinder::syncStart(const std::list<ESM::Pathgrid::Point> &path)
+    void PathFinder::buildSyncedPath(const ESM::Pathgrid::Point &startPoint,
+        const ESM::Pathgrid::Point &endPoint,
+        const MWWorld::CellStore* cell,
+        bool allowShortcuts)
     {
         if (mPath.size() < 2)
-            return false; //nothing to pop
-
-        std::list<ESM::Pathgrid::Point>::const_iterator oldStart = path.begin();
-        std::list<ESM::Pathgrid::Point>::iterator iter = ++mPath.begin();
-
-        if(    (*iter).mX == oldStart->mX
-            && (*iter).mY == oldStart->mY
-            && (*iter).mZ == oldStart->mZ)
         {
-            mPath.pop_front();
+            // if path has one point, then it's the destination.
-            return true;
+            // don't need to worry about bad path for this case
+            buildPath(startPoint, endPoint, cell, allowShortcuts);
+        }
+        else
+        {
+            const ESM::Pathgrid::Point oldStart(*getPath().begin());
+            buildPath(startPoint, endPoint, cell, allowShortcuts);
+            if (mPath.size() >= 2)
+            {
+                // if 2nd waypoint of new path == 1st waypoint of old, 
+                // delete 1st waypoint of new path.
+                std::list<ESM::Pathgrid::Point>::iterator iter = ++mPath.begin();
+                if (iter->mX == oldStart.mX
+                    && iter->mY == oldStart.mY
+                    && iter->mZ == oldStart.mZ)
+                {
+                    mPath.pop_front();
+                }
+            }
         }
-        return false;
     }
 
 }

apps/openmw/mwmechanics/pathfinding.hpp

@@ -63,13 +63,13 @@ namespace MWMechanics
 
             /** Synchronize new path with old one to avoid visiting 1 waypoint 2 times
             @note
-                If the first point is chosen as the nearest one
+                BuildPath() takes closest PathGrid point to NPC as first point of path.
-                the situation can occur when the 1st point of the new path is undesirable
+                This is undesireable if NPC has just passed a Pathgrid point, as this
-                (i.e. the 2nd point of new path == the 1st point of old path).
+                makes the 2nd point of the new path == the 1st point of old path.
-            @param path - old path
+                Which results in NPC "running in a circle" back to the just passed waypoint.
-            @return true if such point was found and deleted
              */
-            bool syncStart(const std::list<ESM::Pathgrid::Point> &path);
+            void buildSyncedPath(const ESM::Pathgrid::Point &startPoint, const ESM::Pathgrid::Point &endPoint,
+                const MWWorld::CellStore* cell, bool allowShortcuts = true);
 
             void addPointToPath(ESM::Pathgrid::Point &point)
             {