Based on previous experimental changes, major update to the unit motion.

With this change, units will not check their movement against all
obstructions when moving: terrain and static obstructions are assumed to
be handled by the long-range pathfinder.
However, when static obstructions are changed, the paths have to be
invalidated. In order to minimize the performance impact, units will
check for obstructions when they move after a passability change. If
they collide with something, they will recompute a path that will take
into account the new passability map.

Also includes some code cleanup. This patch should not change
performance a lot: the lower number of checks should give a small
performance improvement while using the message broadcasting system
should hurt it a bit.

Fixes #3376, #3337, #1914.

This was SVN commit r16998.

source/simulation2/MessageTypes.h

@@ -538,4 +538,17 @@ public:
 	}
 };
 
+/**
+ * Sent when the pathfinder's passability map is modified on update
+ */
+class CMessagePassabilityMapChanged : public CMessage
+{
+public:
+	DEFAULT_MESSAGE_IMPL(PassabilityMapChanged)
+
+	CMessagePassabilityMapChanged()
+	{
+	}
+};
+
 #endif // INCLUDED_MESSAGETYPES

source/simulation2/TypeList.h

@@ -57,6 +57,7 @@ MESSAGE(ValueModification)
 MESSAGE(TemplateModification)
 MESSAGE(VisionRangeChanged)
 MESSAGE(MinimapPing)
+MESSAGE(PassabilityMapChanged)
 
 // TemplateManager must come before all other (non-test) components,
 // so that it is the first to be (de)serialized

source/simulation2/components/CCmpPathfinder.cpp

@@ -528,6 +528,10 @@ void CCmpPathfinder::UpdateGrid()
 	}
 	else
 		m_LongPathfinder.Update(m_Grid, m_ObstructionsDirty.dirtinessGrid);
+
+	// Notify the units that their current paths can be invalid now
+	CMessagePassabilityMapChanged msg;
+	GetSimContext().GetComponentManager().BroadcastMessage(msg);
 }
 
 void CCmpPathfinder::MinimalTerrainUpdate()
@@ -724,7 +728,7 @@ void CCmpPathfinder::ProcessShortRequests(const std::vector<AsyncShortPathReques
 	{
 		const AsyncShortPathRequest& req = shortRequests[i];
 		WaypointPath path;
-		ControlGroupMovementObstructionFilter filter(req.avoidMovingUnits, req.group);
+		ControlGroupMovementObstructionFilter filter(true, req.avoidMovingUnits, req.group);
 		ComputeShortPath(filter, req.x0, req.z0, req.clearance, req.range, req.goal, req.passClass, path);
 		CMessagePathResult msg(req.ticket, path);
 		GetSimContext().GetComponentManager().PostMessage(req.notify, msg);
@@ -794,16 +798,13 @@ bool CCmpPathfinder::CheckMovement(const IObstructionTestFilter& filter,
 	entity_pos_t x0, entity_pos_t z0, entity_pos_t x1, entity_pos_t z1, entity_pos_t r,
 	pass_class_t passClass)
 {
-	// Test against obstructions first
+	// Test against obstructions first. Pathfinding-blocking obstructions are not handled here.
 	CmpPtr<ICmpObstructionManager> cmpObstructionManager(GetSystemEntity());
-	if (!cmpObstructionManager)
+	if (!cmpObstructionManager || cmpObstructionManager->TestLine(filter, x0, z0, x1, z1, r))
 		return false;
 
-	if (cmpObstructionManager->TestLine(filter, x0, z0, x1, z1, r))
+	// Then test against the passability grid.
-		return false;
+	return Pathfinding::CheckLineMovement(x0, z0, x1, z1, passClass, *m_Grid);
-
-	// Then test against the terrain
-	return Pathfinding::CheckLineMovement(x0, z0, x1, z1, passClass, *m_TerrainOnlyGrid);
 }
 
 ICmpObstruction::EFoundationCheck CCmpPathfinder::CheckUnitPlacement(const IObstructionTestFilter& filter,

source/simulation2/components/CCmpUnitMotion.cpp

@@ -146,6 +146,7 @@ public:
 		componentManager.SubscribeToMessageType(MT_PathResult);
 		componentManager.SubscribeToMessageType(MT_ValueModification);
 		componentManager.SubscribeToMessageType(MT_Deserialized);
+		componentManager.SubscribeToMessageType(MT_PassabilityMapChanged);
 	}
 
 	DEFAULT_COMPONENT_ALLOCATOR(UnitMotion)
@@ -265,6 +266,10 @@ public:
 	WaypointPath m_LongPath;
 	WaypointPath m_ShortPath;
 
+	// When the passability map has changed, we cannot fully trust the path computed by the
+	// pathfinder before that change.
+	bool m_PassabilityMapChangedRecently;
+
 	// Motion planning
 	SUnitMotionPlanning m_Planning;
 
@@ -337,6 +342,8 @@ public:
 
 		m_ExpectedPathTicket = 0;
 
+		m_PassabilityMapChangedRecently = false;
+
 		m_TargetEntity = INVALID_ENTITY;
 
 		m_FinalGoal.type = PathGoal::POINT;
@@ -373,6 +380,7 @@ public:
 
 		SerializeVector<SerializeWaypoint>()(serialize, "long path", m_LongPath.m_Waypoints);
 		SerializeVector<SerializeWaypoint>()(serialize, "short path", m_ShortPath.m_Waypoints);
+		serialize.Bool("passability map changed recently", m_PassabilityMapChangedRecently);
 
 		SerializeUnitMotionPlanning()(serialize, "planning", m_Planning);
 
@@ -456,6 +464,9 @@ public:
 			m_RunSpeed = newRunSpeed;
 			break;
 		}
+		case MT_PassabilityMapChanged:
+			m_PassabilityMapChangedRecently = true;
+			break;
 		}
 	}
 
