Initial HackyAI cleanup pass.

OpenRA.Mods.RA/AI/HackyAI.cs

@@ -25,12 +25,12 @@ namespace OpenRA.Mods.RA.AI
 		public readonly string Name = "Unnamed Bot";
 		public readonly int SquadSize = 8;
 
-		//intervals
 		public readonly int AssignRolesInterval = 20;
 		public readonly int RushInterval = 600;
 		public readonly int AttackForceInterval = 30;
 
-		public readonly string RallypointTestBuilding = "fact";		// temporary hack to maintain previous rallypoint behavior.
+		// Temporary hack to maintain previous rallypoint behavior.
+		public readonly string RallypointTestBuilding = "fact";
 		public readonly string[] UnitQueues = { "Vehicle", "Infantry", "Plane", "Ship", "Aircraft" };
 		public readonly bool ShouldRepairBuildings = true;
 
@@ -51,16 +51,6 @@ namespace OpenRA.Mods.RA.AI
 		[FieldLoader.LoadUsing("LoadBuildingLimits")]
 		public readonly Dictionary<string, int> BuildingLimits = null;
 
-		static object LoadActorList(MiniYaml y, string field)
-		{
-			return LoadList<float>(y, field);
-		}
-
-		static object LoadListList(MiniYaml y, string field)
-		{
-			return LoadList<string[]>(y,field);
-		}
-
 		static object LoadList<ValueType>(MiniYaml y, string field)
 		{
 			return y.NodesDict.ContainsKey(field)
@@ -70,11 +60,11 @@ namespace OpenRA.Mods.RA.AI
 				: new Dictionary<string, ValueType>();
 		}
 
-		static object LoadUnits(MiniYaml y) { return LoadActorList(y, "UnitsToBuild"); }
+		static object LoadUnits(MiniYaml y) { return LoadList<float>(y, "UnitsToBuild"); }
-		static object LoadBuildings(MiniYaml y) { return LoadActorList(y, "BuildingFractions"); }
+		static object LoadBuildings(MiniYaml y) { return LoadList<float>(y, "BuildingFractions"); }
 
-		static object LoadUnitsCommonNames(MiniYaml y) { return LoadListList(y, "UnitsCommonNames"); }
+		static object LoadUnitsCommonNames(MiniYaml y) { return LoadList<string[]>(y, "UnitsCommonNames"); }
-		static object LoadBuildingsCommonNames(MiniYaml y) { return LoadListList(y, "BuildingCommonNames"); }
+		static object LoadBuildingsCommonNames(MiniYaml y) { return LoadList<string[]>(y, "BuildingCommonNames"); }
 
 		static object LoadBuildingLimits(MiniYaml y) { return LoadList<int>(y, "BuildingLimits"); }
 
@@ -87,6 +77,8 @@ namespace OpenRA.Mods.RA.AI
 
 	public class HackyAI : ITick, IBot, INotifyDamage
 	{
+		static readonly List<Actor> NoActors = new List<Actor>();
+
 		bool enabled;
 		public int ticks;
 		public Player p;
@@ -95,27 +87,34 @@ namespace OpenRA.Mods.RA.AI
 		PowerManager playerPower;
 		SupportPowerManager supportPowerMngr;
 		PlayerResources playerResource;
-		readonly BuildingInfo rallypointTestBuilding;		// temporary hack
+		BuildingInfo rallypointTestBuilding;
 		internal readonly HackyAIInfo Info;
 
 		string[] resourceTypes;
 
 		RushFuzzy rushFuzzy = new RushFuzzy();
 
-		Cache<Player,Enemy> aggro = new Cache<Player, Enemy>( _ => new Enemy() );
+		Cache<Player, Enemy> aggro = new Cache<Player, Enemy>(_ => new Enemy());
 		BaseBuilder[] builders;
 
+		List<Squad> squads = new List<Squad>();
+		List<Actor> unitsHangingAroundTheBase = new List<Actor>();
+
+		// Units that the ai already knows about. Any unit not on this list needs to be given a role.
+		List<Actor> activeUnits = new List<Actor>();
+
 		const int MaxBaseDistance = 40;
 		public const int feedbackTime = 30;		// ticks; = a bit over 1s. must be >= netlag.
 
 		public World world { get { return p.PlayerActor.World; } }
 		IBotInfo IBot.Info { get { return this.Info; } }
 
-		public HackyAI(HackyAIInfo Info)
+		public HackyAI(HackyAIInfo info)
 		{
-			this.Info = Info;
+			Info = info;
-			// temporary hack.
+
-			this.rallypointTestBuilding = Rules.Info[Info.RallypointTestBuilding].Traits.Get<BuildingInfo>();
+			// Temporary hack.
+			rallypointTestBuilding = Rules.Info[Info.RallypointTestBuilding].Traits.Get<BuildingInfo>();
 		}
 
 		public static void BotDebug(string s, params object[] args)
@@ -124,7 +123,7 @@ namespace OpenRA.Mods.RA.AI
 				Game.Debug(s, args);
 		}
 
