UnitMotion - Fix a rare pathfinding issues where units tried going straight through walls, and make sure a long path is computed even when the target is within short path or direct path range.

This fixes a regression introduced by 055c848c1a: when an entity is
ordered to move to a target within short path distance (or direct path
distance), it no longer computed at least one long path, which meant it
could be stuck forever if the target was not actually reachable (such as
behind a wall).
To fix this, compute a long path after 3 failed computations, which
should result in a delay of 1-3 turns. The previous code did this after
1 failed try - the decision to make it 3 is mostly based on the idea
that in most cases, being stuck means we ran into units, not that we
were ordered somewhere close. Should there be complaints, it could be
lowered to 2 or 1.

This fixes a second issue, reported in #4473: units sometimes get stuck,
particularly when trying to garrison a turret from 'inside' the walls.
The issue is that the turret is not accessible via the inside as its
obstruction + garrison range is blocked by the surrounding walls.
However, as introduced by 6e05a00929, TryGoingStraightToTargetEntity
ignores all entities with the obstruction group of the target (the
reason for this being that otherwise it would never succeed, since the
line towards the target would likely go through the target).
For walls and formations, this means ignoring possibly too many
entities, and in the case of #4473, ignoring wall pieces. The unit thus
mistakenly thought it could direct-path to the turret, and got stuck.

To fix this, we can ignore specifically the targeted entity's
obstruction tag. This can be considered a fix to 6e05a00929.

temple accepted an earlier version of this patch (specifically elexis'
version).

Fixes #4473

Based on a patch by: elexis
Differential Revision: https://code.wildfiregames.com/D1424
This was SVN commit r22533.

source/simulation2/components/CCmpUnitMotion.cpp

@@ -93,6 +93,12 @@ static const entity_pos_t TARGET_UNCERTAINTY_MULTIPLIER = entity_pos_t::FromInt(
  */
 static const u8 MAX_FAILED_PATH_COMPUTATIONS = 15;
 
+/**
+ * If we have failed path computations this many times and ComputePathToGoal is called,
+ * always run a long-path, to avoid getting stuck sometimes (see D1424).
+ */
+static const u8 MAX_FAILED_PATH_COMPUTATIONS_BEFORE_LONG_PATH = 3;
+
 static const CColor OVERLAY_COLOR_LONG_PATH(1, 1, 1, 1);
 static const CColor OVERLAY_COLOR_SHORT_PATH(1, 0, 0, 1);
 
@@ -602,10 +608,8 @@ private:
 
 	/**
 	 * Returns an appropriate obstruction filter for use with path requests.
-	 * noTarget is true only when used inside TryGoingStraightToTarget,
-	 * in which case we do not want the target obstruction otherwise it would always fail
 	 */
-	ControlGroupMovementObstructionFilter GetObstructionFilter(bool noTarget = false) const;
+	ControlGroupMovementObstructionFilter GetObstructionFilter() const;
 
 	/**
 	 * Create a PathGoal from a move request.
@@ -1035,15 +1039,29 @@ bool CCmpUnitMotion::TryGoingStraightToTarget(const CFixedVector2D& from)
 	// Find the point on the goal shape that we should head towards
 	CFixedVector2D goalPos = goal.NearestPointOnGoal(from);
 
-	// Check if there's any collisions on that route
-	if (!cmpPathfinder->CheckMovement(GetObstructionFilter(true), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
+	// Check if there's any collisions on that route.
+	// For entity goals, skip only the specific obstruction tag or with e.g. walls we might ignore too many entities.
+	ICmpObstructionManager::tag_t specificIgnore;
+	if (m_MoveRequest.m_Type == MoveRequest::ENTITY)
+	{
+		CmpPtr<ICmpObstruction> cmpTargetObstruction(GetSimContext(), m_MoveRequest.m_Entity);
+		if (cmpTargetObstruction)
+			specificIgnore = cmpTargetObstruction->GetObstruction();
+	}
+
+	if (specificIgnore.valid())
+	{
+		if (!cmpPathfinder->CheckMovement(SkipTagObstructionFilter(specificIgnore), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
+			return false;
+	}
+	else if (!cmpPathfinder->CheckMovement(GetObstructionFilter(), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
 		return false;
 
+
 	// That route is okay, so update our path
 	m_LongPath.m_Waypoints.clear();
 	m_ShortPath.m_Waypoints.clear();
 	m_ShortPath.m_Waypoints.emplace_back(Waypoint{ goalPos.X, goalPos.Y });
-
 	return true;
 }
 
@@ -1158,10 +1176,9 @@ void CCmpUnitMotion::FaceTowardsPointFromPos(const CFixedVector2D& pos, entity_p
 	}
 }
 
-ControlGroupMovementObstructionFilter CCmpUnitMotion::GetObstructionFilter(bool noTarget) const
+ControlGroupMovementObstructionFilter CCmpUnitMotion::GetObstructionFilter() const
 {
-	entity_id_t group = noTarget ? m_MoveRequest.m_Entity : GetGroup();
-	return ControlGroupMovementObstructionFilter(ShouldAvoidMovingUnits(), group);
+	return ControlGroupMovementObstructionFilter(ShouldAvoidMovingUnits(), GetGroup());
 }
 
 // The pathfinder cannot go to "rounded rectangles" goals, which are what happens with square targets and a non-null range.
@@ -1306,7 +1323,7 @@ void CCmpUnitMotion::BeginPathing(const CFixedVector2D& from, const PathGoal& go
 
 	// If the target is close and we can reach it in a straight line,
 	// then we'll just go along the straight line instead of computing a path.
-	if (TryGoingStraightToTarget(from))
+	if (m_FailedPathComputations != MAX_FAILED_PATH_COMPUTATIONS_BEFORE_LONG_PATH && TryGoingStraightToTarget(from))
 		return;
 
 	// Otherwise we need to compute a path.
@@ -1315,7 +1332,8 @@ void CCmpUnitMotion::BeginPathing(const CFixedVector2D& from, const PathGoal& go
 	// TODO: If it's close on the opposite side of a river then we really
 	// need a long path, so we shouldn't simply check linear distance
 	// the check is arbitrary but should be a reasonably small distance.
-	if (goal.DistanceToPoint(from) < LONG_PATH_MIN_DIST)
+	// To avoid getting stuck because the short-range pathfinder is bounded, occasionally compute a long path instead.
+	if (m_FailedPathComputations != MAX_FAILED_PATH_COMPUTATIONS_BEFORE_LONG_PATH && goal.DistanceToPoint(from) < LONG_PATH_MIN_DIST)
 	{
 		m_LongPath.m_Waypoints.clear();
 		RequestShortPath(from, goal, true);