Merge remote-tracking branch 'dteviot/FixRunningInCircles'

apps/openmw/mwmechanics/aicombat.cpp

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

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

@@ -30,9 +30,9 @@ bool MWMechanics::AiPackage::pathTo(const MWWorld::Ptr& actor, ESM::Pathgrid::Po
     ESM::Position pos = actor.getRefData().getPosition(); //position of the actor
 
     /// Stops the actor when it gets too close to a unloaded cell
+    const ESM::Cell *cell = actor.getCell()->getCell();
     {
         MWWorld::Ptr player = MWBase::Environment::get().getWorld()->getPlayerPtr();
-        const ESM::Cell *cell = actor.getCell()->getCell();
         Movement &movement = actor.getClass().getMovementSettings(actor);
 
         //Ensure pursuer doesn't leave loaded cells
@@ -67,8 +67,8 @@ bool MWMechanics::AiPackage::pathTo(const MWWorld::Ptr& actor, ESM::Pathgrid::Po
     //***********************
     if(mTimer > 0.25)
     {
-        if(distance(mPrevDest, dest) > 10) { //Only rebuild path if it's moved
+        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;
         }
 
@@ -123,3 +123,8 @@ bool MWMechanics::AiPackage::pathTo(const MWWorld::Ptr& actor, ESM::Pathgrid::Po
 
     return false;
 }
+
+bool MWMechanics::AiPackage::doesPathNeedRecalc(ESM::Pathgrid::Point dest, const ESM::Cell *cell)
+{
+    return distance(mPrevDest, dest) > 10;
+}

apps/openmw/mwmechanics/aipackage.hpp

@@ -4,6 +4,8 @@
 #include "pathfinding.hpp"
 #include <components/esm/defs.hpp>
 
+#include "../mwworld/cellstore.hpp"
+
 #include "obstacle.hpp"
 #include "aistate.hpp"
 
@@ -71,6 +73,8 @@ namespace MWMechanics
             /** \return If the actor has arrived at his destination **/
             bool pathTo(const MWWorld::Ptr& actor, ESM::Pathgrid::Point dest, float duration);
 
+            virtual bool doesPathNeedRecalc(ESM::Pathgrid::Point dest, const ESM::Cell *cell);
+
             // TODO: all this does not belong here, move into temporary storage
             PathFinder mPathFinder;
             ObstacleCheck mObstacleCheck;

apps/openmw/mwmechanics/aitravel.cpp

@@ -51,64 +51,31 @@ namespace MWMechanics
 
     bool AiTravel::execute (const MWWorld::Ptr& actor, AiState& state, float duration)
     {
-        MWBase::World *world = MWBase::Environment::get().getWorld();
         ESM::Position pos = actor.getRefData().getPosition();
-        Movement &movement = actor.getClass().getMovementSettings(actor);
-        const ESM::Cell *cell = actor.getCell()->getCell();
 
         actor.getClass().getCreatureStats(actor).setMovementFlag(CreatureStats::Flag_Run, false);
-
         actor.getClass().getCreatureStats(actor).setDrawState(DrawState_Nothing);
 
-        MWWorld::Ptr player = world->getPlayerPtr();
-        if(cell->mData.mX != player.getCell()->getCell()->mData.mX)
-        {
-            int sideX = PathFinder::sgn(cell->mData.mX - player.getCell()->getCell()->mData.mX);
-            //check if actor is near the border of an inactive cell. If so, stop walking.
-            if(sideX * (pos.pos[0] - cell->mData.mX*ESM::Land::REAL_SIZE) >
-               sideX * (ESM::Land::REAL_SIZE/2.0f - 200.0f))
-            {
-                movement.mPosition[1] = 0;
-                return false;
-            }
-        }
-        if(cell->mData.mY != player.getCell()->getCell()->mData.mY)
-        {
-            int sideY = PathFinder::sgn(cell->mData.mY - player.getCell()->getCell()->mData.mY);
-            //check if actor is near the border of an inactive cell. If so, stop walking.
-            if(sideY * (pos.pos[1] - cell->mData.mY*ESM::Land::REAL_SIZE) >
-               sideY * (ESM::Land::REAL_SIZE/2.0f - 200.0f))
-            {
-                movement.mPosition[1] = 0;
-                return false;
-            }
-        }
-
         if (!isWithinMaxRange(Ogre::Vector3(mX, mY, mZ), Ogre::Vector3(pos.pos)))
             return false;
 
+        if (pathTo(actor, ESM::Pathgrid::Point(static_cast<int>(mX), static_cast<int>(mY), static_cast<int>(mZ)), duration))
+        {
+            actor.getClass().getMovementSettings(actor).mPosition[1] = 0;
+            return true;
+        }
+        return false;
+    }
+
+    bool AiTravel::doesPathNeedRecalc(ESM::Pathgrid::Point dest, const ESM::Cell *cell)
+    {
         bool cellChange = cell->mData.mX != mCellX || cell->mData.mY != mCellY;
-        if(!mPathFinder.isPathConstructed() || cellChange)
+        if (!mPathFinder.isPathConstructed() || cellChange)
         {
             mCellX = cell->mData.mX;
             mCellY = cell->mData.mY;
-
-            ESM::Pathgrid::Point dest(static_cast<int>(mX), static_cast<int>(mY), static_cast<int>(mZ));
-
-            ESM::Pathgrid::Point start(PathFinder::MakePathgridPoint(pos));
-
-            mPathFinder.buildPath(start, dest, actor.getCell(), true);
-        }
-
-        if(mPathFinder.checkPathCompleted(pos.pos[0], pos.pos[1]))
-        {
-            movement.mPosition[1] = 0;
             return true;
         }
-
-        zTurn(actor, Ogre::Degree(mPathFinder.getZAngleToNext(pos.pos[0], pos.pos[1])));
-        movement.mPosition[1] = 1;
-
         return false;
     }
 

apps/openmw/mwmechanics/aitravel.hpp

@@ -34,6 +34,9 @@ namespace MWMechanics
 
             virtual int getTypeId() const;
 
+        protected:
+            virtual bool doesPathNeedRecalc(ESM::Pathgrid::Point dest, const ESM::Cell *cell);
+
         private:
             float mX;
             float mY;

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
+        {
-
+            // if path has one point, then it's the destination.
-        std::list<ESM::Pathgrid::Point>::const_iterator oldStart = path.begin();
+            // 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
-        if(    (*iter).mX == oldStart->mX
+                    && iter->mY == oldStart.mY
-            && (*iter).mY == oldStart->mY
+                    && iter->mZ == oldStart.mZ)
-            && (*iter).mZ == oldStart->mZ)
                 {
                     mPath.pop_front();
-            return true;
                 }
-        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)
             {