Unify the code style across crate actions.

OpenRA.Mods.RA/CrateAction.cs

@@ -42,8 +42,8 @@ namespace OpenRA.Mods.RA
 
 	public class CrateAction
 	{
-		public Actor self;
+		readonly Actor self;
-		public CrateActionInfo info;
+		readonly CrateActionInfo info;
 
 		public CrateAction(Actor self, CrateActionInfo info)
 		{
@@ -75,8 +75,7 @@ namespace OpenRA.Mods.RA
 			Sound.PlayToPlayer(collector.Owner, info.Notification);
 
 			if (info.Effect != null)
-				collector.World.AddFrameEndTask(
+				collector.World.AddFrameEndTask(w => w.Add(new CrateEffect(collector, info.Effect, info.Palette)));
-					w => w.Add(new CrateEffect(collector, info.Effect, info.Palette)));
 		}
 	}
 }

OpenRA.Mods.RA/Crates/DuplicateUnitCrateAction.cs

@@ -22,10 +22,10 @@ namespace OpenRA.Mods.RA.Crates
 		[Desc("The maximum number of duplicates to make.")]
 		public readonly int MaxAmount = 2;
 
-		[Desc("The minimum number of duplicates to make.","Overrules MaxDuplicatesWorth.")]
+		[Desc("The minimum number of duplicates to make. Overrules MaxDuplicatesWorth.")]
 		public readonly int MinAmount = 1;
 
-		[Desc("The maximum total cost allowed for the duplicates.","Duplication stops if the total worth will exceed this number.","-1 = no limit")]
+		[Desc("The maximum total cost allowed for the duplicates.", "Duplication stops if the total worth will exceed this number.", "-1 = no limit")]
 		public readonly int MaxDuplicatesWorth = -1;
 
 		[Desc("The list of unit types we are allowed to duplicate.")]
@@ -42,47 +42,54 @@ namespace OpenRA.Mods.RA.Crates
 
 	class DuplicateUnitCrateAction : CrateAction
 	{
-		public readonly DuplicateUnitCrateActionInfo Info;
+		readonly DuplicateUnitCrateActionInfo info;
+		readonly Actor self;
 		readonly List<CPos> usedCells = new List<CPos>();
 
 		public DuplicateUnitCrateAction(Actor self, DuplicateUnitCrateActionInfo info)
-			: base(self, info) { Info = info; }
+			: base(self, info)
+		{
+			this.self = self;
+			this.info = info;
+		}
 
 		public bool CanGiveTo(Actor collector)
 		{
-			if (Info.ValidRaces.Any() && !Info.ValidRaces.Contains(collector.Owner.Country.Race))
+			if (info.ValidRaces.Any() && !info.ValidRaces.Contains(collector.Owner.Country.Race))
 				return false;
 
 			var targetable = collector.Info.Traits.GetOrDefault<ITargetableInfo>();
-			if (targetable == null ||
+			if (targetable == null || !info.ValidDuplicateTypes.Intersect(targetable.GetTargetTypes()).Any())
-				!Info.ValidDuplicateTypes.Intersect(targetable.GetTargetTypes()).Any())
 				return false;
 
-			if (!GetSuitableCells(collector.Location, collector.Info.Name).Any()) return false;
+			if (!GetSuitableCells(collector.Location, collector.Info.Name).Any())
+				return false;
 
 			return true;
 		}
 
 		public override int GetSelectionShares(Actor collector)
 		{
-			if (!CanGiveTo(collector)) return 0;
+			if (!CanGiveTo(collector))
+				return 0;
+
 			return base.GetSelectionShares(collector);
 		}
 
 		public override void Activate(Actor collector)
 		{
-			int AllowedWorthLeft = Info.MaxDuplicatesWorth;
+			var allowedWorthLeft = info.MaxDuplicatesWorth;
-			int DupesMade = 0;
+			var dupesMade = 0;
 
-			while ((DupesMade < Info.MaxAmount) && (AllowedWorthLeft > 0) || (DupesMade < Info.MinAmount))
+			while ((dupesMade < info.MaxAmount && allowedWorthLeft > 0) || dupesMade < info.MinAmount)
 			{
-				//If the collector has a cost, and we have a max duplicate worth, then update how much dupe worth is left
+				// If the collector has a cost, and we have a max duplicate worth, then update how much dupe worth is left
 				var unitCost = collector.Info.Traits.Get<ValuedInfo>().Cost;
-				AllowedWorthLeft -= (Info.MaxDuplicatesWorth > 0) ? unitCost : 0;
+				allowedWorthLeft -= info.MaxDuplicatesWorth > 0 ? unitCost : 0;
-				if ((AllowedWorthLeft < 0) && (DupesMade >= Info.MinAmount))
+				if (allowedWorthLeft < 0 && dupesMade >= info.MinAmount)
 					break;
 
-				DupesMade++;
+				dupesMade++;
 
 				var location = ChooseEmptyCellNear(collector, collector.Info.Name);
 				if (location != null)
@@ -92,10 +99,11 @@ namespace OpenRA.Mods.RA.Crates
 					w => w.CreateActor(collector.Info.Name, new TypeDictionary
 					{
 						new LocationInit(location.Value),
-						new OwnerInit(Info.Owner ?? collector.Owner.InternalName)
+						new OwnerInit(info.Owner ?? collector.Owner.InternalName)
 					}));
 				}
 			}
