Merge repair and rearm anim traits into WithResupplyAnimation

This is the safest approach to avoid conflicts/visual glitches when the host is responsible for both resupply types.
The new trait will simply play a looping animation as long as the actor is resupplying in any form.

OpenRA.Mods.Cnc/Activities/LayMines.cs

@@ -53,11 +53,11 @@ namespace OpenRA.Mods.Cnc.Activities
 				if (rearmTarget == null)
 					return new Wait(20);
 
-				// Add a CloseEnough range of 512 to the Repair activity in order to ensure that we're at the host actor
+				// Add a CloseEnough range of 512 to the Rearm/Repair activities in order to ensure that we're at the host actor
 				return ActivityUtils.SequenceActivities(
 					new MoveAdjacentTo(self, Target.FromActor(rearmTarget)),
 					movement.MoveTo(self.World.Map.CellContaining(rearmTarget.CenterPosition), rearmTarget),
-					new Rearm(self),
+					new Rearm(self, rearmTarget, new WDist(512)),
 					new Repair(self, rearmTarget, new WDist(512)),
 					this);
 			}

OpenRA.Mods.Common/Activities/Rearm.cs

@@ -13,16 +13,20 @@ using System.Collections.Generic;
 using System.Linq;
 using OpenRA.Activities;
 using OpenRA.Mods.Common.Traits;
-using OpenRA.Mods.Common.Traits.Render;
+using OpenRA.Traits;
 
 namespace OpenRA.Mods.Common.Activities
 {
 	public class Rearm : Activity
 	{
+		readonly Target host;
+		readonly WDist closeEnough;
 		readonly AmmoPool[] ammoPools;
 
-		public Rearm(Actor self)
+		public Rearm(Actor self, Actor host, WDist closeEnough)
 		{
+			this.host = Target.FromActor(host);
+			this.closeEnough = closeEnough;
 			ammoPools = self.TraitsImplementing<AmmoPool>().Where(p => !p.AutoReloads).ToArray();
 		}
 
@@ -33,6 +37,28 @@ namespace OpenRA.Mods.Common.Activities
 			// HACK: this really shouldn't be managed from here
 			foreach (var pool in ammoPools)
 				pool.RemainingTicks = pool.Info.ReloadDelay;
+
+			if (host.Type == TargetType.Invalid)
+				return;
+
+			foreach (var notify in host.Actor.TraitsImplementing<INotifyRearm>())
+				notify.RearmingStarted(host.Actor, self);
+		}
+
+		protected override void OnLastRun(Actor self)
+		{
+			if (host.Type == TargetType.Invalid)
+				return;
+
+			foreach (var notify in host.Actor.TraitsImplementing<INotifyRearm>())
+				notify.RearmingFinished(host.Actor, self);
+		}
+
+		protected override void OnActorDispose(Actor self)
+		{
+			// If the actor died (or will be disposed directly) this tick, Activity.TickOuter won't be ticked again,
+			// so we need to run OnLastRun directly (otherwise it would be skipped completely).
+			OnLastRun(self);
 		}
 
 		public override Activity Tick(Actor self)
@@ -40,11 +66,10 @@ namespace OpenRA.Mods.Common.Activities
 			if (IsCanceled)
 				return NextActivity;
 
-			// HACK: check if we are on the helipad/airfield/etc.
+			if (host.Type == TargetType.Invalid)
-			var hostBuilding = self.World.ActorMap.GetActorsAt(self.Location)
+				return NextActivity;
-				.FirstOrDefault(a => a.Info.HasTraitInfo<BuildingInfo>());
 
-			if (hostBuilding == null || !hostBuilding.IsInWorld)
+			if (closeEnough.LengthSquared > 0 && !host.IsInRange(self.CenterPosition, closeEnough))
 				return NextActivity;
 
 			var complete = true;
@@ -52,7 +77,7 @@ namespace OpenRA.Mods.Common.Activities
 			{
 				if (!pool.FullAmmo())
 				{
-					Reload(self, hostBuilding, pool);
+					Reload(self, host.Actor, pool);
 					complete = false;
 				}
 			}
@@ -60,12 +85,12 @@ namespace OpenRA.Mods.Common.Activities
 			return complete ? NextActivity : this;
 		}
 
