fix: weaver test for abstract methods (#2166)

* weaver test for abstract methods * doing nothing is method is abstract * not calling base if it is abstract * adding tests for message to make sure they work
MirageNet · Aug 18, 2020 · 3a276b4 · 3a276b4
1 parent 7294c42
commit 3a276b4
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 8 deletions.
diff --git a/Assets/Mirror/Editor/Weaver/Processors/MessageClassProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/MessageClassProcessor.cs
@@ -33,7 +33,8 @@ static void GenerateSerialization(TypeDefinition td)
         {
             Weaver.DLog(td, "  GenerateSerialization");
             MethodDefinition existingMethod = td.GetMethod("Serialize");
-            if (existingMethod != null && !existingMethod.Body.IsEmptyDefault())
+            // do nothing if method exists and is abstract or not empty
+            if (existingMethod != null && (existingMethod.IsAbstract || !existingMethod.Body.IsEmptyDefault()))
             {
                 return;
             }
@@ -111,21 +112,27 @@ static void CallWriter(ILProcessor worker, FieldDefinition field)
         static void CallBase(TypeDefinition td, ILProcessor worker, string name)
         {
             MethodReference method = Resolvers.TryResolveMethodInParents(td.BaseType, Weaver.CurrentAssembly, name);
-            if (method != null)
+
+            // dont call method if it is null or abstract
+            if (method == null || method.Resolve().IsAbstract)
             {
-                // base
-                worker.Append(worker.Create(OpCodes.Ldarg_0));
-                // writer
-                worker.Append(worker.Create(OpCodes.Ldarg_1));
-                worker.Append(worker.Create(OpCodes.Call, method));
+                return;
             }
+
+            // base
+            worker.Append(worker.Create(OpCodes.Ldarg_0));
+            // writer
+            worker.Append(worker.Create(OpCodes.Ldarg_1));
+            worker.Append(worker.Create(OpCodes.Call, method));
         }
 
         static void GenerateDeSerialization(TypeDefinition td)
         {
             Weaver.DLog(td, "  GenerateDeserialization");
             MethodDefinition existingMethod = td.GetMethod("Deserialize");
-            if (existingMethod != null && !existingMethod.Body.IsEmptyDefault())
+
+            // do nothing if method exists and is abstract or not empty
+            if (existingMethod != null && (existingMethod.IsAbstract || !existingMethod.Body.IsEmptyDefault()))
             {
                 return;
             }

diff --git a/Assets/Mirror/Tests/Editor/MessageBaseTests.cs b/Assets/Mirror/Tests/Editor/MessageBaseTests.cs
@@ -49,6 +49,38 @@ class ArrayIntIMessage : IMessageBase
         public void Serialize(NetworkWriter writer) { }
     }
 
+    abstract class AbstractMessage : IMessageBase
+    {
+        public abstract void Deserialize(NetworkReader reader);
+        public abstract void Serialize(NetworkWriter writer);
+    }
+
+    class OverrideMessage : AbstractMessage
+    {
+        public int someValue;
+
+        // Mirror will fill out these empty methods
+        public override void Serialize(NetworkWriter writer) { }
+        public override void Deserialize(NetworkReader reader) { }
+    }
+
+    class Layer1Message : IMessageBase
+    {
+        public int value1;
+
+        // Mirror will fill out these empty methods
+        public virtual void Serialize(NetworkWriter writer) { }
+        public virtual void Deserialize(NetworkReader reader) { }
+    }
+
+    class Layer2Message : Layer1Message
+    {
+        public int value2;
+    }
+    class Layer3Message : Layer2Message
+    {
+        public int value3;
+    }
 
     [TestFixture]
     public class MessageBaseTests
@@ -88,5 +120,42 @@ public void TestIntArrayInIMessageBase()
             Assert.That(unpacked.array, Is.EquivalentTo(new int[] { 3, 4, 5 }));
         }
 
+        [Test]
+        public void AbstractBaseClassWorks()
+        {
+            const int value = 10;
+            OverrideMessage intMessage = new OverrideMessage
+            {
+                someValue = value
+            };
+
+            byte[] data = MessagePacker.Pack(intMessage);
+
+            OverrideMessage unpacked = MessagePacker.Unpack<OverrideMessage>(data);
+
+            Assert.That(unpacked.someValue, Is.EqualTo(value));
+        }
+
+        [Test]
+        public void MessageInheirtanceWorksWithMultipleLayers()
+        {
+            const int value1 = 10;
+            const int value2 = 13;
+            const int value3 = 15;
+            Layer3Message intMessage = new Layer3Message
+            {
+                value1 = value1,
+                value2 = value2,
+                value3 = value3
+            };
+
+            byte[] data = MessagePacker.Pack(intMessage);
+
+            Layer3Message unpacked = MessagePacker.Unpack<Layer3Message>(data);
+
+            Assert.That(unpacked.value1, Is.EqualTo(value1));
+            Assert.That(unpacked.value2, Is.EqualTo(value2));
+            Assert.That(unpacked.value3, Is.EqualTo(value3));
+        }
     }
 }
diff --git a/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj b/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj
@@ -122,6 +122,7 @@
     <Compile Include="WeaverCommandTests~\ErrorForOptionalNetworkConnectionThatIsNotSenderConnection.cs" />
     <Compile Include="WeaverGeneralTests~\RecursionCount.cs" />
     <Compile Include="WeaverGeneralTests~\TestingScriptableObjectArraySerialization.cs" />
+    <Compile Include="WeaverMessageTests~\AbstractMessageMethods.cs" />
     <Compile Include="WeaverMessageTests~\MessageMemberGeneric.cs" />
     <Compile Include="WeaverMessageTests~\MessageMemberInterface.cs" />
     <Compile Include="WeaverMessageTests~\MessageNestedInheritance.cs" />

diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverMessageTests.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverMessageTests.cs
@@ -41,6 +41,12 @@ public void MessageNestedInheritance()
         {
             Assert.That(weaverErrors, Is.Empty);
         }
+
+        [Test]
+        public void AbstractMessageMethods()
+        {
+            Assert.That(weaverErrors, Is.Empty);
+        }
     }
 }
 
diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverMessageTests~/AbstractMessageMethods.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverMessageTests~/AbstractMessageMethods.cs
@@ -0,0 +1,20 @@
+using Mirror;
+
+namespace WeaverMessageTests.AbstractMessageMethods
+{
+    abstract class AbstractMessage : IMessageBase
+    {
+        public abstract void Deserialize(NetworkReader reader);
+        public abstract void Serialize(NetworkWriter writer);
+    }
+
+    class OverrideMessage : AbstractMessage
+    {
+        public int someValue;
+
+        // Mirror will fill out these empty methods
+
+        public override void Serialize(NetworkWriter writer) { }
+        public override void Deserialize(NetworkReader reader) { }
+    }
+}