+
 			base.Activate(collector);
 		}
 

OpenRA.Mods.RA/Crates/ExplodeCrateAction.cs

@@ -25,13 +25,19 @@ namespace OpenRA.Mods.RA
 
 	class ExplodeCrateAction : CrateAction
 	{
+		readonly ExplodeCrateActionInfo info;
+
 		public ExplodeCrateAction(Actor self, ExplodeCrateActionInfo info)
-			: base(self, info) {}
+			: base(self, info)
+		{
+			this.info = info;
+		}
 
 		public override void Activate(Actor collector)
 		{
-			var weapon = self.World.Map.Rules.Weapons[((ExplodeCrateActionInfo)info).Weapon.ToLowerInvariant()];
+			var weapon = collector.World.Map.Rules.Weapons[info.Weapon.ToLowerInvariant()];
-			weapon.Impact(Target.FromPos(collector.CenterPosition), self, Enumerable.Empty<int>());
+			weapon.Impact(Target.FromPos(collector.CenterPosition), collector, Enumerable.Empty<int>());
+
 			base.Activate(collector);
 		}
 	}

OpenRA.Mods.RA/Crates/GiveCashCrateAction.cs

@@ -27,19 +27,21 @@ namespace OpenRA.Mods.RA
 
 	class GiveCashCrateAction : CrateAction
 	{
+		readonly GiveCashCrateActionInfo info;
 		public GiveCashCrateAction(Actor self, GiveCashCrateActionInfo info)
-			: base(self, info) {}
+			: base(self, info)
+		{
+			this.info = info;
+		}
 
 		public override void Activate(Actor collector)
 		{
 			collector.World.AddFrameEndTask(w =>
 			{
-				var crateInfo = (GiveCashCrateActionInfo)info;
+				collector.Owner.PlayerActor.Trait<PlayerResources>().GiveCash(info.Amount);
-				var amount = crateInfo.Amount;
-				collector.Owner.PlayerActor.Trait<PlayerResources>().GiveCash(amount);
 
-				if (crateInfo.UseCashTick)
+				if (info.UseCashTick)
-					w.Add(new FloatingText(collector.CenterPosition, collector.Owner.Color.RGB, FloatingText.FormatCashTick(amount), 30));
+					w.Add(new FloatingText(collector.CenterPosition, collector.Owner.Color.RGB, FloatingText.FormatCashTick(info.Amount), 30));
 			});
 
 			base.Activate(collector);

OpenRA.Mods.RA/Crates/GiveMcvCrateAction.cs

@@ -12,7 +12,7 @@ using System.Linq;
 
 namespace OpenRA.Mods.RA.Crates
 {
-	[Desc("Spawns units when collected.","Adjust selection shares when player has no base.")]
+	[Desc("Spawns units when collected.", "Adjust selection shares when player has no base.")]
 	class GiveMcvCrateActionInfo : GiveUnitCrateActionInfo
 	{
 		[Desc("The selection shares to use if the collector has no base.")]
@@ -23,19 +23,23 @@ namespace OpenRA.Mods.RA.Crates
 
 	class GiveMcvCrateAction : GiveUnitCrateAction
 	{
+		readonly GiveMcvCrateActionInfo info;
 		public GiveMcvCrateAction(Actor self, GiveMcvCrateActionInfo info)
-			: base(self, info) { }
+			: base(self, info)
+		{
+			this.info = info;
+		}
 
 		public override int GetSelectionShares(Actor collector)
 		{
+			// There's some other really good reason why we shouldn't give this.
 			if (!CanGiveTo(collector))
-				return 0;	// there's some other really good reason why we shouldn't give this.
+				return 0;
 
-			var hasBase = self.World.ActorsWithTrait<BaseBuilding>()
+			var hasBase = collector.World.ActorsWithTrait<BaseBuilding>()
 				.Any(a => a.Actor.Owner == collector.Owner);
 
-			return hasBase ? info.SelectionShares :
+			return hasBase ? info.SelectionShares : info.NoBaseSelectionShares;
-				((GiveMcvCrateActionInfo)info).NoBaseSelectionShares;
 		}
 	}
 }

OpenRA.Mods.RA/Crates/GiveUnitCrateAction.cs

@@ -34,26 +34,29 @@ namespace OpenRA.Mods.RA.Crates
 
 	class GiveUnitCrateAction : CrateAction
 	{
-		public readonly GiveUnitCrateActionInfo Info;
+		readonly Actor self;
+		readonly GiveUnitCrateActionInfo info;
 		readonly List<CPos> usedCells = new List<CPos>();
 
 		public GiveUnitCrateAction(Actor self, GiveUnitCrateActionInfo info)
 			: base(self, info)
 		{
-			Info = info;
+			this.self = self;
-			if (!Info.Units.Any())
+			this.info = info;
+			if (!info.Units.Any())
 				throw new YamlException("A GiveUnitCrateAction does not specify any units to give. This might be because the yaml is referring to 'Unit' rather than 'Units'.");
 		}
 
 		public bool CanGiveTo(Actor collector)
 		{
-			if (Info.ValidRaces.Any() && !Info.ValidRaces.Contains(collector.Owner.Country.Race))
+			if (info.ValidRaces.Any() && !info.ValidRaces.Contains(collector.Owner.Country.Race))
 				return false;
 
-			foreach (string unit in Info.Units)
+			foreach (string unit in info.Units)
 			{
 				// avoid dumping tanks in the sea, and ships on dry land.
-				if (!GetSuitableCells(collector.Location, unit).Any()) return false;
+				if (!GetSuitableCells(collector.Location, unit).Any())
+					return false;
 			}
 
 			return true;
@@ -61,13 +64,15 @@ namespace OpenRA.Mods.RA.Crates
 
 		public override int GetSelectionShares(Actor collector)
 		{
-			if (!CanGiveTo(collector)) return 0;
+			if (!CanGiveTo(collector))
+				return 0;
+
 			return base.GetSelectionShares(collector);
 		}
 
 		public override void Activate(Actor collector)
 		{
-			foreach (var u in Info.Units)
+			foreach (var u in info.Units)
 			{
 				var unit = u; // avoiding access to modified closure
 
@@ -79,10 +84,11 @@ namespace OpenRA.Mods.RA.Crates
 					w => w.CreateActor(unit, new TypeDictionary
 					{
 						new LocationInit(location.Value),
-						new OwnerInit(Info.Owner ?? collector.Owner.InternalName)
+						new OwnerInit(info.Owner ?? collector.Owner.InternalName)
 					}));
 				}
 			}
+
 			base.Activate(collector);
 		}
 

OpenRA.Mods.RA/Crates/HealUnitsCrateAction.cs

@@ -26,15 +26,14 @@ namespace OpenRA.Mods.RA.Crates
 
 		public override void Activate(Actor collector)
 		{
-			base.Activate(collector);
 			foreach (var unit in collector.World.Actors.Where(a => a.Owner == collector.Owner))
 			{
 				var health = unit.TraitOrDefault<Health>();
 				if (health != null && !health.IsDead)
-				{
 					health.InflictDamage(unit, unit, -(health.MaxHP - health.HP), null, true);
 			}
-			}
+
+			base.Activate(collector);
 		}
 	}
 }

OpenRA.Mods.RA/Crates/HideMapCrateAction.cs

@@ -32,9 +32,10 @@ namespace OpenRA.Mods.RA
 
 		public override void Activate(Actor collector)
 		{
-			base.Activate(collector);
 			if (collector.Owner == collector.World.LocalPlayer)
 				collector.Owner.Shroud.ResetExploration();
+
+			base.Activate(collector);
 		}
 	}
 }