-		void Reload(Actor self, Actor hostBuilding, AmmoPool ammoPool)
+		void Reload(Actor self, Actor host, AmmoPool ammoPool)
 		{
 			if (--ammoPool.RemainingTicks <= 0)
 			{
-				foreach (var host in hostBuilding.TraitsImplementing<INotifyRearm>())
+				foreach (var notify in host.TraitsImplementing<INotifyRearm>())
-					host.Rearming(hostBuilding, self);
+					notify.Rearming(host, self);
 
 				ammoPool.RemainingTicks = ammoPool.Info.ReloadDelay;
 				if (!string.IsNullOrEmpty(ammoPool.Info.RearmSound))

OpenRA.Mods.Common/Activities/Repair.cs

@@ -137,5 +137,12 @@ namespace OpenRA.Mods.Common.Activities
 			foreach (var depot in host.Actor.TraitsImplementing<INotifyRepair>())
 				depot.AfterRepair(host.Actor, self);
 		}
+
+		protected override void OnActorDispose(Actor self)
+		{
+			// If the actor died (or will be disposed directly) this tick, Activity.TickOuter won't be ticked again,
+			// so we need to run OnLastRun directly (otherwise it would be skipped completely).
+			OnLastRun(self);
+		}
 	}
 }

OpenRA.Mods.Common/OpenRA.Mods.Common.csproj

@@ -475,8 +475,7 @@
     <Compile Include="Traits\Render\WithNukeLaunchOverlay.cs" />
     <Compile Include="Traits\Render\WithParachute.cs" />
     <Compile Include="Traits\Render\WithRangeCircle.cs" />
-    <Compile Include="Traits\Render\WithRearmAnimation.cs" />
+    <Compile Include="Traits\Render\WithResupplyAnimation.cs" />
-    <Compile Include="Traits\Render\WithRepairAnimation.cs" />
     <Compile Include="Traits\Render\WithRepairOverlay.cs" />
     <Compile Include="Traits\Render\WithResources.cs" />
     <Compile Include="Traits\Render\WithShadow.cs" />
@@ -912,6 +911,7 @@
     <Compile Include="UpdateRules\Rules\20180307\RemoveCanUndeployFromGrantConditionOnDeploy.cs" />
     <Compile Include="UpdateRules\Rules\20180923\DefineNotificationDefaults.cs" />
     <Compile Include="UpdateRules\Rules\20180923\RenameEditorTilesetFilter.cs" />
+    <Compile Include="UpdateRules\Rules\20180923\MergeRearmAndRepairAnimation.cs" />
     <Compile Include="Traits\Player\PlayerResources.cs" />
     <Compile Include="UtilityCommands\DumpSequenceSheetsCommand.cs" />
     <Compile Include="Traits\Render\WithBuildingRepairDecoration.cs" />

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

