Replace WithChargeAnimation with -SpriteBody

PlayFetchIndex on a With*Animation trait conflicts with the animation
concept, as it's bound to conflict with pretty much all 'normal'
animation traits and blocks progress on the animation priority system.

We also already have multiple precedent SpriteBody traits of similar kind,
like WithGateSpriteBody and WithWallSpriteBody.

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

@@ -856,7 +856,7 @@
     <Compile Include="Traits\World\ElevatedBridgeLayer.cs" />
     <Compile Include="Traits\World\ElevatedBridgePlaceholder.cs" />
     <Compile Include="Traits\Attack\AttackCharges.cs" />
-    <Compile Include="Traits\Render\WithChargeAnimation.cs" />
+    <Compile Include="Traits\Render\WithChargeSpriteBody.cs" />
     <Compile Include="Traits\Render\WithChargeOverlay.cs" />
     <Compile Include="WebServices.cs" />
     <Compile Include="Traits\Conditions\GrantConditionOnLineBuildDirection.cs" />
@@ -948,6 +948,7 @@
     <Compile Include="UpdateRules\Rules\20180923\ExtractHackyAIModules.cs" />
     <Compile Include="UpdateRules\Rules\20180923\DefineLevelUpImageDefault.cs" />
     <Compile Include="UpdateRules\Rules\20180923\RemovedAutoCarryallCircleTurnSpeed.cs" />
+    <Compile Include="UpdateRules\Rules\20180923\ReplacedWithChargeAnimation.cs" />
     <Compile Include="Traits\Player\PlayerResources.cs" />
     <Compile Include="UtilityCommands\DumpSequenceSheetsCommand.cs" />
     <Compile Include="Traits\Render\WithBuildingRepairDecoration.cs" />

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

@@ -1,49 +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("Render trait that varies the animation frame based on the AttackCharges trait's charge level.")]
-	class WithChargeAnimationInfo : ConditionalTraitInfo, Requires<WithSpriteBodyInfo>, Requires<AttackChargesInfo>
-	{
-		[SequenceReference]
-		[Desc("Sequence to use for the charge levels.")]
-		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 WithChargeAnimation(init.Self, this); }
-	}
-
-	class WithChargeAnimation : ConditionalTrait<WithChargeAnimationInfo>
-	{
-		readonly WithSpriteBody wsb;
-		readonly AttackCharges attackCharges;
-
-		public WithChargeAnimation(Actor self, WithChargeAnimationInfo info)
-			: base(info)
-		{
-			wsb = self.TraitsImplementing<WithSpriteBody>().Single(w => w.Info.Name == info.Body);
-			attackCharges = self.Trait<AttackCharges>();
-		}
-
-		protected override void TraitEnabled(Actor self)
-		{
-			var attackChargesInfo = (AttackChargesInfo)attackCharges.Info;
-			wsb.DefaultAnimation.PlayFetchIndex(wsb.NormalizeSequence(self, Info.Sequence),
-				() => int2.Lerp(0, wsb.DefaultAnimation.CurrentSequence.Length, attackCharges.ChargeLevel, attackChargesInfo.ChargeLevel + 1));
-		}
-	}
-}

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

@@ -0,0 +1,65 @@
+#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;
+using OpenRA.Graphics;
+using OpenRA.Mods.Common.Graphics;
+using OpenRA.Traits;
+
+namespace OpenRA.Mods.Common.Traits.Render
+{
+	[Desc("Render trait that varies the sprite body frame based on the AttackCharges trait's charge level.")]
+	public class WithChargeSpriteBodyInfo : WithSpriteBodyInfo, Requires<AttackChargesInfo>
+	{
+		public override object Create(ActorInitializer init) { return new WithChargeSpriteBody(init, this); }
+
+		public override IEnumerable<IActorPreview> RenderPreviewSprites(ActorPreviewInitializer init, RenderSpritesInfo rs, string image, int facings, PaletteReference p)
+		{
+			if (!EnabledByDefault)
+				yield break;
+
+			var anim = new Animation(init.World, image);
+			anim.PlayFetchIndex(RenderSprites.NormalizeSequence(anim, init.GetDamageState(), Sequence), () => 0);
+
+			yield return new SpriteActorPreview(anim, () => WVec.Zero, () => 0, p, rs.Scale);
+		}
+	}
+
+	public class WithChargeSpriteBody : WithSpriteBody
+	{
+		readonly AttackCharges attackCharges;
+
+		public WithChargeSpriteBody(ActorInitializer init, WithChargeSpriteBodyInfo info)
+			: base(init, info, () => 0)
+		{
+			attackCharges = init.Self.Trait<AttackCharges>();
+			ConfigureAnimation(init.Self);
+		}
+
+		void ConfigureAnimation(Actor self)
+		{
+			var attackChargesInfo = (AttackChargesInfo)attackCharges.Info;
+			DefaultAnimation.PlayFetchIndex(NormalizeSequence(self, Info.Sequence),
+				() => int2.Lerp(0, DefaultAnimation.CurrentSequence.Length, attackCharges.ChargeLevel, attackChargesInfo.ChargeLevel + 1));
+		}
+
+		protected override void TraitEnabled(Actor self)
+		{
+			// Do nothing - we just want to disable the default WithSpriteBody implementation
+		}
+
+		public override void CancelCustomAnimation(Actor self)
+		{
+			ConfigureAnimation(self);
+		}
+	}
+}

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