-		/* called by the host's player creation code */
+		// Called by the host's player creation code
 		public void Activate(Player p)
 		{
 			this.p = p;
@@ -133,8 +132,9 @@ namespace OpenRA.Mods.RA.AI
 			supportPowerMngr = p.PlayerActor.Trait<SupportPowerManager>();
 			playerResource = p.PlayerActor.Trait<PlayerResources>();
 			builders = new BaseBuilder[] {
-				new BaseBuilder( this, "Building", q => ChooseBuildingToBuild(q, false) ),
+				new BaseBuilder(this, "Building", q => ChooseBuildingToBuild(q, false)),
-				new BaseBuilder( this, "Defense", q => ChooseBuildingToBuild(q, true) ) };
+				new BaseBuilder(this, "Defense", q => ChooseBuildingToBuild(q, true))
+			};
 
 			random = new XRandom((int)p.PlayerActor.ActorID);
 
@@ -145,24 +145,24 @@ namespace OpenRA.Mods.RA.AI
 		int GetPowerProvidedBy(ActorInfo building)
 		{
 			var bi = building.Traits.GetOrDefault<BuildingInfo>();
-			if (bi == null) return 0;
+			return bi != null ? bi.Power : 0;
-			return bi.Power;
 		}
 
 		ActorInfo ChooseRandomUnitToBuild(ProductionQueue queue)
 		{
 			var buildableThings = queue.BuildableItems();
-			if (!buildableThings.Any()) return null;
+			if (!buildableThings.Any())
+				return null;
+
 			var unit = buildableThings.ElementAtOrDefault(random.Next(buildableThings.Count()));
-			if (HasAdequateAirUnits(unit))
+			return HasAdequateAirUnits(unit) ? unit : null;
-				return unit;
-			return null;
 		}
 
 		ActorInfo ChooseUnitToBuild(ProductionQueue queue)
 		{
 			var buildableThings = queue.BuildableItems();
-			if (!buildableThings.Any()) return null;
+			if (!buildableThings.Any())
+				return null;
 
 			var myUnits = p.World
 				.ActorsWithTrait<IPositionable>()
@@ -180,26 +180,33 @@ namespace OpenRA.Mods.RA.AI
 
 		int CountBuilding(string frac, Player owner)
 		{
-			return world.ActorsWithTrait<Building>().Where(a => a.Actor.Owner == owner && a.Actor.Info.Name == frac).Count();
+			return world.ActorsWithTrait<Building>()
+				.Where(a => a.Actor.Owner == owner && a.Actor.Info.Name == frac)
+				.Count();
 		}
 
 		int CountUnits(string unit, Player owner)
 		{
-			return world.ActorsWithTrait<IPositionable>().Where(a => a.Actor.Owner == owner && a.Actor.Info.Name == unit).Count();
+			return world.ActorsWithTrait<IPositionable>()
+				.Where(a => a.Actor.Owner == owner && a.Actor.Info.Name == unit)
+				.Count();
 		}
 
 		int? CountBuildingByCommonName(string commonName, Player owner)
 		{
-			if(Info.BuildingCommonNames.ContainsKey(commonName))
+			if (!Info.BuildingCommonNames.ContainsKey(commonName))
-				return world.ActorsWithTrait<Building>()
+				return null;
-					.Where(a => a.Actor.Owner == owner && Info.BuildingCommonNames[commonName].Contains(a.Actor.Info.Name)).Count();
+
-			return null;
+			return world.ActorsWithTrait<Building>()
+				.Where(a => a.Actor.Owner == owner && Info.BuildingCommonNames[commonName].Contains(a.Actor.Info.Name))
+				.Count();
 		}
 
 		ActorInfo GetBuildingInfoByCommonName(string commonName, Player owner)
 		{
 			if (commonName == "ConstructionYard")
 				return Rules.Info.Where(k => Info.BuildingCommonNames[commonName].Contains(k.Key)).Random(random).Value;
+
 			return GetInfoByCommonName(Info.BuildingCommonNames, commonName, owner);
 		}
 
@@ -210,52 +217,52 @@ namespace OpenRA.Mods.RA.AI
 
 		ActorInfo GetInfoByCommonName(Dictionary<string, string[]> names, string commonName, Player owner)
 		{
-			if (!names.Any() || !names.ContainsKey(commonName)) return null;
+			if (!names.Any() || !names.ContainsKey(commonName))
+				return null;
+
 			return Rules.Info.Where(k => names[commonName].Contains(k.Key) &&
-				k.Value.Traits.Get<BuildableInfo>().Owner.Contains(owner.Country.Race)).Random(random).Value; //random is shit
+				k.Value.Traits.Get<BuildableInfo>().Owner.Contains(owner.Country.Race)).Random(random).Value;
 		}
 
 		bool HasAdequatePower()
 		{
-			/* note: CNC `fact` provides a small amount of power. don't get jammed because of that. */
+			// note: CNC `fact` provides a small amount of power. don't get jammed because of that.
 			return playerPower.PowerProvided > 50 &&
 				playerPower.PowerProvided > playerPower.PowerDrained * 1.2;
 		}
 
 		bool HasAdequateFact()
 		{
-			if (CountBuildingByCommonName("ConstructionYard", p) == 0 && CountBuildingByCommonName("VehiclesFactory", p) > 0)
+			// Require at least one construction yard, unless we have no vehicles factory (can't build it).
-				return false;
+			return CountBuildingByCommonName("ConstructionYard", p) > 0 ||
-			return true;
+				CountBuildingByCommonName("VehiclesFactory", p) == 0;
 		}
 
 		bool HasAdequateProc()
 		{
-			if (CountBuildingByCommonName("Refinery", p) == 0 && CountBuildingByCommonName("Power", p) > 0)
+			// Require at least one refinery, unless we have no power (can't build it).
-				return false;
+			return CountBuildingByCommonName("Refinery", p) > 0 ||
-			return true;
+				CountBuildingByCommonName("Power", p) == 0;
 		}
 
 		bool HasMinimumProc()
 		{
-			if (CountBuildingByCommonName("Refinery", p) < 2 && CountBuildingByCommonName("Power", p) > 0 &&
+			// Require at least two refineries, unless we have no power (can't build it)
-				CountBuildingByCommonName("Barracks",p) > 0)
+			// or barracks (higher priority?)
-				return false;
+			return CountBuildingByCommonName("Refinery", p) >= 2 ||
-			return true;
+				CountBuildingByCommonName("Power", p) == 0 ||
+				CountBuildingByCommonName("Barracks", p) == 0;
 		}
 
 		bool HasAdequateNumber(string frac, Player owner)
 		{
 			if (Info.BuildingLimits.ContainsKey(frac))
-				if (CountBuilding(frac, owner) < Info.BuildingLimits[frac])
+				return CountBuilding(frac, owner) < Info.BuildingLimits[frac];
-					return true;
-				else
-					return false;
 
 			return true;
 		}
 