@@ -489,7 +489,7 @@ namespace OpenRA.Mods.Common.Traits
 		{
 			var name = a.Info.Name;
 			if (Info.RearmBuildings.Contains(name))
-				yield return new Rearm(self);
+				yield return new Rearm(self, a, WDist.Zero);
 
 			// The ResupplyAircraft activity guarantees that we're on the helipad
 			if (Info.RepairBuildings.Contains(name))

OpenRA.Mods.Common/Traits/Render/WithRearmAnimation.cs

@@ -1,58 +0,0 @@
-#region Copyright & License Information
-/*
- * Copyright 2007-2018 The OpenRA Developers (see AUTHORS)
- * This file is part of OpenRA, which is free software. It is made
- * available to you under the terms of the GNU General Public License
- * as published by the Free Software Foundation, either version 3 of
- * the License, or (at your option) any later version. For more
- * information, see COPYING.
- */
-#endregion
-
-using System.Linq;
-using OpenRA.Traits;
-
-namespace OpenRA.Mods.Common.Traits.Render
-{
-	[Desc("Replaces the building animation when it rearms a unit.")]
-	public class WithRearmAnimationInfo : ConditionalTraitInfo, Requires<WithSpriteBodyInfo>
-	{
-		[Desc("Sequence name to use")]
-		[SequenceReference] public readonly string Sequence = "active";
-
-		[Desc("Which sprite body to play the animation on.")]
-		public readonly string Body = "body";
-
-		public override object Create(ActorInitializer init) { return new WithRearmAnimation(init.Self, this); }
-	}
-
-	public class WithRearmAnimation : ConditionalTrait<WithRearmAnimationInfo>, INotifyRearm, INotifyBuildComplete, INotifySold
-	{
-		readonly WithSpriteBody spriteBody;
-		bool buildComplete;
-
-		public WithRearmAnimation(Actor self, WithRearmAnimationInfo info)
-			: base(info)
-		{
-			spriteBody = self.TraitsImplementing<WithSpriteBody>().Single(w => w.Info.Name == Info.Body);
-		}
-
-		void INotifyRearm.Rearming(Actor self, Actor target)
-		{
-			if (buildComplete && !IsTraitDisabled)
-				spriteBody.PlayCustomAnimation(self, Info.Sequence);
-		}
-
-		void INotifyBuildComplete.BuildingComplete(Actor self)
-		{
-			buildComplete = true;
-		}
-
-		void INotifySold.Selling(Actor self)
-		{
-			buildComplete = false;
-		}
-
-		void INotifySold.Sold(Actor self) { }
-	}
-}

OpenRA.Mods.Common/Traits/Render/WithRepairAnimation.cs

@@ -1,62 +0,0 @@
-#region Copyright & License Information
-/*
- * Copyright 2007-2018 The OpenRA Developers (see AUTHORS)
- * This file is part of OpenRA, which is free software. It is made
- * available to you under the terms of the GNU General Public License
- * as published by the Free Software Foundation, either version 3 of
- * the License, or (at your option) any later version. For more
- * information, see COPYING.
- */
-#endregion
-
-using System.Linq;
-using OpenRA.Traits;
-
-namespace OpenRA.Mods.Common.Traits.Render
-{
-	[Desc("Replaces the building animation when it repairs a unit.")]
-	public class WithRepairAnimationInfo : ConditionalTraitInfo, Requires<WithSpriteBodyInfo>
-	{
-		[Desc("Sequence name to use")]
-		[SequenceReference] public readonly string Sequence = "active";
-
-		[Desc("Which sprite body to play the animation on.")]
-		public readonly string Body = "body";
-
-		public override object Create(ActorInitializer init) { return new WithRepairAnimation(init.Self, this); }
-	}
-
-	public class WithRepairAnimation : ConditionalTrait<WithRepairAnimationInfo>, INotifyRepair, INotifyBuildComplete, INotifySold
-	{
-		readonly WithSpriteBody spriteBody;
-		bool buildComplete;
-
-		public WithRepairAnimation(Actor self, WithRepairAnimationInfo info)
-			: base(info)
-		{
-			spriteBody = self.TraitsImplementing<WithSpriteBody>().Single(w => w.Info.Name == Info.Body);
-		}
-
-		void INotifyRepair.BeforeRepair(Actor self, Actor target) { }
-
-		void INotifyRepair.RepairTick(Actor self, Actor target)
-		{
-			if (buildComplete && !IsTraitDisabled)
-				spriteBody.PlayCustomAnimation(self, Info.Sequence);
-		}
-
-		void INotifyRepair.AfterRepair(Actor self, Actor target) { }
-
-		void INotifyBuildComplete.BuildingComplete(Actor self)
-		{
-			buildComplete = true;
-		}
-
-		void INotifySold.Selling(Actor self)
-		{
-			buildComplete = false;
-		}
-
-		void INotifySold.Sold(Actor self) { }
-	}
-}

OpenRA.Mods.Common/Traits/Render/WithResupplyAnimation.cs

@@ -0,0 +1,111 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2018 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using System;
+using System.Linq;
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.Common.Traits.Render
+{
+	[Flags]
+	public enum ResupplyType
+	{
+		Rearm = 1,
+		Repair = 2
+	}
+
+	[Desc("Replaces the default animation when actor resupplies a unit.")]
+	public class WithResupplyAnimationInfo : ConditionalTraitInfo, Requires<WithSpriteBodyInfo>
+	{
+		[Desc("Sequence name to use")]
+		[SequenceReference] public readonly string Sequence = "active";
+
+		[Desc("Which sprite body to play the animation on.")]
+		public readonly string Body = "body";
+
+		[Desc("Events leading to the animation getting played. Possible values currently are: Rearm, Repair.")]
+		public readonly ResupplyType PlayAnimationOn = ResupplyType.Rearm | ResupplyType.Repair;
+
+		public override object Create(ActorInitializer init) { return new WithResupplyAnimation(init.Self, this); }
+	}
+
+	public class WithResupplyAnimation : ConditionalTrait<WithResupplyAnimationInfo>, INotifyRepair, INotifyRearm, INotifyBuildComplete, INotifySold, ITick
+	{
+		readonly WithSpriteBody spriteBody;
+		bool buildComplete;
+		bool animPlaying;
+		bool repairing;
+		bool rearming;
+
+		public WithResupplyAnimation(Actor self, WithResupplyAnimationInfo info)
+			: base(info)
+		{
+			spriteBody = self.TraitsImplementing<WithSpriteBody>().Single(w => w.Info.Name == Info.Body);
+		}
+
+		void ITick.Tick(Actor self)
+		{
+			if (!buildComplete || IsTraitDisabled)
+				return;
+
+			if (!animPlaying
+				&& ((repairing && Info.PlayAnimationOn.HasFlag(ResupplyType.Repair))
+					|| (rearming && Info.PlayAnimationOn.HasFlag(ResupplyType.Rearm))))
+			{
+				spriteBody.PlayCustomAnimationRepeating(self, Info.Sequence);
+				animPlaying = true;
+			}
+			else if (animPlaying
+				&& (!repairing || !Info.PlayAnimationOn.HasFlag(ResupplyType.Repair))
+				&& (!rearming || !Info.PlayAnimationOn.HasFlag(ResupplyType.Rearm)))
+			{
+				spriteBody.CancelCustomAnimation(self);
+				animPlaying = false;
+			}
+		}
+
+		void INotifyRepair.BeforeRepair(Actor self, Actor target)
+		{
+			repairing = true;
+		}
+
+		void INotifyRepair.RepairTick(Actor self, Actor target) { }
+
+		void INotifyRepair.AfterRepair(Actor self, Actor target)
+		{
+			repairing = false;
+		}
+
+		void INotifyRearm.RearmingStarted(Actor self, Actor target)
+		{
+			rearming = true;
+		}
+
+		void INotifyRearm.Rearming(Actor self, Actor target) { }
+
+		void INotifyRearm.RearmingFinished(Actor self, Actor target)
+		{
+			rearming = false;
+		}
+
+		void INotifyBuildComplete.BuildingComplete(Actor self)
+		{
+			buildComplete = true;
+		}
+
+		void INotifySold.Selling(Actor self)
+		{
+			buildComplete = false;
+		}
+
+		void INotifySold.Sold(Actor self) { }
+	}
+}

OpenRA.Mods.Common/Traits/Repairable.cs

@@ -124,7 +124,7 @@ namespace OpenRA.Mods.Common.Traits
 			// will need to be rewritten anyway, so this is OK for now.
 			self.QueueActivity(movement.MoveTo(self.World.Map.CellContaining(targetActor.CenterPosition), targetActor));
 			if (CanRearmAt(targetActor) && CanRearm())
-				self.QueueActivity(new Rearm(self));
+				self.QueueActivity(new Rearm(self, targetActor, new WDist(512)));
 
 			// Add a CloseEnough range of 512 to ensure we're at the host actor
 			self.QueueActivity(new Repair(self, targetActor, new WDist(512)));

OpenRA.Mods.Common/TraitsInterfaces.cs

@@ -311,7 +311,12 @@ namespace OpenRA.Mods.Common.Traits
 	}
 
 	[RequireExplicitImplementation]
