Merge pull request #8858 from RoosterDragon/sync-inherited

Sync members of base classes

OpenRA.Game/Sync.cs

@@ -20,18 +20,22 @@ namespace OpenRA
 {
 	[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
 	public sealed class SyncAttribute : Attribute { }
-	public interface ISync { }	/* marker interface */
+	public interface ISync { } // marker interface
 
 	public static class Sync
 	{
-		static Cache<Type, Func<object, int>> hashFuncCache = new Cache<Type, Func<object, int>>(t => GenerateHashFunc(t));
+		static readonly ConcurrentCache<Type, DynamicMethod> HashMethods =
+			new ConcurrentCache<Type, DynamicMethod>(GenerateHashFunc);
+		static readonly ConcurrentCache<Type, Func<object, int>> HashFunctions =
+			new ConcurrentCache<Type, Func<object, int>>(
+				type => (Func<object, int>)HashMethods[type].CreateDelegate(typeof(Func<object, int>)));
 
 		public static int CalculateSyncHash(object obj)
 		{
-			return hashFuncCache[obj.GetType()](obj);
+			return HashFunctions[obj.GetType()](obj);
 		}
 
-		static Dictionary<Type, MethodInfo> hashFunctions = new Dictionary<Type, MethodInfo>()
+		static readonly Dictionary<Type, MethodInfo> SpecializedHashMethods = new Dictionary<Type, MethodInfo>
 		{
 			{ typeof(int2), ((Func<int2, int>)HashInt2).Method },
 			{ typeof(CPos), ((Func<CPos, int>)HashCPos).Method },
@@ -49,8 +53,8 @@ namespace OpenRA
 
 		static void EmitSyncOpcodes(Type type, ILGenerator il)
 		{
-			if (hashFunctions.ContainsKey(type))
-				il.EmitCall(OpCodes.Call, hashFunctions[type], null);
+			if (SpecializedHashMethods.ContainsKey(type))
+				il.EmitCall(OpCodes.Call, SpecializedHashMethods[type], null);
 			else if (type == typeof(bool))
 			{
 				var l = il.DefineLabel();
@@ -68,9 +72,9 @@ namespace OpenRA
 			il.Emit(OpCodes.Xor);
 		}
 
-		public static Func<object, int> GenerateHashFunc(Type t)
+		static DynamicMethod GenerateHashFunc(Type t)
 		{
-			var d = new DynamicMethod("hash_{0}".F(t.Name), typeof(int), new Type[] { typeof(object) }, t);
+			var d = new DynamicMethod("Hash_{0}".F(t.Name), typeof(int), new Type[] { typeof(object) }, t);
 			var il = d.GetILGenerator();
 			var this_ = il.DeclareLocal(t).LocalIndex;
 			il.Emit(OpCodes.Ldarg_0);
@@ -78,13 +82,17 @@ namespace OpenRA
 			il.Emit(OpCodes.Stloc, this_);
 			il.Emit(OpCodes.Ldc_I4_0);
 
-			const BindingFlags Binding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance;
+			var neededSync = false;
+
+			const BindingFlags Binding = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly;
+
 			foreach (var field in t.GetFields(Binding).Where(x => x.HasAttribute<SyncAttribute>()))
 			{
 				il.Emit(OpCodes.Ldloc, this_);
 				il.Emit(OpCodes.Ldfld, field);
 
 				EmitSyncOpcodes(field.FieldType, il);
+				neededSync = true;
 			}
 
 			foreach (var prop in t.GetProperties(Binding).Where(x => x.HasAttribute<SyncAttribute>()))
@@ -93,10 +101,23 @@ namespace OpenRA
 				il.EmitCall(OpCodes.Call, prop.GetGetMethod(), null);
 
 				EmitSyncOpcodes(prop.PropertyType, il);
+				neededSync = true;
+			}
+
+			if (t.BaseType != null)
+			{
+				var baseHashFunc = HashMethods[t.BaseType];
+				if (baseHashFunc != null)
+				{
+					il.Emit(OpCodes.Ldloc, this_);
+					il.EmitCall(OpCodes.Call, baseHashFunc, null);
+					il.Emit(OpCodes.Xor);
+					neededSync = true;
+				}
 			}
 
 			il.Emit(OpCodes.Ret);
-			return (Func<object, int>)d.CreateDelegate(typeof(Func<object, int>));
+			return neededSync ? d : null;
 		}
 
 		public static int HashInt2(int2 i2)

OpenRA.Test/OpenRA.Game/SyncTest.cs

@@ -0,0 +1,78 @@
+#region Copyright & License Information
+/*
+ * Copyright 2007-2015 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,
+ * see COPYING.
+ */
+#endregion
+
+using NUnit.Framework;
+using OpenRA.Traits;
+
+namespace OpenRA.Test
+{
+	[TestFixture]
+	class SyncTest
+	{
+		class Complex : ISync
+		{
+			[Sync]
+			public bool Bool = true;
+			[Sync]
+			public int Int = -123456789;
+			[Sync]
+			public int2 Int2 = new int2(123, -456);
+			[Sync]
+			public CPos CPos = new CPos(123, -456);
+			[Sync]
+			public CVec CVec = new CVec(123, -456);
+			[Sync]
+			public WDist WDist = new WDist(123);
+			[Sync]
+			public WPos WPos = new WPos(123, -456, int.MaxValue);
+			[Sync]
+			public WVec WVec = new WVec(123, -456, int.MaxValue);
+			[Sync]
+			public WAngle WAngle = new WAngle(123);
+			[Sync]
+			public WRot WRot = new WRot(new WAngle(123), new WAngle(-456), new WAngle(int.MaxValue));
+			[Sync]
+			public Target Target = Target.FromPos(new WPos(123, -456, int.MaxValue));
+		}
+
+		[TestCase(TestName = "Sync hashing has not accidentally changed")]
+		public void ComplexHash()
+		{
+			// If you have intentionally changed the values used for sync hashing, just update the expected value.
+			Assert.AreEqual(-2024026914, Sync.CalculateSyncHash(new Complex()));
+		}
+
+		class Flat : ISync
+		{
+			[Sync]
+			int a = 123456789;
+			[Sync]
+			bool b = true;
+		}
+
+		class Base : ISync
+		{
+			[Sync]
+			bool b = true;
+		}
+
+		class Derived : Base
+		{
+			[Sync]
+			int a = 123456789;
+		}
+
+		[TestCase(TestName = "All sync members in inheritance hierarchy are hashed")]
+		public void DerivedHash()
+		{
+			Assert.AreEqual(Sync.CalculateSyncHash(new Flat()), Sync.CalculateSyncHash(new Derived()));
+		}
+	}
+}

OpenRA.Test/OpenRA.Test.csproj

@@ -49,6 +49,7 @@
     <Compile Include="OpenRA.Game\ActorInfoTest.cs" />
     <Compile Include="OpenRA.Game\OrderTest.cs" />
     <Compile Include="OpenRA.Game\PlatformTest.cs" />
+    <Compile Include="OpenRA.Game\SyncTest.cs" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\OpenRA.Game\OpenRA.Game.csproj">