-		//for mods like RA (number of building must match the number of aircraft)
+		// For mods like RA (number of building must match the number of aircraft)
 		bool HasAdequateAirUnits(ActorInfo actorInfo)
 		{
 			if (!actorInfo.Traits.Contains<ReloadsInfo>() && actorInfo.Traits.Contains<LimitedAmmoInfo>() 
@@ -266,6 +273,7 @@ namespace OpenRA.Mods.RA.AI
 				if (countOwnAir >= countBuildings)
 					return false;
 			}
+
 			return true;
 		}
 
@@ -275,8 +283,8 @@ namespace OpenRA.Mods.RA.AI
 
 			if (!isDefense)
 			{
-				if (!HasAdequatePower())	/* try to maintain 20% excess power */
+				// Try to maintain 20% excess power
-					/* find the best thing we can build which produces power */
+				if (!HasAdequatePower())
 					return buildableThings.Where(a => GetPowerProvidedBy(a) > 0)
 						.OrderByDescending(a => GetPowerProvidedBy(a)).FirstOrDefault();
 
@@ -286,10 +294,12 @@ namespace OpenRA.Mods.RA.AI
 				if (!HasAdequateProc() || !HasMinimumProc())
 					return GetBuildingInfoByCommonName("Refinery", p);
 			}