-	public interface INotifyRearm { void Rearming(Actor host, Actor other); }
+	public interface INotifyRearm
+	{
+		void RearmingStarted(Actor host, Actor other);
+		void Rearming(Actor host, Actor other);
+		void RearmingFinished(Actor host, Actor other);
+	}
 
 	[RequireExplicitImplementation]
 	public interface IRenderInfantrySequenceModifier

OpenRA.Mods.Common/UpdateRules/Rules/20180923/MergeRearmAndRepairAnimation.cs

@@ -0,0 +1,107 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2018 The OpenRA Developers (see AUTHORS)
+ * This file is part of OpenRA, which is free software. It is made
+ * available to you under the terms of the GNU General Public License
+ * as published by the Free Software Foundation, either version 3 of
+ * the License, or (at your option) any later version. For more
+ * information, see COPYING.
+ */
+#endregion
+
+using System.Collections.Generic;
+using System.Linq;
+
+namespace OpenRA.Mods.Common.UpdateRules.Rules
+{
+	public class MergeRearmAndRepairAnimation : UpdateRule
+	{
+		public override string Name { get { return "WithRearmAnimation and WithRepairAnimation were merged to WithResupplyAnimation"; } }
+		public override string Description
+		{
+			get
+			{
+				return "The WithRearmAnimation  and WithRepairAnimation traits were merged intto a single\n" +
+					"WithResupplyAnimation trait.";
+			}
+		}
+
+		bool displayedMessage;
+		public override IEnumerable<string> AfterUpdate(ModData modData)
+		{
+			var message = "If an actor had both a WithRearmAnimation and a WithRepairAnimation\n"
+				+ "or multiple traits of either type, you may want to check the update results for possible\n"
+				+ "redundant entries.\n";
+
+			if (!displayedMessage)
+				yield return message;
+
+			displayedMessage = true;
+		}
+
+		public override IEnumerable<string> UpdateActorNode(ModData modData, MiniYamlNode actorNode)
+		{
+			var rearmAnims = actorNode.ChildrenMatching("WithRearmAnimation");
+			var repairAnims = actorNode.ChildrenMatching("WithRepairAnimation");
+			var rearmAnimsTotal = rearmAnims.Count();
+			var repairAnimsTotal = repairAnims.Count();
+
+			if (rearmAnimsTotal == 0 && repairAnimsTotal == 0)
+				yield break;
+			else if (rearmAnimsTotal == 1 && repairAnimsTotal == 0)
+				foreach (var rearmAnim in rearmAnims)
+					rearmAnim.RenameKey("WithResupplyAnimation");
+			else if (rearmAnimsTotal == 0 && repairAnimsTotal == 1)
+				foreach (var repairAnim in repairAnims)
+					repairAnim.RenameKey("WithResupplyAnimation");
+			else if (rearmAnimsTotal == 1 && repairAnimsTotal == 1)
+			{
+				var rearmAnim = rearmAnims.First();
+				var repairAnim = repairAnims.First();
+				var rearmSequence = rearmAnim.LastChildMatching("Sequence");
+				var rearmBody = rearmAnim.LastChildMatching("Body");
+				var repairSequence = repairAnim.LastChildMatching("Sequence");
+				var repairBody = repairAnim.LastChildMatching("Body");
+				var matchingSequences = (rearmSequence == null && repairSequence == null)
+					|| (rearmSequence != null && repairSequence != null && rearmSequence.Value.Value == repairSequence.Value.Value);
+				var matchingBodies = (rearmBody == null && repairBody == null)
+					|| (rearmBody != null && repairBody != null && rearmBody.Value.Value == repairBody.Value.Value);
+
+				// If neither animation strays from the default values, we can safely merge them
+				if (matchingSequences && matchingBodies)
+				{
+					rearmAnim.RenameKey("WithResupplyAnimation");
+					actorNode.RemoveNode(repairAnim);
+				}
+				else
+				{
+					rearmAnim.RenameKey("WithResupplyAnimation@Rearm", false, true);
+					repairAnim.RenameKey("WithResupplyAnimation@Repair", false, true);
+				}
+			}
+			else
+			{
+				// If we got here, we have more than one of at least one of the two animation traits.
+				var rearmAnimCount = 0;
+				foreach (var rearmAnim in rearmAnims)
+				{
+					++rearmAnimCount;
+					rearmAnim.RenameKey("WithResupplyAnimation@Rearm" + rearmAnimCount.ToString(), false, true);
+					var playOnRearmNode = new MiniYamlNode("PlayAnimationOn", "Rearm");
+					rearmAnim.AddNode(playOnRearmNode);
+				}
+
+				var repairAnimCount = 0;
+				foreach (var repairAnim in repairAnims)
+				{
+					++repairAnimCount;
+					repairAnim.RenameKey("WithResupplyAnimation@Repair" + repairAnimCount.ToString(), false, true);
+					var playOnRepairNode = new MiniYamlNode("PlayAnimationOn", "Repair");
+					repairAnim.AddNode(playOnRepairNode);
+				}
+			}
+
+			yield break;
+		}
+	}
+}

