Unit Motion: MoveTo family of function no longer returns false if the move is un-necessary, instead unitAI checks explicitly.

This also moves the actual "moving" code to states instead of orders,
making states more self-contained and removing the change of errors when
cleaning up a state.

Differential Revision: https://code.wildfiregames.com/D1865
This was SVN commit r22313.

binaries/data/mods/public/simulation/components/tests/test_UnitAI.js

@@ -11,6 +11,7 @@ Engine.LoadComponentScript("interfaces/Heal.js");
 Engine.LoadComponentScript("interfaces/Health.js");
 Engine.LoadComponentScript("interfaces/Pack.js");
 Engine.LoadComponentScript("interfaces/ResourceSupply.js");
+Engine.LoadComponentScript("interfaces/ResourceGatherer.js");
 Engine.LoadComponentScript("interfaces/Timer.js");
 Engine.LoadComponentScript("interfaces/UnitAI.js");
 Engine.LoadComponentScript("Formation.js");
@@ -86,7 +87,7 @@ function TestFormationExiting(mode)
 		GetWalkSpeed: function() { return 1; },
 		MoveToFormationOffset: function(target, x, z) { },
 		IsInTargetRange: function(target, min, max) { return true; },
-		MoveToTargetRange: function(target, min, max) { },
+		MoveToTargetRange: function(target, min, max) { return true; },
 		StopMoving: function() { },
 		GetPassabilityClassName: function() { return "default"; },
 	});
@@ -137,10 +138,11 @@ function TestFormationExiting(mode)
 	});
 
 	AddMock(controller, IID_UnitMotion, {
-		GetWalkSpeed: function() { return 1; },
-		SetSpeedMultiplier: function(speed) { },
-		MoveToPointRange: function(x, z, minRange, maxRange) { },
-		GetPassabilityClassName: function() { return "default"; },
+		"GetWalkSpeed": () => 1,
+		"StopMoving": () => {},
+		"SetSpeedMultiplier": () => {},
+		"MoveToPointRange": () => true,
+		"GetPassabilityClassName": () => "default"
 	});
 
 	controllerAI.OnCreate();
@@ -238,7 +240,7 @@ function TestMoveIntoFormationWhileAttacking()
 			GetWalkSpeed: function() { return 1; },
 			MoveToFormationOffset: function(target, x, z) { },
 			IsInTargetRange: function(target, min, max) { return true; },
-			MoveToTargetRange: function(target, min, max) { },
+			MoveToTargetRange: function(target, min, max) { return true; },
 			StopMoving: function() { },
 			GetPassabilityClassName: function() { return "default"; },
 		});

source/simulation2/components/CCmpUnitMotion.cpp

@@ -1051,25 +1051,25 @@ void CCmpUnitMotion::Move(fixed dt)
 						m_Moving = false;
 						CMessageMotionChanged msg(false, false);
 						GetSimContext().GetComponentManager().PostMessage(GetEntityId(), msg);
+						return;
 					}
 				}
 				else
 				{
 					// check if target was reached in case of a moving target
 					CmpPtr<ICmpUnitMotion> cmpUnitMotion(GetSimContext(), m_TargetEntity);
-					if (cmpUnitMotion && cmpUnitMotion->IsMoving() &&
-						MoveToTargetRange(m_TargetEntity, m_TargetMinRange, m_TargetMaxRange))
-						return;
+					if (!cmpUnitMotion || cmpUnitMotion->IsInTargetRange(GetEntityId(), m_TargetMinRange, m_TargetMaxRange))
+					{
+						// Not in formation, so just finish moving
+						StopMoving();
+						m_State = STATE_IDLE;
+						MoveSucceeded();
 
-					// Not in formation, so just finish moving
-					StopMoving();
-					m_State = STATE_IDLE;
-					MoveSucceeded();
-
-					if (m_FacePointAfterMove)
-						FaceTowardsPointFromPos(pos, m_FinalGoal.x, m_FinalGoal.z);
-					// TODO: if the goal was a square building, we ought to point towards the
-					// nearest point on the square, not towards its center
+						if (m_FacePointAfterMove)
+							FaceTowardsPointFromPos(pos, m_FinalGoal.x, m_FinalGoal.z);
+						// TODO: if the goal was a square building, we ought to point towards the
+						// nearest point on the square, not towards its center
+					}
 				}
 			}
 
@@ -1454,7 +1454,7 @@ bool CCmpUnitMotion::MoveToPointRange(entity_pos_t x, entity_pos_t z, entity_pos
 			// We're already in range - no need to move anywhere
 			if (m_FacePointAfterMove)
 				FaceTowardsPointFromPos(pos, x, z);
-			return false;
+			return true;
 		}
 	}
 
@@ -1623,7 +1623,7 @@ bool CCmpUnitMotion::MoveToTargetRange(entity_id_t target, entity_pos_t minRange
 	{
 		// We're already in range - no need to move anywhere
 		FaceTowardsPointFromPos(pos, goal.x, goal.z);
-		return false;
+		return true;
 	}
 	else
 	{

source/simulation2/components/ICmpUnitMotion.h

@@ -38,10 +38,8 @@ public:
 	/**
 	 * Attempt to walk into range of a to a given point, or as close as possible.
 	 * The range is measured from the center of the unit.
-	 * If the unit is already in range, or cannot move anywhere at all, or if there is
-	 * some other error, then returns false.
-	 * Otherwise, returns true and sends a MotionChanged message after starting to move,
-	 * and sends another MotionChanged after finishing moving.
+	 * If cannot move anywhere at all, or if there is some other error, then returns false.
+	 * Otherwise, returns true.
 	 * If maxRange is negative, then the maximum range is treated as infinity.
 	 */
 	virtual bool MoveToPointRange(entity_pos_t x, entity_pos_t z, entity_pos_t minRange, entity_pos_t maxRange) = 0;
@@ -62,10 +60,8 @@ public:
 	 * Attempt to walk into range of a given target entity, or as close as possible.
 	 * The range is measured between approximately the edges of the unit and the target, so that
 	 * maxRange=0 is not unreachably close to the target.
-	 * If the unit is already in range, or cannot move anywhere at all, or if there is
-	 * some other error, then returns false.
-	 * Otherwise, returns true and sends a MotionChanged message after starting to move,
-	 * and sends another MotionChanged after finishing moving.
+	 * If the unit cannot move anywhere at all, or if there is some other error, then returns false.
+	 * Otherwise, returns true.
 	 * If maxRange is negative, then the maximum range is treated as infinity.
 	 */
 	virtual bool MoveToTargetRange(entity_id_t target, entity_pos_t minRange, entity_pos_t maxRange) = 0;