+
 			var myBuildings = p.World
 				.ActorsWithTrait<Building>()
-				.Where( a => a.Actor.Owner == p )
+				.Where(a => a.Actor.Owner == p)
-				.Select(a => a.Actor.Info.Name).ToArray();
+				.Select(a => a.Actor.Info.Name)
+				.ToArray();
 
 			foreach (var frac in Info.BuildingFractions)
 				if (buildableThings.Any(b => b.Name == frac.Key))
@@ -314,25 +324,28 @@ namespace OpenRA.Mods.RA.AI
 
 		public CPos? ChooseBuildLocation(string actorType, bool distanceToBaseIsImportant, int maxBaseDistance, BuildingType type)
 		{
-			var bi = Rules.Info[actorType].Traits.Get<BuildingInfo>();
+			var bi = Rules.Info[actorType].Traits.GetOrDefault<BuildingInfo>();
-			if (bi == null) return null;
+			if (bi == null)
+				return null;
 
-			Func<WPos, CPos, CPos?> findPos = (WPos pos, CPos center) =>
+			Func<WPos, CPos, CPos?> findPos = (pos, center) =>
 			{
 				for (var k = MaxBaseDistance; k >= 0; k--)
 				{
 					var tlist = world.FindTilesInCircle(center, k)
 						.OrderBy(a => (a.CenterPosition - pos).LengthSquared);
+
 					foreach (var t in tlist)
 						if (world.CanPlaceBuilding(actorType, bi, t, null))
 							if (bi.IsCloseEnoughToBase(world, p, actorType, t))
 								if (NoBuildingsUnder(Util.ExpandFootprint(FootprintUtils.Tiles(actorType, bi, t), false)))
 									return t;
 				}
+
 				return null;
 			};
 
-			switch(type)
+			switch (type)
 			{
 				case BuildingType.Defense:
 					Actor enemyBase = FindEnemyBuildingClosestToPos(baseCenter.CenterPosition);
@@ -346,19 +359,25 @@ namespace OpenRA.Mods.RA.AI
 
 				case BuildingType.Building:
 					for (var k = 0; k < maxBaseDistance; k++)
+					{
 						foreach (var t in world.FindTilesInCircle(baseCenter, k))
+						{
 							if (world.CanPlaceBuilding(actorType, bi, t, null))
 							{
-								if (distanceToBaseIsImportant)
+								if (distanceToBaseIsImportant && !bi.IsCloseEnoughToBase(world, p, actorType, t))
-									if (!bi.IsCloseEnoughToBase(world, p, actorType, t))
+									continue;
-										continue;
+
 								if (NoBuildingsUnder(Util.ExpandFootprint(FootprintUtils.Tiles(actorType, bi, t), false)))
 									return t;
 							}
+						}
+					}
+
 					break;
 			}
 
-			return null;		// i don't know where to put it.
+			// Can't find a build location
+			return null;		
 		}
 
 		public void Tick(Actor self)
