Merge pull request #2446 from cuberite/asyncpathfinder

Fixed a position bug in the pathfinder

src/Mobs/Monster.cpp

@@ -76,8 +76,8 @@ cMonster::cMonster(const AString & a_ConfigName, eMonsterType a_MobType, const A
 	, m_Target(nullptr)
 	, m_Path(nullptr)
 	, m_IsFollowingPath(false)
+	, m_PathfinderActivated(false)
 	, m_GiveUpCounter(0)
-	, m_TicksSinceLastPathReset(1000)
 	, m_LastGroundHeight(POSY_TOINT)
 	, m_JumpCoolDown(0)
 	, m_IdleInterval(0)
@@ -124,15 +124,10 @@ void cMonster::SpawnOn(cClientHandle & a_Client)
 
 bool cMonster::TickPathFinding(cChunk & a_Chunk)
 {
-	if (!m_IsFollowingPath)
+	if (!m_PathfinderActivated)
 	{
 		return false;
 	}
-	if (m_TicksSinceLastPathReset < 1000)
-	{
-		// No need to count beyond 1000. 1000 is arbitary here.
-		++m_TicksSinceLastPathReset;
-	}
 
 	if (ReachedFinalDestination())
 	{
@@ -140,26 +135,6 @@ bool cMonster::TickPathFinding(cChunk & a_Chunk)
 		return false;
 	}
 
-	if ((m_FinalDestination - m_PathFinderDestination).Length() > 0.25)  // if the distance between where we're going and where we should go is too big.
-	{
-		/* If we reached the last path waypoint,
-		Or if we haven't re-calculated for too long.
-		Interval is proportional to distance squared, and its minimum is 10.
-		(Recalculate lots when close, calculate rarely when far) */
-		if (
-			((GetPosition() - m_PathFinderDestination).Length() < 0.25) ||
-			((m_TicksSinceLastPathReset > 10) && (m_TicksSinceLastPathReset > (0.4 * (m_FinalDestination - GetPosition()).SqrLength())))
-		)
-		{
-			/* Re-calculating is expensive when there's no path to target, and it results in mobs freezing very often as a result of always recalculating.
-			This is a workaround till we get better path recalculation. */
-			if (!m_NoPathToTarget)
-			{
-				ResetPathFinding();
-			}
-		}
-	}
-
 	if (m_Path == nullptr)
 	{
 		if (!EnsureProperDestination(a_Chunk))
@@ -167,21 +142,17 @@ bool cMonster::TickPathFinding(cChunk & a_Chunk)
 			StopMovingToPosition();  // Invalid chunks, probably world is loading or something, cancel movement.
 			return false;
 		}
-		m_NoPathToTarget = false;
+		m_GiveUpCounter = 40;
-		m_NoMoreWayPoints = false;
+		m_Path = new cPath(a_Chunk, GetPosition(), m_FinalDestination, 20, GetWidth(), GetHeight());
-		m_PathFinderDestination = m_FinalDestination;
-		m_Path = new cPath(a_Chunk, GetPosition(), m_PathFinderDestination, 20, GetWidth(), GetHeight());
 	}
 
 	switch (m_Path->Step(a_Chunk))
 	{
 		case ePathFinderStatus::NEARBY_FOUND:
 		{
-			m_NoPathToTarget = true;
+			m_FinalDestination = m_Path->AcceptNearbyPath();
-			m_PathFinderDestination = m_Path->AcceptNearbyPath();
 			break;
 		}
-
 		case ePathFinderStatus::PATH_NOT_FOUND:
 		{
 			StopMovingToPosition();  // Try to calculate a path again.
@@ -195,9 +166,10 @@ bool cMonster::TickPathFinding(cChunk & a_Chunk)
 		}
 		case ePathFinderStatus::PATH_FOUND:
 		{
-			if (m_NoMoreWayPoints || (--m_GiveUpCounter == 0))
+			if ((--m_GiveUpCounter) == 0)
 			{
-				if (m_EMState == ATTACKING)
+				// Failed to reach a waypoint - that's a failure condition whichever point we're at
+				if (m_EMState == CHASING)
 				{
 					ResetPathFinding();  // Try to calculate a path again.
 					// This results in mobs hanging around an unreachable target (player).
@@ -216,10 +188,8 @@ bool cMonster::TickPathFinding(cChunk & a_Chunk)
 					m_GiveUpCounter = 40;  // Give up after 40 ticks (2 seconds) if failed to reach m_NextWayPointPosition.
 				}
 			}
-			else
+
-			{
+			m_IsFollowingPath = true;
-				m_NoMoreWayPoints = true;
-			}
 			return true;
 		}
 	}
@@ -389,8 +359,8 @@ bool cMonster::EnsureProperDestination(cChunk & a_Chunk)
 
 void cMonster::MoveToPosition(const Vector3d & a_Position)
 {
-		m_FinalDestination = a_Position;
+	m_FinalDestination = a_Position;
-		m_IsFollowingPath = true;
+	m_PathfinderActivated = true;
 }
 
 
@@ -399,7 +369,7 @@ void cMonster::MoveToPosition(const Vector3d & a_Position)
 
 void cMonster::StopMovingToPosition()
 {
-	m_IsFollowingPath = false;
+	m_PathfinderActivated = false;
 	ResetPathFinding();
 }
 
@@ -409,7 +379,7 @@ void cMonster::StopMovingToPosition()
 
 void cMonster::ResetPathFinding(void)
 {
-	m_TicksSinceLastPathReset = 0;
+	m_IsFollowingPath = false;
 	if (m_Path != nullptr)
 	{
 		delete m_Path;

src/Mobs/Monster.h

@@ -173,9 +173,11 @@ protected:
 	/** Stores if mobile is currently moving towards the ultimate, final destination */
 	bool m_IsFollowingPath;
 
+	/** Stores if pathfinder is being used - set when final destination is set, and unset when stopped moving to final destination */
+	bool m_PathfinderActivated;
+
 	/* If 0, will give up reaching the next m_NextWayPointPosition and will re-compute path. */
 	int m_GiveUpCounter;
-	int m_TicksSinceLastPathReset;
 
 	/** Coordinates of the next position that should be reached */
 	Vector3d m_NextWayPointPosition;
@@ -183,16 +185,6 @@ protected:
 	/** Coordinates for the ultimate, final destination. */
 	Vector3d m_FinalDestination;
 
-	/** Coordinates for the ultimate, final destination last given to the pathfinder. */
-	Vector3d m_PathFinderDestination;
-
-	/** True if there's no path to target and we're walking to an approximated location. */
-	bool m_NoPathToTarget;
-
-	/** Whether The mob has finished their path, note that this does not imply reaching the destination,
-	the destination may sometimes differ from the current path. */
-	bool m_NoMoreWayPoints;
-
 	/** Finds the lowest non-air block position (not the highest, as cWorld::GetHeight does)
 	If current Y is nonsolid, goes down to try to find a solid block, then returns that + 1
 	If current Y is solid, goes up to find first nonsolid block, and returns that.

src/Mobs/Path.cpp

@@ -17,9 +17,6 @@
 
 
 
-
-
-
 bool compareHeuristics::operator()(cPathCell * & a_Cell1, cPathCell * & a_Cell2)
 {
 	return a_Cell1->m_F > a_Cell2->m_F;
@@ -36,7 +33,7 @@ cPath::cPath(
 	double a_BoundingBoxWidth, double a_BoundingBoxHeight,
 	int a_MaxUp, int a_MaxDown
 ) :
-
+	m_StepsLeft(a_MaxSteps),
 	m_CurrentPoint(0),  // GetNextPoint increments this to 1, but that's fine, since the first cell is always a_StartingPoint
 	m_Chunk(&a_Chunk),
 	m_BadChunkFound(false)
@@ -67,22 +64,8 @@ cPath::cPath(
 
 	m_NearestPointToTarget = GetCell(m_Source);
 	m_Status = ePathFinderStatus::CALCULATING;
-	m_StepsLeft = a_MaxSteps;
 
-	ProcessCell(GetCell(a_StartingPoint), nullptr, 0);
+	ProcessCell(GetCell(m_Source), nullptr, 0);
-	m_Chunk = nullptr;
-}
-
-
-
-
-
-cPath::~cPath()
-{
-	if (m_Status == ePathFinderStatus::CALCULATING)
-	{
-		FinishCalculation();
-	}
 }
 
 
@@ -113,7 +96,7 @@ ePathFinderStatus cPath::Step(cChunk & a_Chunk)
 		int i;
 		for (i = 0; i < CALCULATIONS_PER_STEP; ++i)
 		{
-			if (Step_Internal())  // Step_Internal returns true when no more calculation is needed.
+			if (StepOnce())  // StepOnce returns true when no more calculation is needed.
 			{
 				break;  // if we're here, m_Status must have changed either to PATH_FOUND or PATH_NOT_FOUND.
 			}
@@ -179,7 +162,7 @@ bool cPath::IsSolid(const Vector3i & a_Location)
 
 
 
-bool cPath::Step_Internal()
+bool cPath::StepOnce()
 {
 	cPathCell * CurrentCell = OpenListPop();
 

src/Mobs/Path.h

@@ -72,10 +72,7 @@ public:
 		double a_BoundingBoxWidth, double a_BoundingBoxHeight,
 		int a_MaxUp = 1, int a_MaxDown = 1
 	);
-
+	
-	/** Destroys the path and frees its memory. */
-	~cPath();
-
 	/** Performs part of the path calculation and returns the appropriate status.
 	If NEARBY_FOUND is returned, it means that the destination is not reachable, but a nearby destination
 	is reachable. If the user likes the alternative destination, they can call AcceptNearbyPath to treat the path as found,
@@ -99,45 +96,27 @@ public:
 
 
 	/** Checks whether this is the last point or not. Never call getnextPoint when this is true. */
-	inline bool IsLastPoint()
+	inline bool IsLastPoint() const
 	{
 		ASSERT(m_Status == ePathFinderStatus::PATH_FOUND);
 		return (m_CurrentPoint == m_PathPoints.size() - 1);
 	}
 
-
+	inline bool IsFirstPoint() const
-	inline bool IsFirstPoint()
 	{
 		ASSERT(m_Status == ePathFinderStatus::PATH_FOUND);
 		return (m_CurrentPoint == 0);
 	}
 
 
-	/** Get the point at a_index. Remark: Internally, the indexes are reversed. */
-	inline Vector3d GetPoint(size_t a_index)
-	{
-		ASSERT(m_Status == ePathFinderStatus::PATH_FOUND);
-		ASSERT(a_index < m_PathPoints.size());
-		Vector3i Point = m_PathPoints[m_PathPoints.size() - 1 - a_index];
-		return Vector3d(Point.x + m_HalfWidth, Point.y, Point.z + m_HalfWidth);
-	}
 
 
-	/** Returns the total number of points this path has. */
-	inline size_t GetPointCount()
-	{
-		if (m_Status != ePathFinderStatus::PATH_FOUND)
-		{
-			return 0;
-		}
-		return m_PathPoints.size();
-	}
 
 private:
 
 	/* General */
 	bool IsSolid(const Vector3i & a_Location);  // Query our hosting world and ask it if there's a solid at a_location.
-	bool Step_Internal();  // The public version just calls this version * CALCULATIONS_PER_CALL times.
+	bool StepOnce();  // The public version just calls this version * CALCULATIONS_PER_CALL times.
 	void FinishCalculation();  // Clears the memory used for calculating the path.
 	void FinishCalculation(ePathFinderStatus a_NewStatus);  // Clears the memory used for calculating the path and changes the status.
 	void AttemptToFindAlternative();