Base AttackAircraft on AttackFollow and get rid of SequenceActivities.

OpenRA.Mods.Cnc/Traits/Attack/AttackTDGunboatTurreted.cs

@@ -59,10 +59,10 @@ namespace OpenRA.Mods.Cnc.Traits
 				{
 					// Check that AttackTDGunboatTurreted hasn't cancelled the target by modifying attack.Target
 					// Having both this and AttackTDGunboatTurreted modify that field is a horrible hack.
-					if (hasTicked && attack.requestedTarget.Type == TargetType.Invalid)
+					if (hasTicked && attack.RequestedTarget.Type == TargetType.Invalid)
 						return NextActivity;
 
-					attack.requestedTarget = target;
+					attack.RequestedTarget = target;
 					hasTicked = true;
 				}
 

OpenRA.Mods.Common/Activities/Air/FlyAttack.cs

@@ -22,12 +22,13 @@ namespace OpenRA.Mods.Common.Activities
 		readonly Aircraft aircraft;
 		readonly AttackAircraft attackAircraft;
 		readonly Rearmable rearmable;
+		readonly int ticksUntilTurn;
+
 		Target target;
 		Target lastVisibleTarget;
 		WDist lastVisibleMaximumRange;
 		bool useLastVisibleTarget;
-
+		bool hasTicked;
-		int ticksUntilTurn;
 
 		public FlyAttack(Actor self, Target target)
 		{
@@ -49,15 +50,38 @@ namespace OpenRA.Mods.Common.Activities
 
 		public override Activity Tick(Actor self)
 		{
+			if (ChildActivity != null)
+			{
+				ChildActivity = ActivityUtils.RunActivity(self, ChildActivity);
+				if (ChildActivity != null)
+					return this;
+			}
+
 			// Refuse to take off if it would land immediately again.
 			if (aircraft.ForceLanding)
 				Cancel(self);
 
 			if (IsCanceling)
+			{
+				// Cancel the requested target, but keep firing on it while in range
+				attackAircraft.OpportunityTarget = attackAircraft.RequestedTarget;
+				attackAircraft.RequestedTarget = Target.Invalid;
+				return NextActivity;
+			}
+
+			// Check that AttackFollow hasn't cancelled the target by modifying attack.Target
+			// Having both this and AttackFollow modify that field is a horrible hack.
+			if (hasTicked && attackAircraft.RequestedTarget.Type == TargetType.Invalid)
 				return NextActivity;
 
+			if (attackAircraft.IsTraitPaused)
+				return this;
+
 			bool targetIsHiddenActor;
-			target = target.Recalculate(self.Owner, out targetIsHiddenActor);
+			attackAircraft.RequestedTarget = target = target.Recalculate(self.Owner, out targetIsHiddenActor);
+			attackAircraft.RequestedTargetLastTick = self.World.WorldTick;
+			hasTicked = true;
+
 			if (!targetIsHiddenActor && target.Type == TargetType.Actor)
 			{
 				lastVisibleTarget = Target.FromTargetPositions(target);
@@ -73,11 +97,17 @@ namespace OpenRA.Mods.Common.Activities
 
 			// Target is hidden or dead, and we don't have a fallback position to move towards
 			if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self))
+			{
+				attackAircraft.RequestedTarget = Target.Invalid;
 				return NextActivity;
+			}
 
 			// If all valid weapons have depleted their ammo and Rearmable trait exists, return to RearmActor to reload and then resume the activity
 			if (rearmable != null && !useLastVisibleTarget && attackAircraft.Armaments.All(x => x.IsTraitPaused || !x.Weapon.IsValidAgainst(target, self.World, self)))
-				return ActivityUtils.SequenceActivities(self, new ReturnToBase(self, aircraft.Info.AbortOnResupply), this);
+			{
+				QueueChild(self, new ReturnToBase(self, aircraft.Info.AbortOnResupply), true);
+				return this;
+			}
 
 			var pos = self.CenterPosition;
 			var checkTarget = useLastVisibleTarget ? lastVisibleTarget : target;
@@ -87,37 +117,25 @@ namespace OpenRA.Mods.Common.Activities
 			{
 				// We've reached the assumed position but it is not there - give up
 				if (checkTarget.IsInRange(pos, lastVisibleMaximumRange))
+				{
+					attackAircraft.RequestedTarget = Target.Invalid;
 					return NextActivity;
+				}
 
 				// Fly towards the last known position
-				return ActivityUtils.SequenceActivities(self,
+				QueueChild(self, new Fly(self, target, WDist.Zero, lastVisibleMaximumRange, checkTarget.CenterPosition, Color.Red), true);
-					new Fly(self, target, WDist.Zero, lastVisibleMaximumRange, checkTarget.CenterPosition, Color.Red),
+				return this;
-					this);
 			}
 
-			// If we reach here the target is guaranteed to be both visible and alive, so use target instead of checkTarget.
+			if (self.World.Map.DistanceAboveTerrain(self.CenterPosition).Length < aircraft.Info.MinAirborneAltitude)
-			// The target may not be in range, but try attacking anyway...
+				QueueChild(self, new TakeOff(self), true);
-			attackAircraft.DoAttack(self, target);
 
-			if (ChildActivity == null)
+			// TODO: This should fire each weapon at its maximum range
-			{
+			if (attackAircraft != null && target.IsInRange(self.CenterPosition, attackAircraft.GetMinimumRange()))
-				// TODO: This should fire each weapon at its maximum range
+				QueueChild(self, new FlyTimed(ticksUntilTurn, self), true);
-				if (attackAircraft != null && target.IsInRange(self.CenterPosition, attackAircraft.GetMinimumRange()))
-					ChildActivity = ActivityUtils.SequenceActivities(self,
-						new FlyTimed(ticksUntilTurn, self),
-						new Fly(self, target, checkTarget.CenterPosition, Color.Red),
-						new FlyTimed(ticksUntilTurn, self));
-				else
-					ChildActivity = ActivityUtils.SequenceActivities(self,
-						new Fly(self, target, checkTarget.CenterPosition, Color.Red),
-						new FlyTimed(ticksUntilTurn, self));
 
-				// HACK: This needs to be done in this round-about way because TakeOff doesn't behave as expected when it doesn't have a NextActivity.
+			QueueChild(self, new Fly(self, target, checkTarget.CenterPosition, Color.Red), true);
-				if (self.World.Map.DistanceAboveTerrain(self.CenterPosition).Length < aircraft.Info.MinAirborneAltitude)
+			QueueChild(self, new FlyTimed(ticksUntilTurn, self));
-					ChildActivity = ActivityUtils.SequenceActivities(self, new TakeOff(self), ChildActivity);
-			}
-
-			ActivityUtils.RunActivity(self, ChildActivity);
 
 			return this;
 		}

