Fix formations reshaping incorrectly & related issues.

Formation were reshaping in cases where they should not (such as when
the controller is waiting on members).
This made them look very odd for some orders.

This also cleans up held positions for formations somewhat.

Also:
- ed54ad3486 fixed units not facing correctly after a formation-walk.
The fix is correct, but we can actually make it slightly more
self-contained by moving everything in `leave`. Also clarify comments.
- This fixes some very broken code in 4ca448a686 in
FORMATIONMEMBER.WALKING.MovementUpdate. It never errored because
formation controllers weren't moved out of the world before.

Reviewed By: Angen
Fixes #5443

Differential Revision: https://code.wildfiregames.com/D2763
This was SVN commit r23806.

binaries/data/mods/public/simulation/components/Formation.js

@@ -312,8 +312,7 @@ Formation.prototype.AreAllMembersWaiting = function()
 };
 
 /**
- * Set whether we should rearrange formation members if
- * units are removed from the formation.
+ * Set whether we are allowed to rearrange formation members.
  */
 Formation.prototype.SetRearrange = function(rearrange)
 {
@@ -902,29 +901,33 @@ Formation.prototype.ComputeMotionParameters = function()
 
 Formation.prototype.ShapeUpdate = function()
 {
+	if (!this.rearrange)
+		return;
+
 	// Check the distance to twin formations, and merge if when
 	// the formations could collide
-	for (var i = this.twinFormations.length - 1; i >= 0; --i)
+	for (let i = this.twinFormations.length - 1; i >= 0; --i)
 	{
 		// only do the check on one side
 		if (this.twinFormations[i] <= this.entity)
 			continue;
-
-		var cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
-		var cmpOtherPosition = Engine.QueryInterface(this.twinFormations[i], IID_Position);
-		var cmpOtherFormation = Engine.QueryInterface(this.twinFormations[i], IID_Formation);
-		if (!cmpPosition || !cmpOtherPosition || !cmpOtherFormation)
+		let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
+		let cmpOtherPosition = Engine.QueryInterface(this.twinFormations[i], IID_Position);
+		let cmpOtherFormation = Engine.QueryInterface(this.twinFormations[i], IID_Formation);
+		if (!cmpPosition || !cmpOtherPosition || !cmpOtherFormation ||
+		     !cmpPosition.IsInWorld() || !cmpOtherPosition.IsInWorld())
 			continue;
 
-		var thisPosition = cmpPosition.GetPosition2D();
-		var otherPosition = cmpOtherPosition.GetPosition2D();
-		var dx = thisPosition.x - otherPosition.x;
-		var dy = thisPosition.y - otherPosition.y;
-		var dist = Math.sqrt(dx * dx + dy * dy);
+		let thisPosition = cmpPosition.GetPosition2D();
+		let otherPosition = cmpOtherPosition.GetPosition2D();
 
-		var thisSize = this.GetSize();
-		var otherSize = cmpOtherFormation.GetSize();
-		var minDist = Math.max(thisSize.width / 2, thisSize.depth / 2) +
+		let dx = thisPosition.x - otherPosition.x;
+		let dy = thisPosition.y - otherPosition.y;
+		let dist = Math.sqrt(dx * dx + dy * dy);
+
+		let thisSize = this.GetSize();
+		let otherSize = cmpOtherFormation.GetSize();
+		let minDist = Math.max(thisSize.width / 2, thisSize.depth / 2) +
 			Math.max(otherSize.width / 2, otherSize.depth / 2) +
 			this.formationSeparation;
 
@@ -937,12 +940,12 @@ Formation.prototype.ShapeUpdate = function()
 		cmpOtherFormation.RemoveMembers(otherMembers);
 		this.AddMembers(otherMembers);
 		Engine.DestroyEntity(this.twinFormations[i]);
-		this.twinFormations.splice(i,1);
+		this.twinFormations.splice(i, 1);
 	}
 	// Switch between column and box if necessary
-	var cmpUnitAI = Engine.QueryInterface(this.entity, IID_UnitAI);
-	var walkingDistance = cmpUnitAI.ComputeWalkingDistance();
-	var columnar = walkingDistance > g_ColumnDistanceThreshold;
+	let cmpUnitAI = Engine.QueryInterface(this.entity, IID_UnitAI);
+	let walkingDistance = cmpUnitAI.ComputeWalkingDistance();
+	let columnar = walkingDistance > g_ColumnDistanceThreshold;
 	if (columnar != this.columnar)
 	{
 		this.offsets = undefined;

binaries/data/mods/public/simulation/components/UnitAI.js

@@ -749,6 +749,9 @@ UnitAI.prototype.UnitFsmSpec = {
 				this.CallMemberFunction("Stop", [false]);
 			this.StopMoving();
 			this.FinishOrder();
+			// Don't move the members back into formation,
+			// as the formation then resets and it looks odd when walk-stopping.
+			// TODO: this should be improved in the formation reshaping code.
 		},
 
 		"Order.Attack": function(msg) {
@@ -937,8 +940,6 @@ UnitAI.prototype.UnitFsmSpec = {
 
 		"IDLE": {
 			"enter": function(msg) {
-				var cmpFormation = Engine.QueryInterface(this.entity, IID_Formation);
-				cmpFormation.SetRearrange(false);
 				return false;
 			},
 		},
@@ -1214,12 +1215,32 @@ UnitAI.prototype.UnitFsmSpec = {
 			},
 		},
 
+		// Wait for individual members to finish
 		"MEMBER": {
-			// Wait for individual members to finish
+			"OrderTargetRenamed": function(msg) {
+				// In general, don't react - we don't want to send spurious messages to members.
+				// This looks odd for hunting hwoever because we wait for all
+				// entities to ahve clumped around the dead resource before proceeding
+				// so explicitly handle this case.
+				if (this.order && this.order.data && this.order.data.hunting &&
+				     this.order.data.target == msg.data.newentity &&
+				     this.orderQueue.length > 1)
+					this.FinishOrder();
+			},
+
 			"enter": function(msg) {
-				var cmpFormation = Engine.QueryInterface(this.entity, IID_Formation);
-				cmpFormation.SetRearrange(false);
-				this.StopMoving();
+				// Don't rearrange the formation, as that forces all units to stop
+				// what they're doing.
+				let cmpFormation = Engine.QueryInterface(this.entity, IID_Formation);
+				if (cmpFormation)
+					cmpFormation.SetRearrange(false);
+				// While waiting on members, the formation is more like
+				// a group of unit and does not have a well-defined position,
+				// so move the controller out of the world to enforce that.
+				let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
+				if (cmpPosition)
+					cmpPosition.MoveOutOfWorld();
+
 				this.StartTimer(1000, 1000);
 				return false;
 			},
@@ -1238,13 +1259,23 @@ UnitAI.prototype.UnitFsmSpec = {
 						this.FindWalkAndFightTargets();
 					return;
 				}
-				return false;
+				return;
 			},
 
 			"leave": function(msg) {
 				this.StopTimer();
+				// Reform entirely as members might be all over the place now.
 				let cmpFormation = Engine.QueryInterface(this.entity, IID_Formation);
-				cmpFormation.MoveToMembersCenter();
+				if (cmpFormation)
+					cmpFormation.MoveMembersIntoFormation(true);
+
+				// Update the held position so entities respond to orders.
+				let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
+				if (cmpPosition && cmpPosition.IsInWorld())
+				{
+					let pos = cmpPosition.GetPosition2D();
+					this.CallMemberFunction("SetHeldPosition", [pos.x, pos.y]);
+				}
 			},
 		},
 	},
@@ -1318,10 +1349,7 @@ UnitAI.prototype.UnitFsmSpec = {
 
 		"WALKING": {
 			"enter": function() {
-				this.formationOffset = { "x": this.order.data.x, "z": this.order.data.z };
 				let cmpUnitMotion = Engine.QueryInterface(this.entity, IID_UnitMotion);
-				// Prevent unit to turn when stopmoving is called.
-				cmpUnitMotion.SetFacePointAfterMove(false);
 				cmpUnitMotion.MoveToFormationOffset(this.order.data.target, this.order.data.x, this.order.data.z);
 				if (this.order.data.offsetsChanged)
 				{
@@ -1339,28 +1367,30 @@ UnitAI.prototype.UnitFsmSpec = {
 			},
 
 			"leave": function() {
+				// Don't use the logic from unitMotion, as SetInPosition
+				// has already given us a custom rotation
+				// (or we failed to move and thus don't care.)
+				this.SetFacePointAfterMove(false);
 				this.StopMoving();
+				// Reset default behaviour (TODO: actually get the previuos behaviour).
 				this.SetFacePointAfterMove(true);
 			},
 
 			// Occurs when the unit has reached its destination and the controller
 			// is done moving. The controller is notified.
 			"MovementUpdate": function(msg) {
-				// We can only finish this order if the move was really completed.
-				let cmpPosition = Engine.QueryInterface(this.formationController, IID_Position);
-				let atDestination = cmpPosition && cmpPosition.IsInWorld();
-				if (!atDestination && cmpPosition)
+				// We're supposed to be walking in formation,
+				// but the controller has no position -> abort.
+				let cmpControllerPosition = Engine.QueryInterface(this.formationController, IID_Position);
+				if (!cmpControllerPosition || !cmpControllerPosition.IsInWorld())
 				{
-					let pos = cmpPosition.GetPosition2D();
-					atDestination = this.CheckPointRangeExplicit(pos.X + this.order.data.x, pos.Y + this.order.data.z, 0, 1);
+					this.FinishOrder();
+					return;
 				}
-				if (!atDestination && !msg.likelyFailure)
+				if (!msg.likelyFailure && !msg.likelySuccess)
 					return;
 
-				if (this.FinishOrder())
-					return;
-
-				delete this.formationOffset;
+				this.FinishOrder();
 			},
 		},
 
@@ -1526,18 +1556,16 @@ UnitAI.prototype.UnitFsmSpec = {
 				if (this.FindNewTargets())
 					return;
 
-				if (this.formationOffset && this.formationController)
-				{
-					this.PushOrder("FormationWalk", {
-						"target": this.formationController,
-						"x": this.formationOffset.x,
-						"z": this.formationOffset.z
-					});
-					return;
-				}
-
 				if (!this.isIdle)
 				{
+					// Move back to the held position if we drifted away.
+					// (only if not a formation member).
+					if (!this.IsFormationMember() &&
+					     this.GetStance().respondHoldGround && this.heldPosition &&
+					     !this.CheckPointRangeExplicit(this.heldPosition.x, this.heldPosition.z, 0, 10) &&
+					     this.WalkToHeldPosition())
+						return;
+
 					this.isIdle = true;
 					Engine.PostMessage(this.entity, MT_UnitIdleChanged, { "idle": this.isIdle });
 				}
@@ -3716,7 +3744,8 @@ UnitAI.prototype.FinishOrder = function()
 	this.order = this.orderQueue[0];
 
 	let cmpPosition = Engine.QueryInterface(this.entity, IID_Position);
-	if (this.orderQueue.length && (this.IsGarrisoned() || cmpPosition && cmpPosition.IsInWorld()))
+	if (this.orderQueue.length && (this.IsGarrisoned() || this.IsFormationController() ||
+	        cmpPosition && cmpPosition.IsInWorld()))
 	{
 		let ret = this.UnitFsm.ProcessMessage(this,
 			{ "type": "Order."+this.order.type, "data": this.order.data }

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

@@ -148,7 +148,8 @@ function TestFormationExiting(mode)
 	});
 
 	AddMock(SYSTEM_ENTITY, IID_ObstructionManager, {
-		"IsInTargetRange": (ent, target, min, max, opposite) => true
+		"IsInTargetRange": () => true,
+		"IsInPointRange": () => true
 	});
 
 	var unitAI = ConstructComponent(unit, "UnitAI", { "FormationController": "false", "DefaultStance": "aggressive" });
@@ -221,6 +222,7 @@ function TestFormationExiting(mode)
 		GetPosition2D: function() { return new Vector2D(this.x, this.z); },
 		GetRotation: function() { return { "y": 0 }; },
 		IsInWorld: function() { return true; },
+		MoveOutOfWorld: () => {}
 	});
 
 	AddMock(controller, IID_UnitMotion, {
@@ -366,12 +368,13 @@ function TestMoveIntoFormationWhileAttacking()
 	var controllerAI = ConstructComponent(controller, "UnitAI", { "FormationController": "true", "DefaultStance": "aggressive" });
 
 	AddMock(controller, IID_Position, {
-		GetTurretParent: function() { return INVALID_ENTITY; },
-		JumpTo: function(x, z) { this.x = x; this.z = z; },
-		GetPosition: function() { return new Vector3D(this.x, 0, this.z); },
-		GetPosition2D: function() { return new Vector2D(this.x, this.z); },
-		GetRotation: function() { return { "y": 0 }; },
-		IsInWorld: function() { return true; },
+		"GetTurretParent": () => INVALID_ENTITY,
+		"JumpTo": function(x, z) { this.x = x; this.z = z; },
+		"GetPosition": function(){ return new Vector3D(this.x, 0, this.z); },
+		"GetPosition2D": function(){ return new Vector2D(this.x, this.z); },
+		"GetRotation": () => ({ "y": 0 }),
+		"IsInWorld": () => true,
+		"MoveOutOfWorld": () => {},
 	});
 
 	AddMock(controller, IID_UnitMotion, {