@@ -384,9 +403,11 @@ namespace OpenRA.Mods.RA.AI
 
 		internal Actor ChooseEnemyTarget()
 		{
+			if (p.WinState != WinState.Undefined)
+				return null;
+
 			var liveEnemies = world.Players
-				.Where(q => p != q && p.Stances[q] == Stance.Enemy)
+				.Where(q => p != q && p.Stances[q] == Stance.Enemy && q.WinState == WinState.Undefined);
-				.Where(q => p.WinState == WinState.Undefined && q.WinState == WinState.Undefined);
 
 			if (!liveEnemies.Any())
 				return null;
@@ -396,19 +417,13 @@ namespace OpenRA.Mods.RA.AI
 				.OrderByDescending(g => g.Key)
 				.FirstOrDefault();
 
-			Player enemy;
+			var enemy = (leastLikedEnemies != null) ?
-			if (leastLikedEnemies == null)
+				leastLikedEnemies.Random(random) : liveEnemies.FirstOrDefault();
-				enemy = liveEnemies.FirstOrDefault();
-			else
-				enemy = leastLikedEnemies.Random(random);
 
-			/* pick something worth attacking owned by that player */
+			// Pick something worth attacking owned by that player
-			var targets = world.Actors
+			var target = world.Actors
-				.Where(a => a.Owner == enemy && a.HasTrait<IOccupySpace>());
+				.Where(a => a.Owner == enemy && a.HasTrait<IOccupySpace>())
-			Actor target = null;
+				.ClosestTo(baseCenter.CenterPosition);
-
-			if (targets.Any())
-				target = targets.ClosestTo(baseCenter.CenterPosition);
 
 			if (target == null)
 			{
@@ -419,7 +434,7 @@ namespace OpenRA.Mods.RA.AI
 				return null;
 			}
 
-			/* bump the aggro slightly to avoid changing our mind */
+			// Bump the aggro slightly to avoid changing our mind
 			if (leastLikedEnemies.Count() > 1)
 				aggro[enemy].Aggro++;
 
@@ -434,17 +449,19 @@ namespace OpenRA.Mods.RA.AI
 
 			if (allEnemyUnits.Count > 0)
 				return allEnemyUnits.ClosestTo(pos);
+
 			return null;
 		}
 
 		internal Actor FindClosestEnemy(WPos pos, WRange radius)
 		{
 			var enemyUnits = world.FindActorsInCircle(pos, radius)
-								.Where(unit => p.Stances[unit.Owner] == Stance.Enemy &&
+				.Where(unit => p.Stances[unit.Owner] == Stance.Enemy &&
-									!unit.HasTrait<Husk>() && unit.HasTrait<ITargetable>()).ToList();
+					!unit.HasTrait<Husk>() && unit.HasTrait<ITargetable>()).ToList();
 
 			if (enemyUnits.Count > 0)
 				return enemyUnits.ClosestTo(pos);
+
 			return null;
 		}
 
@@ -452,30 +469,26 @@ namespace OpenRA.Mods.RA.AI
 		{
 			var bases = world.Actors.Where(a => p.Stances[a.Owner] == Stance.Enemy && !a.Destroyed
 				&& a.HasTrait<BaseBuilding>() && !a.HasTrait<Mobile>()).ToList();
-			return bases != null ? bases : new List<Actor>();
+
+			return bases ?? NoActors;
 		}
 
 		Actor FindEnemyBuildingClosestToPos(WPos pos)
 		{
 			var closestBuilding = world.Actors.Where(a => p.Stances[a.Owner] == Stance.Enemy
 			   && !a.Destroyed && a.HasTrait<Building>()).ClosestTo(pos);
+
 			return closestBuilding;
 		}
 
-		List<Squad> squads = new List<Squad>();
-
-		List<Actor> unitsHangingAroundTheBase = new List<Actor>();
-		//Units that the ai already knows about. Any unit not on this list needs to be given a role.
-		List<Actor> activeUnits = new List<Actor>();
-
 		void CleanSquads()
 		{
 			squads.RemoveAll(s => s.IsEmpty);
-			foreach (Squad squad in squads)
+			foreach (var s in squads)
-				squad.units.RemoveAll(a => a.Destroyed || a.IsDead());
+				s.units.RemoveAll(a => a.Destroyed || a.IsDead());
 		}
 