OpenRA.Mods.Common/Activities/Air/HeliAttack.cs

@@ -27,6 +27,7 @@ namespace OpenRA.Mods.Common.Activities
 		Target lastVisibleTarget;
 		WDist lastVisibleMaximumRange;
 		bool useLastVisibleTarget;
+		bool hasTicked;
 
 		public HeliAttack(Actor self, Target target)
 		{
@@ -59,10 +60,26 @@ namespace OpenRA.Mods.Common.Activities
 				Cancel(self);
 
 			if (IsCanceling)
+			{
+				// Cancel the requested target, but keep firing on it while in range
+				attackAircraft.OpportunityTarget = attackAircraft.RequestedTarget;
+				attackAircraft.RequestedTarget = Target.Invalid;
+				return NextActivity;
+			}
+
+			// Check that AttackFollow hasn't cancelled the target by modifying attack.Target
+			// Having both this and AttackFollow modify that field is a horrible hack.
+			if (hasTicked && attackAircraft.RequestedTarget.Type == TargetType.Invalid)
 				return NextActivity;
 
+			if (attackAircraft.IsTraitPaused)
+				return this;
+
 			bool targetIsHiddenActor;
-			target = target.Recalculate(self.Owner, out targetIsHiddenActor);
+			attackAircraft.RequestedTarget = target = target.Recalculate(self.Owner, out targetIsHiddenActor);
+			attackAircraft.RequestedTargetLastTick = self.World.WorldTick;
+			hasTicked = true;
+
 			if (!targetIsHiddenActor && target.Type == TargetType.Actor)
 			{
 				lastVisibleTarget = Target.FromTargetPositions(target);
@@ -78,7 +95,10 @@ namespace OpenRA.Mods.Common.Activities
 
 			// Target is hidden or dead, and we don't have a fallback position to move towards
 			if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self))