OpenRA.Mods.Common/UpdateRules/UpdatePath.cs

@@ -91,6 +91,7 @@ namespace OpenRA.Mods.Common.UpdateRules
 				// Bleed only changes here
 				new RenameEditorTilesetFilter(),
 				new DefineNotificationDefaults(),
+				new MergeRearmAndRepairAnimation(),
 			})
 		};
 

mods/cnc/rules/structures.yaml

@@ -521,7 +521,7 @@ HPAD:
 		HpPerStep: 1000
 		PlayerExperience: 25
 		StartRepairingNotification: Repairing
-	WithRepairAnimation:
+	WithResupplyAnimation:
 	RallyPoint:
 	ProductionQueue@GDI:
 		Type: Aircraft.GDI
@@ -654,7 +654,7 @@ FIX:
 		PlayerExperience: 25
 		StartRepairingNotification: Repairing
 	RallyPoint:
-	WithRepairAnimation:
+	WithResupplyAnimation:
 	Power:
 		Amount: -20
 	ProvidesPrerequisite@buildingname:

mods/ra/rules/structures.yaml

@@ -1338,7 +1338,7 @@ HPAD:
 		ReferencePoint: Top
 		ZOffset: 256
 		RequiresCondition: primary
-	WithRearmAnimation:
+	WithResupplyAnimation:
 
 AFLD:
 	Inherits: ^Building
@@ -1478,7 +1478,7 @@ AFLD:
 		ReferencePoint: Top
 		ZOffset: 256
 		RequiresCondition: primary
-	WithRearmAnimation:
+	WithResupplyAnimation:
 
 AFLD.Ukraine:
 	Inherits: AFLD
@@ -1874,8 +1874,7 @@ FIX:
 		StartRepairingNotification: Repairing
 		FinishRepairingNotification: UnitRepaired
 		PlayerExperience: 15
-	WithRepairAnimation:
+	WithResupplyAnimation:
-	WithRearmAnimation:
 	Power:
 		Amount: -30
 	ProvidesPrerequisite@buildingname: