Refactoring pass.

- Extract an enum for edges rather than using magic numbers for everything.
- Remove duplicated code between FoggedEdges and ShroudedEdges by hosting the visibility function into a delegate.
- Make minimap methods more readable.
- Tidy formatting.
- Make some fields readonly.
- Remove unused usings.

OpenRA.Game/Graphics/Minimap.cs

@@ -36,17 +36,16 @@ namespace OpenRA.Graphics
 
 			unsafe
 			{
-				var c = (int*)bitmapData.Scan0;
+				var colors = (int*)bitmapData.Scan0;
-
+				var stride = bitmapData.Stride / 4;
-				for (var x = 0; x < b.Width; x++)
-				{
 				for (var y = 0; y < b.Height; y++)
+				{
+					for (var x = 0; x < b.Width; x++)
 					{
 						var mapX = x + b.Left;
 						var mapY = y + b.Top;
 						var type = tileset.GetTerrainInfo(mapTiles[mapX, mapY]);
-
+						colors[y * stride + x] = type.Color.ToArgb();
-						*(c + (y * bitmapData.Stride >> 2) + x) = type.Color.ToArgb();
 					}
 				}
 			}
@@ -67,11 +66,11 @@ namespace OpenRA.Graphics
 
 			unsafe
 			{
-				var c = (int*)bitmapData.Scan0;
+				var colors = (int*)bitmapData.Scan0;
-
+				var stride = bitmapData.Stride / 4;
-				for (var x = 0; x < b.Width; x++)
-				{
 				for (var y = 0; y < b.Height; y++)
+				{
+					for (var x = 0; x < b.Width; x++)
 					{
 						var mapX = x + b.Left;
 						var mapY = y + b.Top;
@@ -85,7 +84,7 @@ namespace OpenRA.Graphics
 						if (res == null)
 							continue;
 
-						*(c + (y * bitmapData.Stride >> 2) + x) = tileset[tileset.GetTerrainIndex(res)].Color.ToArgb();
+						colors[y * stride + x] = tileset[tileset.GetTerrainIndex(res)].Color.ToArgb();
 					}
 				}
 			}
@@ -107,19 +106,18 @@ namespace OpenRA.Graphics
 
 			unsafe
 			{
-				var c = (int*)bitmapData.Scan0;
+				var colors = (int*)bitmapData.Scan0;
-
+				var stride = bitmapData.Stride / 4;
-				for (var x = 0; x < b.Width; x++)
-				{
 				for (var y = 0; y < b.Height; y++)
+				{
+					for (var x = 0; x < b.Width; x++)
 					{
 						var mapX = x + b.Left;
 						var mapY = y + b.Top;
 						var custom = map.CustomTerrain[mapX, mapY];
 						if (custom == -1)
 							continue;
-
+						colors[y * stride + x] = world.TileSet[custom].Color.ToArgb();
-						*(c + (y * bitmapData.Stride >> 2) + x) = world.TileSet[custom].Color.ToArgb();
 					}
 				}
 			}
@@ -140,8 +138,8 @@ namespace OpenRA.Graphics
 
 			unsafe
 			{
-				var c = (int*)bitmapData.Scan0;
+				var colors = (int*)bitmapData.Scan0;
-
+				var stride = bitmapData.Stride / 4;
 				foreach (var t in world.ActorsWithTrait<IRadarSignature>())
 				{
 					if (world.FogObscures(t.Actor))
@@ -152,7 +150,7 @@ namespace OpenRA.Graphics
 					{
 						var uv = Map.CellToMap(map.TileShape, cell);
 						if (b.Contains(uv.X, uv.Y))
-							*(c + ((uv.Y - b.Top) * bitmapData.Stride >> 2) + uv.X - b.Left) = color.ToArgb();
+							colors[(uv.Y - b.Top) * stride + uv.X - b.Left] = color.ToArgb();
 					}
 				}
 			}
@@ -180,15 +178,15 @@ namespace OpenRA.Graphics
 
 			unsafe
 			{
-				var c = (int*)bitmapData.Scan0;
+				var colors = (int*)bitmapData.Scan0;
-
+				var stride = bitmapData.Stride / 4;
 				foreach (var cell in map.Cells)
 				{
 					var uv = Map.CellToMap(map.TileShape, cell) - offset;
 					if (world.ShroudObscures(cell))
-						*(c + (uv.Y * bitmapData.Stride >> 2) + uv.X) = shroud;
+						colors[uv.Y * stride + uv.X] = shroud;
 					else if (world.FogObscures(cell))
-						*(c + (uv.Y * bitmapData.Stride >> 2) + uv.X) = fog;
+						colors[uv.Y * stride + uv.X] = fog;
 				}
 			}
 

OpenRA.Game/Map/CellLayer.cs

@@ -12,7 +12,6 @@ using System;
 using System.Collections;
 using System.Collections.Generic;
 using System.Drawing;
-using OpenRA.Graphics;
 
 namespace OpenRA
 {
@@ -21,7 +20,7 @@ namespace OpenRA
 	{
 		public readonly Size Size;
 		public readonly TileShape Shape;
-		T[] entries;
+		readonly T[] entries;
 
 		public CellLayer(Map map)
 			: this(map.TileShape, new Size(map.MapSize.X, map.MapSize.Y)) { }
@@ -37,35 +36,27 @@ namespace OpenRA
 		int Index(CPos cell)
 		{
 			var uv = Map.CellToMap(Shape, cell);
-			return uv.Y * Size.Width + uv.X;
+			return Index(uv.X, uv.Y);
+		}
+
+		// Resolve an array index from map coordinates
+		int Index(int u, int v)
+		{
+			return v * Size.Width + u;
 		}
 
 		/// <summary>Gets or sets the <see cref="OpenRA.CellLayer"/> using cell coordinates</summary>
 		public T this[CPos cell]
 		{
-			get
+			get { return entries[Index(cell)]; }
-			{
+			set { entries[Index(cell)] = value; }
-				return entries[Index(cell)];
-			}
-
-			set
-			{
-				entries[Index(cell)] = value;
-			}
 		}
 
 		/// <summary>Gets or sets the layer contents using raw map coordinates (not CPos!)</summary>
 		public T this[int u, int v]
 		{
-			get
+			get { return entries[Index(u, v)]; }
-			{
+			set { entries[Index(u, v)] = value; }
-				return entries[v * Size.Width + u];
-			}
-
-			set
-			{
-				entries[v * Size.Width + u] = value;
-			}
 		}
 
 		/// <summary>Clears the layer contents with a known value</summary>

OpenRA.Game/Map/CellRegion.cs

@@ -8,11 +8,8 @@
  */
 #endregion
 
-using System;
 using System.Collections;
 using System.Collections.Generic;
-using System.Drawing;
-using OpenRA.Graphics;
 
 namespace OpenRA
 {

OpenRA.Game/Traits/Player/FrozenActorLayer.cs

@@ -1,6 +1,6 @@
 #region Copyright & License Information
 /*
- * Copyright 2007-2013 The OpenRA Developers (see AUTHORS)
+ * Copyright 2007-2014 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. For more information,
@@ -27,7 +27,7 @@ namespace OpenRA.Traits
 		public readonly Rectangle Bounds;
 		readonly Actor actor;
 
-		public IRenderable[] Renderables { set; private get; }
+		public IRenderable[] Renderables { private get; set; }
 		public Player Owner;
 
 		public string TooltipName;
@@ -84,6 +84,7 @@ namespace OpenRA.Traits
 				return Renderables.Concat(Renderables.Where(r => !r.IsDecoration)
 					.Select(r => r.WithPalette(highlight)));
 			}
+
 			return Renderables;
 		}
 

OpenRA.Game/Traits/World/Shroud.cs

@@ -1,6 +1,6 @@
 #region Copyright & License Information
 /*
- * Copyright 2007-2011 The OpenRA Developers (see AUTHORS)
+ * Copyright 2007-2014 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. For more information,
@@ -10,7 +10,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Drawing;
 using System.Linq;
 
 namespace OpenRA.Traits
@@ -22,21 +21,21 @@ namespace OpenRA.Traits
 
 	public class Shroud
 	{
-		[Sync] public bool Disabled = false;
+		[Sync] public bool Disabled;
 
-		Actor self;
+		readonly Actor self;
-		Map map;
+		readonly Map map;
 
-		CellLayer<int> visibleCount;
+		readonly CellLayer<int> visibleCount;
-		CellLayer<int> generatedShroudCount;
+		readonly CellLayer<int> generatedShroudCount;
-		CellLayer<bool> explored;
+		readonly CellLayer<bool> explored;
 
 		readonly Lazy<IFogVisibilityModifier[]> fogVisibilities;
 
 		// Cache of visibility that was added, so no matter what crazy trait code does, it
 		// can't make us invalid.
-		Dictionary<Actor, CPos[]> visibility = new Dictionary<Actor, CPos[]>();
+		readonly Dictionary<Actor, CPos[]> visibility = new Dictionary<Actor, CPos[]>();
-		Dictionary<Actor, CPos[]> generation = new Dictionary<Actor, CPos[]>();
+		readonly Dictionary<Actor, CPos[]> generation = new Dictionary<Actor, CPos[]>();
 
 		public int Hash { get; private set; }
 
@@ -186,14 +185,17 @@ namespace OpenRA.Traits
 
 		public void Explore(World world, CPos center, WRange range)
 		{
-			foreach (var q in FindVisibleTiles(world, center, range))
+			foreach (var c in FindVisibleTiles(world, center, range))
-				explored[q] = true;
+				explored[c] = true;
 
 			Invalidate();
 		}
 
 		public void Explore(Shroud s)
 		{
+			if (map.Bounds != s.map.Bounds)
+				throw new ArgumentException("The map bounds of these shrouds do not match.", "s");
+
 			foreach (var cell in map.Cells)
 				if (s.explored[cell])
 					explored[cell] = true;

OpenRA.Game/World.cs

@@ -10,7 +10,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Drawing;
 using System.Linq;
 using OpenRA.Effects;
 using OpenRA.FileFormats;

OpenRA.Mods.RA/Modifiers/FrozenUnderFog.cs

@@ -1,6 +1,6 @@
 #region Copyright & License Information
 /*
- * Copyright 2007-2013 The OpenRA Developers (see AUTHORS)
+ * Copyright 2007-2014 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. For more information,
@@ -28,13 +28,16 @@ namespace OpenRA.Mods.RA
 	{
 		[Sync] public int VisibilityHash;
 
-		bool initialized, startsRevealed;
+		readonly bool startsRevealed;
 		readonly CPos[] footprint;
-		Lazy<IToolTip> tooltip;
-		Lazy<Health> health;
 
-		Dictionary<Player, bool> visible;
+		readonly Lazy<IToolTip> tooltip;
-		Dictionary<Player, FrozenActor> frozen;
+		readonly Lazy<Health> health;
+
+		readonly Dictionary<Player, bool> visible;
+		readonly Dictionary<Player, FrozenActor> frozen;
+
+		bool initialized;
 
 		public FrozenUnderFog(ActorInitializer init, FrozenUnderFogInfo info)
 		{

OpenRA.Mods.RA/ShroudRenderer.cs

@@ -9,9 +9,6 @@
 #endregion
 
 using System;
-using System.Drawing;
-using System.Linq;
-using OpenRA;
 using OpenRA.Graphics;
 using OpenRA.Traits;
 
@@ -47,6 +44,27 @@ namespace OpenRA.Mods.RA
 
 	public class ShroudRenderer : IRenderShroud, IWorldLoaded
 	{
+		[Flags]
+		enum Edges : byte
+		{
+			None = 0,
+			TopLeft = 0x01,
+			TopRight = 0x02,
+			BottomRight = 0x04,
+			BottomLeft = 0x08,
+			AllCorners = TopLeft | TopRight | BottomRight | BottomLeft,
+			TopSide = 0x10,
+			RightSide = 0x20,
+			BottomSide = 0x40,
+			LeftSide = 0x80,
+			AllSides = TopSide | RightSide | BottomSide | LeftSide,
+			Top = TopSide | TopLeft | TopRight,
+			Right = RightSide | TopRight | BottomRight,
+			Bottom = BottomSide | BottomRight | BottomLeft,
+			Left = LeftSide | TopLeft | BottomLeft,
+			All = Top | Right | Bottom | Left
+		}
+
 		class ShroudTile
 		{
 			public readonly CPos Position;
@@ -70,12 +88,19 @@ namespace OpenRA.Mods.RA
 		readonly CellLayer<ShroudTile> tiles;
 		readonly int variantStride;
 		readonly Map map;
+		readonly Edges notVisibleEdges;
 
 		PaletteReference fogPalette, shroudPalette;
 		int shroudHash;
 
 		public ShroudRenderer(World world, ShroudRendererInfo info)
 		{
+			if (info.ShroudVariants.Length != info.FogVariants.Length)
+				throw new ArgumentException("ShroudRenderer must define the same number of shroud and fog variants!", "info");
+
+			if ((info.OverrideFullFog == null) ^ (info.OverrideFullShroud == null))
+				throw new ArgumentException("ShroudRenderer cannot define overrides for only one of shroud or fog!", "info");
+
 			this.info = info;
 			map = world.Map;
 
@@ -85,12 +110,6 @@ namespace OpenRA.Mods.RA
 			shroudHash = -1;
 
 			// Load sprite variants
-			if (info.ShroudVariants.Length != info.FogVariants.Length)
-				throw new InvalidOperationException("ShroudRenderer must define the same number of shroud and fog variants!");
-
-			if ((info.OverrideFullFog == null) ^ (info.OverrideFullShroud == null))
-				throw new InvalidOperationException("ShroudRenderer cannot define overrides for only one of shroud or fog!");
-
 			var variantCount = info.ShroudVariants.Length;
 			variantStride = info.Index.Length + (info.OverrideFullShroud != null ? 1 : 0);
 			shroudSprites = new Sprite[variantCount * variantStride];
@@ -121,25 +140,27 @@ namespace OpenRA.Mods.RA
 
 			if (info.OverrideFullShroud != null)
 				spriteMap[info.OverrideShroudIndex] = variantStride - 1;
+
+			notVisibleEdges = info.UseExtendedIndex ? Edges.AllSides : Edges.AllCorners;
 		}
 
-		static int FoggedEdges(Shroud s, CPos p, bool useExtendedIndex)
+		Edges GetEdges(int x, int y, Func<int, int, bool> isVisible)
 		{
-			if (!s.IsVisible(p))
+			if (!isVisible(x, y))
-				return useExtendedIndex ? 240 : 15;
+				return notVisibleEdges;
 
 			// If a side is shrouded then we also count the corners
-			var u = 0;
+			var u = Edges.None;
-			if (!s.IsVisible(p + new CVec(0, -1))) u |= 0x13;
+			if (!isVisible(x, y - 1)) u |= Edges.Top;
-			if (!s.IsVisible(p + new CVec(1, 0))) u |= 0x26;
+			if (!isVisible(x + 1, y)) u |= Edges.Right;
-			if (!s.IsVisible(p + new CVec(0, 1))) u |= 0x4C;
+			if (!isVisible(x, y + 1)) u |= Edges.Bottom;
-			if (!s.IsVisible(p + new CVec(-1, 0))) u |= 0x89;
+			if (!isVisible(x - 1, y)) u |= Edges.Left;
 
-			var uside = u & 0x0F;
+			var ucorner = u & Edges.AllCorners;
-			if (!s.IsVisible(p + new CVec(-1, -1))) u |= 0x01;
+			if (!isVisible(x - 1, y - 1)) u |= Edges.TopLeft;
-			if (!s.IsVisible(p + new CVec(1, -1))) u |= 0x02;
+			if (!isVisible(x + 1, y - 1)) u |= Edges.TopRight;
-			if (!s.IsVisible(p + new CVec(1, 1))) u |= 0x04;
+			if (!isVisible(x + 1, y + 1)) u |= Edges.BottomRight;
-			if (!s.IsVisible(p + new CVec(-1, 1))) u |= 0x08;
+			if (!isVisible(x - 1, y + 1)) u |= Edges.BottomLeft;
 
 			// RA provides a set of frames for tiles with shrouded
 			// corners but unshrouded edges. We want to detect this
@@ -147,51 +168,24 @@ namespace OpenRA.Mods.RA
 			// in other combinations. The XOR turns off the corner
 			// bits that are enabled twice, which gives the behavior
 			// we want here.
-			return useExtendedIndex ? u ^ uside : u & 0x0F;
+			return info.UseExtendedIndex ? u ^ ucorner : u & Edges.AllCorners;
 		}
 
-		static int ShroudedEdges(Shroud s, CPos p, bool useExtendedIndex)
+		Edges GetObserverEdges(int x, int y)
 		{
-			if (!s.IsExplored(p))
+			var u = Edges.None;
-				return useExtendedIndex ? 240 : 15;
+			if (!map.Contains(new CPos(x, y - 1))) u |= Edges.Top;
+			if (!map.Contains(new CPos(x + 1, y))) u |= Edges.Right;
+			if (!map.Contains(new CPos(x, y + 1))) u |= Edges.Bottom;
+			if (!map.Contains(new CPos(x - 1, y))) u |= Edges.Left;
 
-			// If a side is shrouded then we also count the corners
+			var ucorner = u & Edges.AllCorners;
-			var u = 0;
+			if (!map.Contains(new CPos(x - 1, y - 1))) u |= Edges.TopLeft;
-			if (!s.IsExplored(p + new CVec(0, -1))) u |= 0x13;
+			if (!map.Contains(new CPos(x + 1, y - 1))) u |= Edges.TopRight;
-			if (!s.IsExplored(p + new CVec(1, 0))) u |= 0x26;
+			if (!map.Contains(new CPos(x + 1, y + 1))) u |= Edges.BottomRight;
-			if (!s.IsExplored(p + new CVec(0, 1))) u |= 0x4C;
+			if (!map.Contains(new CPos(x - 1, y + 1))) u |= Edges.BottomLeft;
-			if (!s.IsExplored(p + new CVec(-1, 0))) u |= 0x89;
 
-			var uside = u & 0x0F;
+			return info.UseExtendedIndex ? u ^ ucorner : u & Edges.AllCorners;
-			if (!s.IsExplored(p + new CVec(-1, -1))) u |= 0x01;
-			if (!s.IsExplored(p + new CVec(1, -1))) u |= 0x02;
-			if (!s.IsExplored(p + new CVec(1, 1))) u |= 0x04;
-			if (!s.IsExplored(p + new CVec(-1, 1))) u |= 0x08;
-
-			// RA provides a set of frames for tiles with shrouded
-			// corners but unshrouded edges. We want to detect this
-			// situation without breaking the edge -> corner enabling
-			// in other combinations. The XOR turns off the corner
-			// bits that are enabled twice, which gives the behavior
-			// we want here.
-			return useExtendedIndex ? u ^ uside : u & 0x0F;
-		}
-
-		static int ObserverShroudedEdges(Map map, CPos p, bool useExtendedIndex)
-		{
-			var u = 0;
-			if (!map.Contains(p + new CVec(0, -1))) u |= 0x13;
-			if (!map.Contains(p + new CVec(1, 0))) u |= 0x26;
-			if (!map.Contains(p + new CVec(0, 1))) u |= 0x4C;
-			if (!map.Contains(p + new CVec(-1, 0))) u |= 0x89;
-
-			var uside = u & 0x0F;
-			if (!map.Contains(p + new CVec(-1, -1))) u |= 0x01;
-			if (!map.Contains(p + new CVec(1, -1))) u |= 0x02;
-			if (!map.Contains(p + new CVec(1, 1))) u |= 0x04;
-			if (!map.Contains(p + new CVec(-1, 1))) u |= 0x08;
-
-			return useExtendedIndex ? u ^ uside : u & 0x0F;
 		}
 
 		public void WorldLoaded(World w, WorldRenderer wr)
@@ -206,22 +200,19 @@ namespace OpenRA.Mods.RA
 
 				// Set the cells outside the border so they don't need to be touched again
 				if (!map.Contains(cell))
-				{
+					tiles[cell].Shroud = GetTile(notVisibleEdges, variant);
-					var index = info.UseExtendedIndex ? 240 : 15;
-					tiles[cell].Shroud = shroudSprites[variant * variantStride + spriteMap[index]];
-				}
 			}
 
 			fogPalette = wr.Palette(info.FogPalette);
 			shroudPalette = wr.Palette(info.ShroudPalette);
 		}
 
-		Sprite GetTile(int flags, int variant)
+		Sprite GetTile(Edges edges, int variant)
 		{
-			if (flags == 0)
+			if (edges == Edges.None)
 				return null;
 
-			return shroudSprites[variant * variantStride + spriteMap[flags]];
+			return shroudSprites[variant * variantStride + spriteMap[(byte)edges]];
 		}
 
 		void Update(Shroud shroud)
@@ -237,22 +228,24 @@ namespace OpenRA.Mods.RA
 				foreach (var cell in map.Cells)
 				{
 					var t = tiles[cell];
-					var shrouded = ObserverShroudedEdges(map, t.Position, info.UseExtendedIndex);
+					var shrouded = GetObserverEdges(cell.X, cell.Y);
 
-					t.Shroud = shrouded != 0 ? shroudSprites[t.Variant * variantStride + spriteMap[shrouded]] : null;
+					t.Shroud = shrouded != 0 ? shroudSprites[t.Variant * variantStride + spriteMap[(byte)shrouded]] : null;
-					t.Fog = shrouded != 0 ? fogSprites[t.Variant * variantStride + spriteMap[shrouded]] : null;
+					t.Fog = shrouded != 0 ? fogSprites[t.Variant * variantStride + spriteMap[(byte)shrouded]] : null;
 				}
 			}
 			else
 			{
 				foreach (var cell in map.Cells)
 				{
+					Func<int, int, bool> visibleUnderShroud = (x, y) => shroud.IsExplored(new CPos(x, y));
+					Func<int, int, bool> visibleUnderFog = (x, y) => shroud.IsVisible(new CPos(x, y));
 					var t = tiles[cell];
-					var shrouded = ShroudedEdges(shroud, t.Position, info.UseExtendedIndex);
+					var shrouded = GetEdges(cell.X, cell.Y, visibleUnderShroud);
-					var fogged = FoggedEdges(shroud, t.Position, info.UseExtendedIndex);
+					var fogged = GetEdges(cell.X, cell.Y, visibleUnderFog);
 
-					t.Shroud = shrouded != 0 ? shroudSprites[t.Variant * variantStride + spriteMap[shrouded]] : null;
+					t.Shroud = shrouded != 0 ? shroudSprites[t.Variant * variantStride + spriteMap[(byte)shrouded]] : null;
-					t.Fog = fogged != 0 ? fogSprites[t.Variant * variantStride + spriteMap[fogged]] : null;
+					t.Fog = fogged != 0 ? fogSprites[t.Variant * variantStride + spriteMap[(byte)fogged]] : null;
 				}
 			}
 		}