+			{
+				attackAircraft.RequestedTarget = Target.Invalid;
 				return NextActivity;
+			}
 
 			// If all valid weapons have depleted their ammo and Rearmable trait exists, return to RearmActor to reload and then resume the activity
 			if (rearmable != null && !useLastVisibleTarget && attackAircraft.Armaments.All(x => x.IsTraitPaused || !x.Weapon.IsValidAgainst(target, self.World, self)))
@@ -102,7 +122,10 @@ namespace OpenRA.Mods.Common.Activities
 			{
 				// We've reached the assumed position but it is not there - give up
 				if (checkTarget.IsInRange(pos, lastVisibleMaximumRange))
+				{
+					attackAircraft.RequestedTarget = Target.Invalid;
 					return NextActivity;
+				}
 
 				// Fly towards the last known position
 				aircraft.SetPosition(self, aircraft.CenterPosition + aircraft.FlyStep(desiredFacing));
@@ -125,8 +148,6 @@ namespace OpenRA.Mods.Common.Activities
 				aircraft.SetPosition(self, aircraft.CenterPosition + aircraft.FlyStep(-facing));
 			}
 
-			attackAircraft.DoAttack(self, target);
-
 			return this;
 		}
 	}

OpenRA.Mods.Common/Traits/Air/AttackAircraft.cs

@@ -15,15 +15,26 @@ using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Traits
 {
-	public class AttackAircraftInfo : AttackFrontalInfo, Requires<AircraftInfo>
+	public class AttackAircraftInfo : AttackFollowInfo, Requires<AircraftInfo>
 	{
 		[Desc("Delay, in game ticks, before non-hovering aircraft turns to attack.")]
 		public readonly int AttackTurnDelay = 50;
 
+		[Desc("Tolerance for attack angle. Range [0, 128], 128 covers 360 degrees.")]
+		public readonly int FacingTolerance = 0;
+
+		public override void RulesetLoaded(Ruleset rules, ActorInfo ai)
+		{
+			base.RulesetLoaded(rules, ai);
+
+			if (FacingTolerance < 0 || FacingTolerance > 128)
+				throw new YamlException("Facing tolerance must be in range of [0, 128], 128 covers 360 degrees.");
+		}
+
 		public override object Create(ActorInitializer init) { return new AttackAircraft(init.Self, this); }
 	}
 
-	public class AttackAircraft : AttackFrontal
+	public class AttackAircraft : AttackFollow
 	{
 		public readonly AttackAircraftInfo AttackAircraftInfo;
 		readonly AircraftInfo aircraftInfo;
@@ -46,9 +57,21 @@ namespace OpenRA.Mods.Common.Traits
 		protected override bool CanAttack(Actor self, Target target)
 		{
 			// Don't fire while landed or when outside the map.
-			return base.CanAttack(self, target)
+			if (self.World.Map.DistanceAboveTerrain(self.CenterPosition).Length < aircraftInfo.MinAirborneAltitude
-				&& self.World.Map.DistanceAboveTerrain(self.CenterPosition).Length >= aircraftInfo.MinAirborneAltitude
+				|| !self.World.Map.Contains(self.Location))
-				&& self.World.Map.Contains(self.Location);
+				return false;
+
+			if (!base.CanAttack(self, target))
+				return false;
+
+			var pos = self.CenterPosition;
+			var targetedPosition = GetTargetPosition(pos, target);
+			var delta = targetedPosition - pos;
+
+			if (delta.HorizontalLengthSquared == 0)
+				return true;
+
+			return Util.FacingWithinTolerance(facing.Facing, delta.Yaw.Facing, AttackAircraftInfo.FacingTolerance);
 		}
 	}
 }

