fix(weaver): Warning for multiple write/read functions for the same t…

…ype (#2453) * fix(weaver): Warning for multiple write/read functions for the same type Adding warning message when trying to register a write or read function for a type that is already in dictionary. Previously weaver would be silent about this making it hard to know when an extension method overrides another one. * test for warnings
MirrorNetworking · Nov 22, 2020 · 301e791 · 301e791
1 parent fd9a7b0
commit 301e791
Show file tree

Hide file tree

Showing 5 changed files with 57 additions and 1 deletion.
diff --git a/Assets/Mirror/Editor/Weaver/Readers.cs b/Assets/Mirror/Editor/Weaver/Readers.cs
@@ -17,6 +17,12 @@ public static void Init()
 
         internal static void Register(TypeReference dataType, MethodReference methodReference)
         {
+            string typeName = dataType.FullName;
+            if (readFuncs.ContainsKey(typeName))
+            {
+                Weaver.Warning($"Registering a Read method for {typeName} when one already exists", methodReference);
+            }
+
             readFuncs[dataType.FullName] = methodReference;
         }
 

diff --git a/Assets/Mirror/Editor/Weaver/Writers.cs b/Assets/Mirror/Editor/Weaver/Writers.cs
@@ -17,7 +17,13 @@ public static void Init()
 
         public static void Register(TypeReference dataType, MethodReference methodReference)
         {
-            writeFuncs[dataType.FullName] = methodReference;
+            string typeName = dataType.FullName;
+            if (writeFuncs.ContainsKey(typeName))
+            {
+                Weaver.Warning($"Registering a Write method for {typeName} when one already exists", methodReference);
+            }
+
+            writeFuncs[typeName] = methodReference;
         }
 
         static void RegisterWriteFunc(TypeReference typeReference, MethodDefinition newWriterFunc)

diff --git a/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj b/Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj
@@ -133,6 +133,7 @@
     <Compile Include="WeaverCommandTests~\ErrorForOptionalNetworkConnectionThatIsNotSenderConnection.cs" />
     <Compile Include="WeaverGeneralTests~\RecursionCount.cs" />
     <Compile Include="WeaverGeneralTests~\TestingScriptableObjectArraySerialization.cs" />
+    <Compile Include="WeaverGeneratedReaderWriterTests~\GivesWarningWhenRegisteringExistingExtensionMethod.cs" />
     <Compile Include="WeaverMessageTests~\AbstractMessageMethods.cs" />
     <Compile Include="WeaverMessageTests~\MessageMemberGeneric.cs" />
     <Compile Include="WeaverMessageTests~\MessageMemberInterface.cs" />

diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverGeneratedReaderWriterTests.cs
@@ -203,5 +203,16 @@ public void GivesErrorForInvalidListType()
             //HasError("Cannot generate reader for List because element MonoBehaviour does not have a reader. Use a supported type or provide a custom reader",
             //    "System.Collections.Generic.List`1<UnityEngine.MonoBehaviour>");
         }
+
+        [Test]
+        public void GivesWarningWhenRegisteringExistingExtensionMethod()
+        {
+            const string typeName = "GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.MyType";
+            HasNoErrors();
+            HasWarning($"Registering a Write method for {typeName} when one already exists",
+                "System.Void GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.ReadWriteExtension::WriteMyType2(Mirror.NetworkWriter,GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.MyType)");
+            HasWarning($"Registering a Read method for {typeName} when one already exists",
+                "GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.MyType GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod.ReadWriteExtension::ReadMyType2(Mirror.NetworkReader)");
+        }
     }
 }
diff --git a/...r/WeaverGeneratedReaderWriterTests~/GivesWarningWhenRegisteringExistingExtensionMethod.cs b/...r/WeaverGeneratedReaderWriterTests~/GivesWarningWhenRegisteringExistingExtensionMethod.cs
@@ -0,0 +1,32 @@
+using Mirror;
+
+namespace GeneratedReaderWriter.GivesWarningWhenRegisteringExistingExtensionMethod
+{
+    public struct MyType
+    {
+        public int number;
+    }
+
+    public static class ReadWriteExtension
+    {
+        public static void WriteMyType(this NetworkWriter writer, MyType data)
+        {
+            writer.WriteInt32(data.number);
+        }
+
+        public static void WriteMyType2(this NetworkWriter writer, MyType data)
+        {
+            writer.WriteInt32(data.number);
+        }
+
+        public static MyType ReadMyType(this NetworkReader reader)
+        {
+            return new MyType { number = reader.ReadInt32() };
+        }
+
+        public static MyType ReadMyType2(this NetworkReader reader)
+        {
+            return new MyType { number = reader.ReadInt32() };
+        }
+    }
+}