@@ -92,7 +92,7 @@ namespace OpenRA.Mods.Common.Traits.Render
 			DefaultAnimation.PlayRepeating(NormalizeSequence(self, Info.Sequence));
 		}
 
-		public void PlayCustomAnimation(Actor self, string name, Action after = null)
+		public virtual void PlayCustomAnimation(Actor self, string name, Action after = null)
 		{
 			DefaultAnimation.PlayThen(NormalizeSequence(self, name), () =>
 			{
@@ -102,12 +102,12 @@ namespace OpenRA.Mods.Common.Traits.Render
 			});
 		}
 
-		public void PlayCustomAnimationRepeating(Actor self, string name)
+		public virtual void PlayCustomAnimationRepeating(Actor self, string name)
 		{
 			DefaultAnimation.PlayRepeating(NormalizeSequence(self, name));
 		}
 
-		public void PlayCustomAnimationBackwards(Actor self, string name, Action after = null)
+		public virtual void PlayCustomAnimationBackwards(Actor self, string name, Action after = null)
 		{
 			DefaultAnimation.PlayBackwardsThen(NormalizeSequence(self, name), () =>
 			{
@@ -117,7 +117,7 @@ namespace OpenRA.Mods.Common.Traits.Render
 			});
 		}
 
-		public void CancelCustomAnimation(Actor self)
+		public virtual void CancelCustomAnimation(Actor self)
 		{
 			DefaultAnimation.PlayRepeating(NormalizeSequence(self, Info.Sequence));
 		}

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

@@ -0,0 +1,73 @@
+#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 ReplacedWithChargeAnimation : UpdateRule
+	{
+		public override string Name { get { return "Replaced WithChargeAnimation with WithChargeSpriteBody"; } }
+		public override string Description
+		{
+			get
+			{
+				return "Replaced WithChargeAnimation with WithChargeSpriteBody.";
+			}
+		}
+
+		readonly List<string> locations = new List<string>();
+
+		public override IEnumerable<string> AfterUpdate(ModData modData)
+		{
+			if (locations.Any())
+				yield return "WithChargeAnimation has been replaced by WithChargeSpriteBody.\n" +
+					"You may need to disable/remove any previous (including inherited) *SpriteBody traits\n" +
+					"on the following actors:\n" +
+					UpdateUtils.FormatMessageList(locations);
+
+			locations.Clear();
+		}
+
+		public override IEnumerable<string> UpdateActorNode(ModData modData, MiniYamlNode actorNode)
+		{
+			var chargeAnims = actorNode.ChildrenMatching("WithChargeAnimation");
+
+			foreach (var ca in chargeAnims)
+			{
+				// If it's a trait removal, we only rename it.
+				if (ca.IsRemoval())
+				{
+					ca.RenameKey("WithChargeSpriteBody");
+					continue;
+				}
+
+				var sequence = ca.LastChildMatching("Sequence");
+				var body = ca.LastChildMatching("Body");
+
+				if (sequence == null)
+				{
+					var newSequenceNode = new MiniYamlNode("Sequence", "active");
+					ca.AddNode(newSequenceNode);
+				}
+
+				if (body != null)
+					ca.RemoveNode(body);
+
+				ca.RenameKey("WithChargeSpriteBody");
+				locations.Add("{0} ({1})".F(actorNode.Key, ca.Location.Filename));
+			}
+
+			yield break;
+		}
+	}
+}