OpenRA.Mods.Common/Traits/Attack/AttackFollow.cs

@@ -28,10 +28,10 @@ namespace OpenRA.Mods.Common.Traits
 
 	public class AttackFollow : AttackBase, INotifyOwnerChanged
 	{
-		protected Target requestedTarget;
+		public Target RequestedTarget;
 		protected bool requestedForceAttack;
-		protected int requestedTargetLastTick;
+		public int RequestedTargetLastTick;
-		protected Target opportunityTarget;
+		public Target OpportunityTarget;
 		protected bool opportunityForceAttack;
 		Mobile mobile;
 		AutoTarget autoTarget;
@@ -66,45 +66,45 @@ namespace OpenRA.Mods.Common.Traits
 		protected override void Tick(Actor self)
 		{
 			if (IsTraitDisabled)
-				requestedTarget = opportunityTarget = Target.Invalid;
+				RequestedTarget = OpportunityTarget = Target.Invalid;
 
-			if (requestedTargetLastTick != self.World.WorldTick)
+			if (RequestedTargetLastTick != self.World.WorldTick)
 			{
 				// Activities tick before traits, so if we are here it means the activity didn't run
 				// (either queued next or already cancelled) and we need to recalculate the target ourself
 				bool targetIsHiddenActor;
-				requestedTarget = requestedTarget.Recalculate(self.Owner, out targetIsHiddenActor);
+				RequestedTarget = RequestedTarget.Recalculate(self.Owner, out targetIsHiddenActor);
 			}
 
 			// Can't fire on anything
 			if (mobile != null && !mobile.CanInteractWithGroundLayer(self))
 				return;
 
-			if (requestedTarget.Type != TargetType.Invalid)
+			if (RequestedTarget.Type != TargetType.Invalid)
 			{
-				IsAiming = CanAimAtTarget(self, requestedTarget, requestedForceAttack);
+				IsAiming = CanAimAtTarget(self, RequestedTarget, requestedForceAttack);
 				if (IsAiming)
-					DoAttack(self, requestedTarget);
+					DoAttack(self, RequestedTarget);
 			}
 			else
 			{
 				IsAiming = false;
 
-				if (opportunityTarget.Type != TargetType.Invalid)
+				if (OpportunityTarget.Type != TargetType.Invalid)
-					IsAiming = CanAimAtTarget(self, opportunityTarget, opportunityForceAttack);
+					IsAiming = CanAimAtTarget(self, OpportunityTarget, opportunityForceAttack);
 
 				if (!IsAiming && ((AttackFollowInfo)Info).OpportunityFire && autoTarget != null &&
 				    !autoTarget.IsTraitDisabled && autoTarget.Stance >= UnitStance.Defend)
 				{
-					opportunityTarget = autoTarget.ScanForTarget(self, false);
+					OpportunityTarget = autoTarget.ScanForTarget(self, false);
 					opportunityForceAttack = false;
 
-					if (opportunityTarget.Type != TargetType.Invalid)
+					if (OpportunityTarget.Type != TargetType.Invalid)
-						IsAiming = CanAimAtTarget(self, opportunityTarget, opportunityForceAttack);
+						IsAiming = CanAimAtTarget(self, OpportunityTarget, opportunityForceAttack);
 				}
 
 				if (IsAiming)
-					DoAttack(self, opportunityTarget);
+					DoAttack(self, OpportunityTarget);
 			}
 
 			base.Tick(self);
@@ -122,21 +122,21 @@ namespace OpenRA.Mods.Common.Traits
 			// the last order (usually a move) and set the target immediately
 			if (!queued)
 			{
-				requestedTarget = target;
+				RequestedTarget = target;
 				requestedForceAttack = forceAttack;
-				requestedTargetLastTick = self.World.WorldTick;
+				RequestedTargetLastTick = self.World.WorldTick;
 			}
 		}
 
 		public override void OnStopOrder(Actor self)
 		{
-			requestedTarget = opportunityTarget = Target.Invalid;
+			RequestedTarget = OpportunityTarget = Target.Invalid;
 			base.OnStopOrder(self);
 		}
 
 		void INotifyOwnerChanged.OnOwnerChanged(Actor self, Player oldOwner, Player newOwner)
 		{
-			requestedTarget = opportunityTarget = Target.Invalid;
+			RequestedTarget = OpportunityTarget = Target.Invalid;
 		}
 
 		class AttackActivity : Activity
@@ -186,15 +186,15 @@ namespace OpenRA.Mods.Common.Traits
 				if (IsCanceling)
 				{
 					// Cancel the requested target, but keep firing on it while in range
-					attack.opportunityTarget = attack.requestedTarget;
+					attack.OpportunityTarget = attack.RequestedTarget;
 					attack.opportunityForceAttack = attack.requestedForceAttack;
-					attack.requestedTarget = Target.Invalid;
+					attack.RequestedTarget = Target.Invalid;
 					return NextActivity;
 				}
 
 				// Check that AttackFollow hasn't cancelled the target by modifying attack.Target
 				// Having both this and AttackFollow modify that field is a horrible hack.
-				if (hasTicked && attack.requestedTarget.Type == TargetType.Invalid)
+				if (hasTicked && attack.RequestedTarget.Type == TargetType.Invalid)
 					return NextActivity;
 
 				if (attack.IsTraitPaused)
@@ -202,8 +202,8 @@ namespace OpenRA.Mods.Common.Traits
 
 				bool targetIsHiddenActor;
 				attack.requestedForceAttack = forceAttack;
-				attack.requestedTarget = target = target.Recalculate(self.Owner, out targetIsHiddenActor);
+				attack.RequestedTarget = target = target.Recalculate(self.Owner, out targetIsHiddenActor);
-				attack.requestedTargetLastTick = self.World.WorldTick;
+				attack.RequestedTargetLastTick = self.World.WorldTick;
 				hasTicked = true;
 
 				if (!targetIsHiddenActor && target.Type == TargetType.Actor)
@@ -243,7 +243,7 @@ namespace OpenRA.Mods.Common.Traits
 				// Either we are in range and can see the target, or we've lost track of it and should give up
 				if (wasMovingWithinRange && targetIsHiddenActor)
 				{
-					attack.requestedTarget = Target.Invalid;
+					attack.RequestedTarget = Target.Invalid;
 					return NextActivity;
 				}
 
@@ -254,7 +254,7 @@ namespace OpenRA.Mods.Common.Traits
 				// Target is hidden or dead, and we don't have a fallback position to move towards
 				if (useLastVisibleTarget && !lastVisibleTarget.IsValidFor(self))
 				{
-					attack.requestedTarget = Target.Invalid;
+					attack.RequestedTarget = Target.Invalid;
 					return NextActivity;
 				}
 
@@ -267,7 +267,7 @@ namespace OpenRA.Mods.Common.Traits
 				{
 					if (useLastVisibleTarget)
 					{
-						attack.requestedTarget = Target.Invalid;
+						attack.RequestedTarget = Target.Invalid;
 						return NextActivity;
 					}
 
@@ -277,7 +277,7 @@ namespace OpenRA.Mods.Common.Traits
 				// We can't move into range, so give up
 				if (move == null || maxRange == WDist.Zero || maxRange < minRange)
 				{
-					attack.requestedTarget = Target.Invalid;
+					attack.RequestedTarget = Target.Invalid;
 					return NextActivity;
 				}