@@ -661,7 +672,14 @@ private:
 	/**
 	 * Returns an appropriate obstruction filter for use with path requests.
 	 */
-	ControlGroupMovementObstructionFilter GetObstructionFilter(bool forceAvoidMovingUnits = false) const;
+	ControlGroupMovementObstructionFilter GetObstructionFilter(bool avoidPathfindingShapes) const;
+
+	/**
+	 * Checks our movement towards the next path waypoint.
+	 * Pathfinding-blocking shapes are assumed to be taken into account during the path computation
+	 * and this will only be used to avoid moving units.
+	 */
+	bool CheckMovement(entity_pos_t x0, entity_pos_t z0, entity_pos_t x1, entity_pos_t z1) const;
 
 	/**
 	 * Start moving to the given goal, from our current position 'from'.
@@ -708,10 +726,27 @@ void CCmpUnitMotion::PathResult(u32 ticket, const WaypointPath& path)
 
 	m_ExpectedPathTicket = 0; // we don't expect to get this result again
 
-	if (m_PathState == PATHSTATE_WAITING_REQUESTING_LONG)
+	// Check that we are still able to do something with that path
+	CmpPtr<ICmpPosition> cmpPosition(GetEntityHandle());
+	if (!cmpPosition || !cmpPosition->IsInWorld())
 	{
+		if (m_PathState == PATHSTATE_WAITING_REQUESTING_LONG || m_PathState == PATHSTATE_WAITING_REQUESTING_SHORT)
+			StartFailed();
+		else if (m_PathState == PATHSTATE_FOLLOWING_REQUESTING_LONG || m_PathState == PATHSTATE_FOLLOWING_REQUESTING_SHORT)
+			StopMoving();
+		return;
+	}
+
+	if (m_PathState == PATHSTATE_WAITING_REQUESTING_LONG || m_PathState == PATHSTATE_FOLLOWING_REQUESTING_LONG)
+	{
+		m_PassabilityMapChangedRecently = false;
+
 		m_LongPath = path;
-		m_ShortPath.m_Waypoints.clear();
+
+		// If we are following a path, leave the old m_ShortPath so we can carry on following it
+		// until a new short path has been computed
+		if (m_PathState == PATHSTATE_WAITING_REQUESTING_LONG)
+			m_ShortPath.m_Waypoints.clear();
 
 		// If there's no waypoints then we couldn't get near the target.
 		// Sort of hack: Just try going directly to the goal point instead
@@ -720,15 +755,10 @@ void CCmpUnitMotion::PathResult(u32 ticket, const WaypointPath& path)
 		if (m_LongPath.m_Waypoints.empty())
 			m_LongPath.m_Waypoints.emplace_back(Waypoint{ m_FinalGoal.x, m_FinalGoal.z });
 
-		CmpPtr<ICmpPosition> cmpPosition(GetEntityHandle());
-		if (!cmpPosition || !cmpPosition->IsInWorld())
-		{
-			StartFailed();
-			return;
-		}
-
 		m_PathState = PATHSTATE_FOLLOWING;
-		StartSucceeded();
+
+		if (m_PathState == PATHSTATE_WAITING_REQUESTING_LONG)
+			StartSucceeded();
 	}
 	else if (m_PathState == PATHSTATE_WAITING_REQUESTING_SHORT)
 	{
@@ -753,44 +783,10 @@ void CCmpUnitMotion::PathResult(u32 ticket, const WaypointPath& path)
 			}
 		}
 
-		CmpPtr<ICmpPosition> cmpPosition(GetEntityHandle());
-		if (!cmpPosition || !cmpPosition->IsInWorld())
-		{
-			StartFailed();
-			return;
-		}
-
 		// Now we've got a short path that we can follow
 		m_PathState = PATHSTATE_FOLLOWING;
-
 		StartSucceeded();
 	}
-	else if (m_PathState == PATHSTATE_FOLLOWING_REQUESTING_LONG)
-	{
-		m_LongPath = path;
-		// Leave the old m_ShortPath - we'll carry on following it until the
-		// new short path has been computed
-
-		// If there's no waypoints then we couldn't get near the target.
-		// Sort of hack: Just try going directly to the goal point instead
-		// (via the short pathfinder), so if we're stuck and the user clicks
-		// close enough to the unit then we can probably get unstuck
-		if (m_LongPath.m_Waypoints.empty())
-			m_LongPath.m_Waypoints.emplace_back(Waypoint{ m_FinalGoal.x, m_FinalGoal.z });
-
-		CmpPtr<ICmpPosition> cmpPosition(GetEntityHandle());
-		if (!cmpPosition || !cmpPosition->IsInWorld())
-		{
-			StopMoving();
-			return;
-		}
-
-		m_PathState = PATHSTATE_FOLLOWING;
-
-		// (TODO: is this entirely safe? We might continue moving along our
-		// old path while this request is active, so it'll be slightly incorrect
-		// by the time the request has completed)
-	}
 	else if (m_PathState == PATHSTATE_FOLLOWING_REQUESTING_SHORT)
 	{
 		// Replace the current path with the new one
@@ -860,10 +856,6 @@ void CCmpUnitMotion::Move(fixed dt)
 		// Maybe we should split the updates into multiple phases to minimise
 		// that problem.
 
-		CmpPtr<ICmpPathfinder> cmpPathfinder(GetSystemEntity());
-		if (!cmpPathfinder)
-			return;
-
 		CmpPtr<ICmpPosition> cmpPosition(GetEntityHandle());
 		if (!cmpPosition || !cmpPosition->IsInWorld())
 			return;
@@ -923,7 +915,7 @@ void CCmpUnitMotion::Move(fixed dt)
 			fixed offsetLength = offset.Length();
 			if (offsetLength <= maxdist)
 			{
-				if (cmpPathfinder->CheckMovement(GetObstructionFilter(), pos.X, pos.Y, target.X, target.Y, m_Clearance, m_PassClass))
+				if (CheckMovement(pos.X, pos.Y, target.X, target.Y))
 				{
 					pos = target;
 
@@ -953,7 +945,7 @@ void CCmpUnitMotion::Move(fixed dt)
 				offset.Normalize(maxdist);
 				target = pos + offset;
 
-				if (cmpPathfinder->CheckMovement(GetObstructionFilter(), pos.X, pos.Y, target.X, target.Y, m_Clearance, m_PassClass))
+				if (CheckMovement(pos.X, pos.Y, target.X, target.Y))
 				{
 					pos = target;
 					break;
@@ -982,16 +974,15 @@ void CCmpUnitMotion::Move(fixed dt)
 		
 		if (wasObstructed)
 		{
-			// Oops, we hit something (very likely another unit).
+			// Oops, we hit something (very likely another unit, or a new obstruction).
 			// Stop, and recompute the whole path.
 			// TODO: if the target has UnitMotion and is higher priority,
 			// we should wait a little bit.
 
 			// Recompute our path
-			// If we are following a long path
+			// If we are following a long path and it is still valid
-			if (!m_LongPath.m_Waypoints.empty())
+			if (!m_LongPath.m_Waypoints.empty() && !m_PassabilityMapChangedRecently)
 			{
-				m_ShortPath.m_Waypoints.clear();
 				PathGoal goal = { PathGoal::POINT, m_LongPath.m_Waypoints.back().x, m_LongPath.m_Waypoints.back().z };
 				RequestShortPath(pos, goal, true);
 				m_PathState = PATHSTATE_WAITING_REQUESTING_SHORT;
@@ -1066,14 +1057,12 @@ void CCmpUnitMotion::PlanNextStep(const CFixedVector2D& pos)
 	if (m_LongPath.m_Waypoints.empty())
 		return;
 
-	CmpPtr<ICmpPathfinder> cmpPathfinder(GetSystemEntity());
 
 	const Waypoint& nextPoint = m_LongPath.m_Waypoints.back();
 
 	// The next step was obstructed the last time we checked; also check that
 	// the step is still obstructed (maybe the units in our way moved in the meantime)
-	if (!m_Planning.nextStepClean &&
+	if (!m_Planning.nextStepClean && CheckMovement(pos.X, pos.Y, nextPoint.x, nextPoint.z))
-		!cmpPathfinder->CheckMovement(GetObstructionFilter(), pos.X, pos.Y, nextPoint.x, nextPoint.z, m_Clearance, m_PassClass))
 	{
 		// If the short path computation is over, use it, else just forget about it
 		if (!m_Planning.nextStepShortPath.m_Waypoints.empty())
@@ -1089,10 +1078,13 @@ void CCmpUnitMotion::PlanNextStep(const CFixedVector2D& pos)
 		return;
 
 	const Waypoint& followingPoint = m_LongPath.m_Waypoints.rbegin()[1]; // penultimate element
-	m_Planning.nextStepClean = cmpPathfinder->CheckMovement(
+	m_Planning.nextStepClean = CheckMovement(nextPoint.x, nextPoint.z, followingPoint.x, followingPoint.z);
-		GetObstructionFilter(), nextPoint.x, nextPoint.z, followingPoint.x, followingPoint.z, m_Clearance, m_PassClass);
 	if (!m_Planning.nextStepClean)
 	{
+		CmpPtr<ICmpPathfinder> cmpPathfinder(GetSystemEntity());
+		if (!cmpPathfinder)
+			return;
+
 		PathGoal goal = { PathGoal::POINT, followingPoint.x, followingPoint.z };
 		m_Planning.expectedPathTicket = cmpPathfinder->ComputeShortPathAsync(
 			nextPoint.x, nextPoint.z, m_Clearance, SHORT_PATH_SEARCH_RANGE, goal, m_PassClass, false, m_TargetEntity, GetEntityId());
@@ -1139,7 +1131,7 @@ bool CCmpUnitMotion::TryGoingStraightToGoalPoint(const CFixedVector2D& from)
 		return false;
 
 	// Check if there's any collisions on that route
-	if (!cmpPathfinder->CheckMovement(GetObstructionFilter(), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
+	if (!cmpPathfinder->CheckMovement(GetObstructionFilter(true), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
 		return false;
 
 	// That route is okay, so update our path
@@ -1175,7 +1167,7 @@ bool CCmpUnitMotion::TryGoingStraightToTargetEntity(const CFixedVector2D& from)
 	CFixedVector2D goalPos = goal.NearestPointOnGoal(from);
 
 	// Check if there's any collisions on that route
-	if (!cmpPathfinder->CheckMovement(GetObstructionFilter(), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
+	if (!cmpPathfinder->CheckMovement(GetObstructionFilter(true), from.X, from.Y, goalPos.X, goalPos.Y, m_Clearance, m_PassClass))
 		return false;
 
 	// That route is okay, so update our path
@@ -1283,7 +1275,7 @@ void CCmpUnitMotion::FaceTowardsPointFromPos(const CFixedVector2D& pos, entity_p
 	}
 }
 
-ControlGroupMovementObstructionFilter CCmpUnitMotion::GetObstructionFilter(bool forceAvoidMovingUnits) const
+ControlGroupMovementObstructionFilter CCmpUnitMotion::GetObstructionFilter(bool avoidPathfindingShapes) const
 {
 	entity_id_t group;
 	if (IsFormationMember())
@@ -1291,9 +1283,25 @@ ControlGroupMovementObstructionFilter CCmpUnitMotion::GetObstructionFilter(bool
 	else
 		group = GetEntityId();
 
-	return ControlGroupMovementObstructionFilter(forceAvoidMovingUnits || ShouldAvoidMovingUnits(), group);
+	return ControlGroupMovementObstructionFilter(avoidPathfindingShapes, ShouldAvoidMovingUnits(), group);
 }
 
+bool CCmpUnitMotion::CheckMovement(entity_pos_t x0, entity_pos_t z0, entity_pos_t x1, entity_pos_t z1) const
+{
+	// If the passability map has changed, we have to check everything in our way until we compute a new path.
+	if (m_PassabilityMapChangedRecently)
+	{
+		CmpPtr<ICmpPathfinder> cmpPathfinder(GetSystemEntity());
+		return cmpPathfinder && cmpPathfinder->CheckMovement(GetObstructionFilter(true), x0, z0, x1, z1, m_Clearance, m_PassClass);
+	}
+
+	// If an obstruction blocks tile-based pathfinding, it will be handled during the path computation
+	// and doesn't need to be matched by this filter for the movement.
+	ControlGroupMovementObstructionFilter filter = GetObstructionFilter(false);
+
+	CmpPtr<ICmpObstructionManager> cmpObstructionManager(GetSystemEntity());
+	return cmpObstructionManager && !cmpObstructionManager->TestLine(filter, x0, z0, x1, z1, m_Clearance);
+}
 
 
 void CCmpUnitMotion::BeginPathing(const CFixedVector2D& from, const PathGoal& goal)
@@ -1309,6 +1317,9 @@ void CCmpUnitMotion::BeginPathing(const CFixedVector2D& from, const PathGoal& go
 	if (cmpObstruction)
 		cmpObstruction->SetMovingFlag(true);
 
+	// We are going to recompute our path, so we will use the most recent passability grid
+	m_PassabilityMapChangedRecently = false;
+
 #if DISABLE_PATHFINDER
 	{
 		CmpPtr<ICmpPathfinder> cmpPathfinder (GetSimContext(), SYSTEM_ENTITY);

source/simulation2/components/ICmpObstructionManager.h

@@ -333,12 +333,13 @@ public:
  */
 class ControlGroupMovementObstructionFilter : public IObstructionTestFilter
 {
+	bool m_AvoidPathfindingShapes;
 	bool m_AvoidMoving;
 	entity_id_t m_Group;
 
 public:
-	ControlGroupMovementObstructionFilter(bool avoidMoving, entity_id_t group) :
+	ControlGroupMovementObstructionFilter(bool avoidPathfindingShapes, bool avoidMoving, entity_id_t group) :
-		m_AvoidMoving(avoidMoving), m_Group(group)
+		m_AvoidPathfindingShapes(avoidPathfindingShapes), m_AvoidMoving(avoidMoving), m_Group(group)
 	{}
 
 	virtual bool TestShape(tag_t UNUSED(tag), flags_t flags, entity_id_t group, entity_id_t group2) const
@@ -346,6 +347,9 @@ public:
 		if (group == m_Group || (group2 != INVALID_ENTITY && group2 == m_Group))
 			return false;
 
+		if ((flags & ICmpObstructionManager::FLAG_BLOCK_PATHFINDING) && !m_AvoidPathfindingShapes)
+			return false;
+
 		if (!(flags & ICmpObstructionManager::FLAG_BLOCK_MOVEMENT))
 			return false;
 

source/simulation2/scripting/MessageTypeConversions.cpp

@@ -1,4 +1,4 @@
-/* Copyright (C) 2014 Wildfire Games.
+/* Copyright (C) 2015 Wildfire Games.
  * This file is part of 0 A.D.
  *
  * 0 A.D. is free software: you can redistribute it and/or modify
@@ -470,6 +470,19 @@ CMessage* CMessageMinimapPing::FromJSVal(ScriptInterface& UNUSED(scriptInterface
 	return new CMessageMinimapPing();
 }
 
+////////////////////////////////
+
+JS::Value CMessagePassabilityMapChanged::ToJSVal(ScriptInterface& scriptInterface) const
+{
+	TOJSVAL_SETUP();
+	return JS::ObjectValue(*obj);
+}
+
+CMessage* CMessagePassabilityMapChanged::FromJSVal(ScriptInterface& UNUSED(scriptInterface), JS::HandleValue UNUSED(val))
+{
+	return new CMessagePassabilityMapChanged();
+}
+
 ////////////////////////////////////////////////////////////////
 
 CMessage* CMessageFromJSVal(int mtid, ScriptInterface& scriptingInterface, JS::HandleValue val)