OpenRA.Mods.Common/UpdateRules/UpdatePath.cs

@@ -111,6 +111,7 @@ namespace OpenRA.Mods.Common.UpdateRules
 				new DefineLevelUpImageDefault(),
 				new RemovedAutoCarryallCircleTurnSpeed(),
 				new RemoveAttackIgnoresVisibility(),
+				new ReplacedWithChargeAnimation(),
 			})
 		};
 

OpenRA.Mods.Common/UpdateRules/UpdateUtils.cs

@@ -237,10 +237,16 @@ namespace OpenRA.Mods.Common.UpdateRules
 					file.Item1.Update(file.Item2, Encoding.UTF8.GetBytes(file.Item3.WriteToString()));
 		}
 
+		/// <summary>Checks if node is a removal (has '-' prefix)</summary>
+		public static bool IsRemoval(this MiniYamlNode node)
+		{
+			return node.Key[0].ToString() == "-";
+		}
+
 		/// <summary>Renames a yaml key preserving any @suffix</summary>
 		public static void RenameKey(this MiniYamlNode node, string newKey, bool preserveSuffix = true, bool includeRemovals = true)
 		{
-			var prefix = includeRemovals && node.Key[0].ToString() == "-" ? "-" : "";
+			var prefix = includeRemovals && node.IsRemoval() ? "-" : "";
 			var split = node.Key.IndexOf("@", StringComparison.Ordinal);
 			if (preserveSuffix && split > -1)
 				node.Key = prefix + newKey + node.Key.Substring(split);
@@ -298,7 +304,7 @@ namespace OpenRA.Mods.Common.UpdateRules
 			if (node.Key == null)
 				return false;
 
-			var prefix = includeRemovals && node.Key[0].ToString() == "-" ? "-" : "";
+			var prefix = includeRemovals && node.IsRemoval() ? "-" : "";
 			if (node.Key == prefix + match)
 				return true;
 
@@ -310,12 +316,33 @@ namespace OpenRA.Mods.Common.UpdateRules
 			return atPosition > 0 && node.Key.Substring(0, atPosition) == prefix + match;
 		}
 
+		/// <summary>Returns true if the node is of the form <*match*>, <*match*>@arbitrary or <arbitrary>@*match*</summary>
+		public static bool KeyContains(this MiniYamlNode node, string match, bool ignoreSuffix = true, bool includeRemovals = true)
+		{
+			if (node.Key == null)
+				return false;
+
+			var atPosition = node.Key.IndexOf('@');
+			var relevantPart = ignoreSuffix && atPosition > 0 ? node.Key.Substring(0, atPosition) : node.Key;
+
+			if (relevantPart.Contains(match) && (includeRemovals || !node.IsRemoval()))
+				return true;
+
+			return false;
+		}
+
 		/// <summary>Returns children with keys equal to [match] or [match]@[arbitrary suffix]</summary>
 		public static IEnumerable<MiniYamlNode> ChildrenMatching(this MiniYamlNode node, string match, bool ignoreSuffix = true, bool includeRemovals = true)
 		{
 			return node.Value.Nodes.Where(n => n.KeyMatches(match, ignoreSuffix, includeRemovals));
 		}
 
+		/// <summary>Returns children whose keys contain 'match' (optionally in the suffix)</summary>
+		public static IEnumerable<MiniYamlNode> ChildrenContaining(this MiniYamlNode node, string match, bool ignoreSuffix = true, bool includeRemovals = true)
+		{
+			return node.Value.Nodes.Where(n => n.KeyContains(match, ignoreSuffix, includeRemovals));
+		}
+
 		public static MiniYamlNode LastChildMatching(this MiniYamlNode node, string match, bool includeRemovals = true)
 		{
 			return node.ChildrenMatching(match, includeRemovals: includeRemovals).LastOrDefault();

mods/cnc/rules/structures.yaml

@@ -923,8 +923,9 @@ OBLI:
 		Range: 8c0
 	WithBuildingBib:
 		HasMinibib: Yes
-	WithChargeAnimation:
-		RequiresCondition: !build-incomplete
+	-WithSpriteBody:
+	WithChargeSpriteBody:
+		Sequence: active
 	Armament:
 		Weapon: Laser
 		LocalOffset: 0,-85,1280