-		//use of this function requires that one squad of this type. Hence it is a piece of shit
+		// Use of this function requires that one squad of this type. Hence it is a piece of shit
 		Squad GetSquadOfType(SquadType type)
 		{
 			return squads.Where(s => s.type == type).FirstOrDefault();
@@ -513,8 +526,8 @@ namespace OpenRA.Mods.RA.AI
 
 			if (--assignRolesTicks > 0)
 				return;
-			else
+
-				assignRolesTicks = Info.AssignRolesInterval;
+			assignRolesTicks = Info.AssignRolesInterval;
 
 			GiveOrdersToIdleHarvesters();
 			FindNewUnits(self);
@@ -528,17 +541,21 @@ namespace OpenRA.Mods.RA.AI
 			foreach (var a in activeUnits)
 			{
 				var harv = a.TraitOrDefault<Harvester>();
-				if (harv == null) continue;
+				if (harv == null)
+					continue;
 
 				if (!a.IsIdle)
 				{
-					Activity act = a.GetCurrentActivity();
+					var act = a.GetCurrentActivity();
+
 					// A Wait activity is technically idle:
 					if ((act.GetType() != typeof(OpenRA.Mods.RA.Activities.Wait)) &&
 						(act.NextActivity == null || act.NextActivity.GetType() != typeof(OpenRA.Mods.RA.Activities.FindResources)))
 						continue;
 				}
-				if (!harv.IsEmpty) continue;
+
+				if (!harv.IsEmpty)
+					continue;
 
 				// Tell the idle harvester to quit slacking:
 				world.IssueOrder(new Order("Harvest", a, false));
@@ -549,8 +566,8 @@ namespace OpenRA.Mods.RA.AI
 		{
 			var newUnits = self.World.ActorsWithTrait<IPositionable>()
 				.Where(a => a.Actor.Owner == p && !a.Actor.HasTrait<BaseBuilding>()
-			&& !activeUnits.Contains(a.Actor))
+					&& !activeUnits.Contains(a.Actor))
-			.Select(a => a.Actor).ToArray();
+				.Select(a => a.Actor);
 
 			foreach (var a in newUnits)
 			{
@@ -559,6 +576,7 @@ namespace OpenRA.Mods.RA.AI
 					world.IssueOrder(new Order("Harvest", a, false));
 				else
 					unitsHangingAroundTheBase.Add(a);
+
 				if (a.HasTrait<Aircraft>() && a.HasTrait<AttackBase>())
 				{
 					var air = GetSquadOfType(SquadType.Air);
@@ -567,14 +585,15 @@ namespace OpenRA.Mods.RA.AI
 
 					air.units.Add(a);
 				}
+
 				activeUnits.Add(a);
 			}  
 		}
 
 		void CreateAttackForce()
 		{
-			/* Create an attack force when we have enough units around our base. */
+			// Create an attack force when we have enough units around our base.
-			// (don't bother leaving any behind for defense.)
+			// (don't bother leaving any behind for defense)
 			var randomizedSquadSize = Info.SquadSize + random.Next(30);
 
 			if (unitsHangingAroundTheBase.Count >= randomizedSquadSize)
@@ -584,6 +603,7 @@ namespace OpenRA.Mods.RA.AI
 				foreach (var a in unitsHangingAroundTheBase)
 					if (!a.HasTrait<Aircraft>())
 						attackForce.units.Add(a);
+
 				unitsHangingAroundTheBase.Clear();
 			}
 		}
@@ -593,7 +613,10 @@ namespace OpenRA.Mods.RA.AI
 			var allEnemyBaseBuilder = FindEnemyConstructionYards();
 			var ownUnits = activeUnits
 				.Where(unit => unit.HasTrait<AttackBase>() && !unit.HasTrait<Aircraft>() && unit.IsIdle).ToList();
-			if (!allEnemyBaseBuilder.Any() || (ownUnits.Count < Info.SquadSize)) return;
+
+			if (!allEnemyBaseBuilder.Any() || (ownUnits.Count < Info.SquadSize))
+				return;
+
 			foreach (var b in allEnemyBaseBuilder)
 			{
 				var enemys = world.FindActorsInCircle(b.CenterPosition, WRange.FromCells(15))
@@ -623,11 +646,13 @@ namespace OpenRA.Mods.RA.AI
 
 			if (!protectSq.TargetIsValid)
 				protectSq.Target = attacker;
+
 			if (protectSq.IsEmpty)
 			{
 				var ownUnits = world.FindActorsInCircle(baseCenter.CenterPosition, WRange.FromCells(15))
-									.Where(unit => unit.Owner == p && !unit.HasTrait<Building>()
+					.Where(unit => unit.Owner == p && !unit.HasTrait<Building>()
-										&& unit.HasTrait<AttackBase>()).ToList();
+						&& unit.HasTrait<AttackBase>()).ToList();
+
 				foreach (var a in ownUnits)
 					protectSq.units.Add(a);
 			}
@@ -635,8 +660,8 @@ namespace OpenRA.Mods.RA.AI
 
 		bool IsRallyPointValid(CPos x)
 		{
-			// this is actually WRONG as soon as HackyAI is building units with a variety of
+			// This is actually WRONG as soon as HackyAI is building units with
-			// movement capabilities. (has always been wrong)
+			// a variety of movement capabilities. (has always been wrong)
 			return world.IsCellBuildable(x, rallypointTestBuilding);
 		}
 
@@ -651,17 +676,16 @@ namespace OpenRA.Mods.RA.AI
 					p.PlayerName, buildings.Length);
 
 			foreach (var a in buildings)