OpenRA.Mods.RA/Crates/LevelUpCrateAction.cs

@@ -29,12 +29,14 @@ namespace OpenRA.Mods.RA
 
 	class LevelUpCrateAction : CrateAction
 	{
-		LevelUpCrateActionInfo Info;
+		readonly Actor self;
+		readonly LevelUpCrateActionInfo info;
 
 		public LevelUpCrateAction(Actor self, LevelUpCrateActionInfo info)
 			: base(self, info)
 		{
-			Info = info;
+			this.self = self;
+			this.info = info;
 		}
 
 		public override int GetSelectionShares(Actor collector)
@@ -45,26 +47,26 @@ namespace OpenRA.Mods.RA
 
 		public override void Activate(Actor collector)
 		{
-			collector.World.AddFrameEndTask(w =>
+			var inRange = self.World.FindActorsInCircle(self.CenterPosition, info.Range).Where(a =>
 			{
-				var gainsExperience = collector.TraitOrDefault<GainsExperience>();
+				// Don't upgrade the same unit twice
-				if (gainsExperience != null)
+				if (a == collector)
-					gainsExperience.GiveLevels(((LevelUpCrateActionInfo)info).Levels);
+					return false;
+
+				// Only upgrade the collecting player's units
+				// TODO: Also apply to allied units?
+				if (a.Owner != collector.Owner)
+					return false;
+
+				// Ignore units that can't level up
+				var ge = a.TraitOrDefault<GainsExperience>();
+				return ge != null && ge.CanGainLevel;
 			});
 
-			var inRange = self.World.FindActorsInCircle(self.CenterPosition, Info.Range);
+			if (info.MaxExtraCollectors > -1)
-			inRange = inRange.Where(a =>
+				inRange = inRange.Take(info.MaxExtraCollectors);
-				(a.Owner == collector.Owner) &&
-				(a != collector) &&
-				(a.TraitOrDefault<GainsExperience>() != null) &&
-				(a.TraitOrDefault<GainsExperience>().CanGainLevel));
-			if (inRange.Any())
-			{
-				if (Info.MaxExtraCollectors > -1)
-					inRange = inRange.Take(Info.MaxExtraCollectors);
 
-				if (inRange.Any())
+			foreach (var actor in inRange.Append(collector))
-					foreach (Actor actor in inRange)
 			{
 				actor.World.AddFrameEndTask(w =>
 				{
@@ -73,7 +75,6 @@ namespace OpenRA.Mods.RA
 						gainsExperience.GiveLevels(((LevelUpCrateActionInfo)info).Levels);
 				});
 			}
-			}
 
 			base.Activate(collector);
 		}

OpenRA.Mods.RA/Crates/RevealMapCrateAction.cs

@@ -23,12 +23,17 @@ namespace OpenRA.Mods.RA
 
 	class RevealMapCrateAction : CrateAction
 	{
+		readonly RevealMapCrateActionInfo info;
+
 		public RevealMapCrateAction(Actor self, RevealMapCrateActionInfo info)
-			: base(self, info) {}
+			: base(self, info)
+		{
+			this.info = info;
+		}
 
 		bool ShouldReveal(Player collectingPlayer)
 		{
-			if (((RevealMapCrateActionInfo)info).IncludeAllies)
+			if (info.IncludeAllies)
 				return collectingPlayer.World.LocalPlayer != null &&
 					collectingPlayer.Stances[collectingPlayer.World.LocalPlayer] == Stance.Ally;
 
@@ -37,10 +42,10 @@ namespace OpenRA.Mods.RA
 
 		public override void Activate(Actor collector)
 		{
-			base.Activate(collector);
+			if (ShouldReveal(collector.Owner))
-
-			if (ShouldReveal( collector.Owner ))
 				collector.Owner.Shroud.ExploreAll(collector.World);
+
+			base.Activate(collector);
 		}
 	}
 }

OpenRA.Mods.RA/Crates/SupportPowerCrateAction.cs

@@ -24,20 +24,23 @@ namespace OpenRA.Mods.RA.Crates
 
 	class SupportPowerCrateAction : CrateAction
 	{
-		SupportPowerCrateActionInfo Info;
+		SupportPowerCrateActionInfo info;
 		public SupportPowerCrateAction(Actor self, SupportPowerCrateActionInfo info)
-			: base(self, info) { Info = info; }
+			: base(self, info)
+		{
+			this.info = info;
+		}
 
 		// The free unit crate requires same race and the actor needs to be mobile.
 		// We want neither of these properties for crate power proxies.
 		public override void Activate(Actor collector)
 		{
-			base.Activate(collector);
+			collector.World.AddFrameEndTask(w => w.CreateActor(info.Proxy, new TypeDictionary
-
-			collector.World.AddFrameEndTask(w => w.CreateActor(Info.Proxy, new TypeDictionary
 			{
 				new OwnerInit(collector.Owner)
 			}));
+
+			base.Activate(collector);
 		}
 	}
 }

OpenRA.Mods.RA/Crates/UnitUpgradeCrateAction.cs

@@ -17,7 +17,7 @@ namespace OpenRA.Mods.RA.Crates
 	public class UnitUpgradeCrateActionInfo : CrateActionInfo
 	{
 		[Desc("The upgrade to grant.")]
-		public readonly string[] Upgrades = {};
+		public readonly string[] Upgrades = { };
 
 		[Desc("The range to search for extra collectors in.", "Extra collectors will also be granted the crate action.")]
 		public readonly WRange Range = new WRange(3);
@@ -30,24 +30,26 @@ namespace OpenRA.Mods.RA.Crates
 
 	public class UnitUpgradeCrateAction : CrateAction
 	{
-		readonly UnitUpgradeCrateActionInfo Info;
+		readonly Actor self;
+		readonly UnitUpgradeCrateActionInfo info;
 
 		public UnitUpgradeCrateAction(Actor self, UnitUpgradeCrateActionInfo info)
 			: base(self, info) 
 		{
-			Info = info; 
+			this.self = self;
+			this.info = info;
 		}
 
 		bool AcceptsUpgrade(Actor a)
 		{
 			return a.TraitsImplementing<IUpgradable>()
-				.Any(up => Info.Upgrades.Any(u => up.AcceptsUpgrade(u)));
+				.Any(up => info.Upgrades.Any(u => up.AcceptsUpgrade(u)));
 		}
 
 		void GrantActorUpgrades(Actor a)
 		{
 			foreach (var up in a.TraitsImplementing<IUpgradable>())
-				foreach (var u in Info.Upgrades)
+				foreach (var u in info.Upgrades)
 					if (up.AcceptsUpgrade(u))
 						up.UpgradeAvailable(a, u, true);
 		}
@@ -61,13 +63,13 @@ namespace OpenRA.Mods.RA.Crates
 		{
 			collector.World.AddFrameEndTask(w => GrantActorUpgrades(collector));
 
-			var actorsInRange = self.World.FindActorsInCircle(self.CenterPosition, Info.Range)
+			var actorsInRange = self.World.FindActorsInCircle(self.CenterPosition, info.Range)
 				.Where(a => a != self && a.Owner == collector.Owner && AcceptsUpgrade(a));
 
 			if (actorsInRange.Any())
 			{
-				if (Info.MaxExtraCollectors > -1)
+				if (info.MaxExtraCollectors > -1)
-					actorsInRange = actorsInRange.Take(Info.MaxExtraCollectors);
+					actorsInRange = actorsInRange.Take(info.MaxExtraCollectors);
 
 				collector.World.AddFrameEndTask(w =>
 				{