fix: weaver syncLists now checks for SerializeItem in base class (#1768)

* tests for override in base class

* fixing overrides in base class

* moving check up so that typedef cant be null for the check

Assets/Mirror/Editor/Weaver/Extensions.cs

@@ -180,5 +180,41 @@ public static MethodDefinition GetMethod(this TypeDefinition td, string methodNa
             }
             return null;
         }
+
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="td"></param>
+        /// <param name="methodName"></param>
+        /// <param name="stopAt"></param>
+        /// <returns></returns>
+        public static bool HasMethodInBaseType(this TypeDefinition td, string methodName, TypeReference stopAt)
+        {
+            TypeDefinition typedef = td;
+            while (typedef != null)
+            {
+                if (typedef.FullName == stopAt.FullName)
+                    break;
+
+                foreach (MethodDefinition md in typedef.Methods)
+                {
+                    if (md.Name == methodName)
+                        return true;
+                }
+
+                try
+                {
+                    TypeReference parent = typedef.BaseType;
+                    typedef = parent?.Resolve();
+                }
+                catch (AssemblyResolutionException)
+                {
+                    // this can happen for pluins.
+                    break;
+                }
+            }
+
+            return false;
+        }
     }
 }

Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs

@@ -15,7 +15,7 @@ public static void Process(TypeDefinition td)
 
             if (resolver.GetGenericFromBaseClass(td, 0, Weaver.SyncDictionaryType, out TypeReference keyType))
             {
-                SyncObjectProcessor.GenerateSerialization(td, keyType, "SerializeKey", "DeserializeKey");
+                SyncObjectProcessor.GenerateSerialization(td, keyType, Weaver.SyncDictionaryType, "SerializeKey", "DeserializeKey");
             }
             else
             {
@@ -25,7 +25,7 @@ public static void Process(TypeDefinition td)
 
             if (resolver.GetGenericFromBaseClass(td, 1, Weaver.SyncDictionaryType, out TypeReference itemType))
             {
-                SyncObjectProcessor.GenerateSerialization(td, itemType, "SerializeItem", "DeserializeItem");
+                SyncObjectProcessor.GenerateSerialization(td, itemType, Weaver.SyncDictionaryType, "SerializeItem", "DeserializeItem");
             }
             else
             {

Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs

@@ -9,17 +9,18 @@ static class SyncListProcessor
         /// Generates serialization methods for synclists
         /// </summary>
         /// <param name="td">The synclist class</param>
-        public static void Process(TypeDefinition td, TypeReference baseType)
+        /// <param name="mirrorBaseType">the base SyncObject td inherits from</param>
+        public static void Process(TypeDefinition td, TypeReference mirrorBaseType)
         {
             GenericArgumentResolver resolver = new GenericArgumentResolver(1);
 
-            if (resolver.GetGenericFromBaseClass(td, 0, baseType, out TypeReference itemType))
+            if (resolver.GetGenericFromBaseClass(td, 0, mirrorBaseType, out TypeReference itemType))
             {
-                SyncObjectProcessor.GenerateSerialization(td, itemType, "SerializeItem", "DeserializeItem");
+                SyncObjectProcessor.GenerateSerialization(td, itemType, mirrorBaseType, "SerializeItem", "DeserializeItem");
             }
             else
             {
-                Weaver.Error($"Could not find generic arguments for {baseType} using {td}");
+                Weaver.Error($"Could not find generic arguments for {mirrorBaseType} using {td}");
             }
         }
     }

Assets/Mirror/Editor/Weaver/Processors/SyncObjectProcessor.cs

@@ -9,35 +9,33 @@ public static class SyncObjectProcessor
         /// Generates the serialization and deserialization methods for a specified generic argument
         /// </summary>
         /// <param name="td">The type of the class that needs serialization methods</param>
-        /// <param name="genericArgument">Which generic argument to serialize,  0 is the first one</param>
-        /// <param name="baseType">the type that has generic arguments</param>
+        /// <param name="itemType">generic argument to serialize</param>
+        /// <param name="mirrorBaseType">the base SyncObject td inherits from</param>
         /// <param name="serializeMethod">The name of the serialize method</param>
         /// <param name="deserializeMethod">The name of the deserialize method</param>
-        public static void GenerateSerialization(TypeDefinition td, TypeReference itemType, string serializeMethod, string deserializeMethod)
+        public static void GenerateSerialization(TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType, string serializeMethod, string deserializeMethod)
         {
             Weaver.DLog(td, "SyncObjectProcessor Start item:" + itemType.FullName);
 
-            MethodReference writeItemFunc = GenerateSerialization(serializeMethod, td, itemType);
+            bool success = GenerateSerialization(serializeMethod, td, itemType, mirrorBaseType);
             if (Weaver.WeavingFailed)
             {
                 return;
             }
 
-            MethodReference readItemFunc = GenerateDeserialization(deserializeMethod, td, itemType);
+            success |= GenerateDeserialization(deserializeMethod, td, itemType, mirrorBaseType);
 
-            if (readItemFunc == null || writeItemFunc == null)
-                return;
-
-            Weaver.DLog(td, "SyncObjectProcessor Done");
+            if (success)
+                Weaver.DLog(td, "SyncObjectProcessor Done");
         }
 
         // serialization of individual element
-        static MethodReference GenerateSerialization(string methodName, TypeDefinition td, TypeReference itemType)
+        static bool GenerateSerialization(string methodName, TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType)
         {
             Weaver.DLog(td, "  GenerateSerialization");
-            MethodDefinition existing = td.GetMethod(methodName);
-            if (existing != null)
-                return existing;
+            bool existing = td.HasMethodInBaseType(methodName, mirrorBaseType);
+            if (existing)
+                return true;
 
 
             // this check needs to happen inside GenerateSerialization because
@@ -45,7 +43,7 @@ static MethodReference GenerateSerialization(string methodName, TypeDefinition t
             if (itemType.IsGenericInstance)
             {
                 Weaver.Error($"{td} Can not create Serialize or Deserialize for generic element. Override virtual methods with custom Serialize and Deserialize to use {itemType} in SyncList");
-                return null;
+                return false;
             }
 
             MethodDefinition serializeFunc = new MethodDefinition(methodName, MethodAttributes.Public |
@@ -68,27 +66,27 @@ static MethodReference GenerateSerialization(string methodName, TypeDefinition t
             else
             {
                 Weaver.Error($"{td} cannot have item of type {itemType}.  Use a type supported by mirror instead");
-                return null;
+                return false;
             }
             serWorker.Append(serWorker.Create(OpCodes.Ret));
 
             td.Methods.Add(serializeFunc);
-            return serializeFunc;
+            return true;
         }
 
-        static MethodReference GenerateDeserialization(string methodName, TypeDefinition td, TypeReference itemType)
+        static bool GenerateDeserialization(string methodName, TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType)
         {
             Weaver.DLog(td, "  GenerateDeserialization");
-            MethodDefinition existing = td.GetMethod(methodName);
-            if (existing != null)
-                return existing;
+            bool existing = td.HasMethodInBaseType(methodName, mirrorBaseType);
+            if (existing)
+                return true;
 
             // this check needs to happen inside GenerateDeserialization because
             // we need to check if user has made custom function above
             if (itemType.IsGenericInstance)
             {
                 Weaver.Error($"{td} Can not create Serialize or Deserialize for generic element. Override virtual methods with custom Serialize and Deserialize to use {itemType} in SyncList");
-                return null;
+                return false;
             }
 
             MethodDefinition deserializeFunction = new MethodDefinition(methodName, MethodAttributes.Public |
@@ -111,11 +109,11 @@ static MethodReference GenerateDeserialization(string methodName, TypeDefinition
             else
             {
                 Weaver.Error($"{td} cannot have item of type {itemType}.  Use a type supported by mirror instead");
-                return null;
+                return false;
             }
 
             td.Methods.Add(deserializeFunction);
-            return deserializeFunction;
+            return true;
         }
     }
 }

Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests.cs

@@ -175,6 +175,13 @@ public void SyncListInterfaceWithCustomMethods()
             Assert.That(weaverErrors, Is.Empty);
         }
 
+        [Test]
+        public void SyncListInheritanceWithOverrides()
+        {
+            Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
+            Assert.That(weaverErrors, Is.Empty);
+        }
+
         [Test]
         public void SyncListErrorWhenUsingGenericListInNetworkBehaviour()
         {

Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests~/SyncListInheritanceWithOverrides.cs

@@ -0,0 +1,42 @@
+using Mirror;
+using UnityEngine;
+
+namespace MirrorTest
+{
+    class SyncListInheritanceWithOverrides : NetworkBehaviour
+    {
+        readonly SomeExtraList superSyncListString = new SomeExtraList();
+    }
+
+    // Type that cant have custom writer
+    public class MyBehaviourWithValue : NetworkBehaviour
+    {
+        public Vector3 target;
+    }
+
+    public class SomeBaseList : SyncList<MyBehaviourWithValue>
+    {
+        protected override void SerializeItem(NetworkWriter writer, MyBehaviourWithValue item)
+        {
+            writer.WriteUInt32(item.netId);
+            writer.WriteVector3(item.target);
+        }
+        protected override MyBehaviourWithValue DeserializeItem(NetworkReader reader)
+        {
+            NetworkIdentity item = NetworkIdentity.spawned[reader.ReadUInt32()];
+            MyBehaviourWithValue behaviour = item.GetComponent<MyBehaviourWithValue>();
+
+            behaviour.target = reader.ReadVector3();
+
+            return behaviour;
+        }
+    }
+
+    // Sync List type is MyBehaviourWithValue
+    // MyBehaviourWithValue is an invalid type, so requires custom writers
+    // Custom writers exist in base class so SomeExtraList should work without errors
+    public class SomeExtraList : SomeBaseList
+    {
+        // do extra stuff here
+    }
+}