-			{
+				world.IssueOrder(new Order("SetRallyPoint", a.Actor, false) { TargetLocation = ChooseRallyLocationNear(a.Actor.Location) });
-				CPos newRallyPoint = ChooseRallyLocationNear(a.Actor.Location);
-				world.IssueOrder(new Order("SetRallyPoint", a.Actor, false) { TargetLocation = newRallyPoint });
-			}
 		}
 
-		//won't work for shipyards...
+		// Won't work for shipyards...
 		CPos ChooseRallyLocationNear(CPos startPos)
 		{
-			var possibleRallyPoints = world.FindTilesInCircle(startPos, 8).Where(IsRallyPointValid).ToArray();
+			var possibleRallyPoints = world.FindTilesInCircle(startPos, 8)
-			if (possibleRallyPoints.Length == 0)
+				.Where(IsRallyPointValid);
+
+			if (!possibleRallyPoints.Any())
 			{
 				BotDebug("Bot Bug: No possible rallypoint near {0}", startPos);
 				return startPos;
@@ -672,7 +696,7 @@ namespace OpenRA.Mods.RA.AI
 
 		void DeployMcv(Actor self)
 		{
-			/* find our mcv and deploy it */
+			// Find and deploy our mcv
 			var mcv = self.World.Actors
 				.FirstOrDefault(a => a.Owner == p && a.HasTrait<BaseBuilding>());
 
@@ -680,7 +704,8 @@ namespace OpenRA.Mods.RA.AI
 			{
 				baseCenter = mcv.Location;
 				defenseCenter = baseCenter;
-				//Don't transform the mcv if it is a fact
+
+				// Don't transform the mcv if it is a fact
 				if (mcv.HasTrait<Mobile>())
 					world.IssueOrder(new Order("DeployTransform", mcv, false));
 			}
@@ -714,16 +739,18 @@ namespace OpenRA.Mods.RA.AI
 
 		void TryToUseSupportPower(Actor self)
 		{
-			if (supportPowerMngr == null) return;
+			if (supportPowerMngr == null)
-			var powers = supportPowerMngr.Powers.Where(p => !p.Value.Disabled);
+				return;
 
+			var powers = supportPowerMngr.Powers.Where(p => !p.Value.Disabled);
 			foreach (var kv in powers)
 			{
 				var sp = kv.Value;
 				if (sp.Ready)
 				{
 					var attackLocation = FindAttackLocationToSupportPower(5);
-					if (attackLocation == null) return;
+					if (attackLocation == null)
+						return;
 
 					world.IssueOrder(new Order(sp.Info.OrderName, supportPowerMngr.self, false) { TargetLocation = attackLocation.Value });
 				}
@@ -733,27 +760,31 @@ namespace OpenRA.Mods.RA.AI
 		CPos? FindAttackLocationToSupportPower(int radiusOfPower)
 		{
 			CPos? resLoc = null;
-			int countUnits = 0;
+			var countUnits = 0;
 
-			int x = (world.Map.MapSize.X % radiusOfPower) == 0 ? world.Map.MapSize.X : world.Map.MapSize.X + radiusOfPower;
+			var x = (world.Map.MapSize.X % radiusOfPower) == 0 ? world.Map.MapSize.X : world.Map.MapSize.X + radiusOfPower;
-			int y = (world.Map.MapSize.Y % radiusOfPower) == 0 ? world.Map.MapSize.Y : world.Map.MapSize.Y + radiusOfPower;
+			var y = (world.Map.MapSize.Y % radiusOfPower) == 0 ? world.Map.MapSize.Y : world.Map.MapSize.Y + radiusOfPower;
 
 			for (int i = 0; i < x; i += radiusOfPower * 2)
+			{
 				for (int j = 0; j < y; j += radiusOfPower * 2)
 				{
-					CPos pos = new CPos(i, j);
+					var pos = new CPos(i, j);
 					var targets = world.FindActorsInCircle(pos.CenterPosition, WRange.FromCells(radiusOfPower)).ToList();
 					var enemys = targets.Where(unit => p.Stances[unit.Owner] == Stance.Enemy).ToList();
 					var ally = targets.Where(unit => p.Stances[unit.Owner] == Stance.Ally || unit.Owner == p).ToList();
 
 					if (enemys.Count < ally.Count || !enemys.Any())
 						continue;
+
 					if (enemys.Count > countUnits)
 					{
 						countUnits = enemys.Count;
 						resLoc = enemys.Random(random).Location;
 					}
 				}
+			}
+
 			return resLoc;
 		}
 
@@ -766,34 +797,28 @@ namespace OpenRA.Mods.RA.AI
 
 		void ProductionUnits(Actor self)
 		{
-			if (!HasAdequateProc()) /* Stop building until economy is back on */
+			// Stop building until economy is restored
+			if (!HasAdequateProc())
 				return;
-			if (!HasAdequateFact())
+
-				if (!self.World.Actors.Where(a => a.Owner == p && a.HasTrait<BaseBuilding>() && a.HasTrait<Mobile>()).Any())
+			// No construction yards - Build a new MCV
-					BuildUnit("Vehicle", GetUnitInfoByCommonName("Mcv",p).Name);
+			if (!HasAdequateFact() && !self.World.Actors.Where(a => a.Owner == p && a.HasTrait<BaseBuilding>() && a.HasTrait<Mobile>()).Any())
+				BuildUnit("Vehicle", GetUnitInfoByCommonName("Mcv", p).Name);
+
 			foreach (var q in Info.UnitQueues)
-			{
+				BuildUnit(q, unitsHangingAroundTheBase.Count < 12);
-				if (unitsHangingAroundTheBase.Count < 12)
-				{
-					BuildUnit(q, true);
-					continue;
-				}
-				BuildUnit(q, false);
-			}
 		}
 
 		void BuildUnit(string category, bool buildRandom)
 		{
 			// Pick a free queue
-			var queue = FindQueues(category).FirstOrDefault( q => q.CurrentItem() == null );
+			var queue = FindQueues(category).FirstOrDefault(q => q.CurrentItem() == null);
 			if (queue == null)
 				return;
 
-			ActorInfo unit;
+			var unit = buildRandom ?
-			if(buildRandom)
+				ChooseRandomUnitToBuild(queue) : 
-				unit = ChooseRandomUnitToBuild(queue);
+				ChooseUnitToBuild(queue);
-			else
-				unit = ChooseUnitToBuild(queue);
 
 			if (unit != null && Info.UnitsToBuild.Any(u => u.Key == unit.Name))
 				world.IssueOrder(Order.StartProduction(queue.self, unit.Name, 1));
@@ -801,19 +826,25 @@ namespace OpenRA.Mods.RA.AI
 
 		void BuildUnit(string category, string name)
 		{
-			var queue = FindQueues(category).FirstOrDefault( q => q.CurrentItem() == null );
+			var queue = FindQueues(category).FirstOrDefault(q => q.CurrentItem() == null);
-			if (queue == null) return;
+			if (queue == null)
-			if(Rules.Info[name] != null)
+				return;
+
+			if (Rules.Info[name] != null)
 				world.IssueOrder(Order.StartProduction(queue.self, name, 1));
 		}
 
 		public void Damaged(Actor self, AttackInfo e)
 		{
-			if (!enabled) return;
+			// TODO: Surely we want to do this even if their destroyer died?
-			if (e.Attacker.Destroyed) return;
+			if (!enabled || e.Attacker.Destroyed)
-			if (!e.Attacker.HasTrait<ITargetable>()) return;
+				return;
+
+			if (!e.Attacker.HasTrait<ITargetable>())
+				return;
 
 			if (Info.ShouldRepairBuildings && self.HasTrait<RepairableBuilding>())
+			{
 				if (e.DamageState > DamageState.Light && e.PreviousDamageState <= DamageState.Light)
 				{
 					BotDebug("Bot noticed damage {0} {1}->{2}, repairing.",
@@ -821,11 +852,12 @@ namespace OpenRA.Mods.RA.AI
 					world.IssueOrder(new Order("RepairBuilding", self.Owner.PlayerActor, false)
 						{ TargetActor = self });
 				}
+			}
 
 			if (e.Attacker != null && e.Damage > 0)
 				aggro[e.Attacker.Owner].Aggro += e.Damage;
 
-			//protected harvesters or building
+			// Protected harvesters or building
 			if ((self.HasTrait<Harvester>() || self.HasTrait<Building>()) &&
 			    p.Stances[e.Attacker.Owner] == Stance.Enemy)
 			{