From c91308fb0461e54292940ce6fa42bb6cd9800d89 Mon Sep 17 00:00:00 2001
From: James Frowen <jamesfrowendev@gmail.com>
Date: Tue, 30 Jun 2020 08:58:50 +0100
Subject: [PATCH] fix: sync events can not have the same name if they are in
 different classes (#2054)

* test for sync event with same name

* using full name instead of name

* fix test
---
 .../Processors/PropertySiteProcessor.cs       |  7 +-
 .../Weaver/Processors/SyncEventProcessor.cs   |  2 +-
 .../Tests/Editor/SyncEventSameNameTest.cs     | 68 +++++++++++++++++++
 .../Editor/SyncEventSameNameTest.cs.meta      | 11 +++
 4 files changed, 83 insertions(+), 5 deletions(-)
 create mode 100644 Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs
 create mode 100644 Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs.meta

diff --git a/Assets/Mirror/Editor/Weaver/Processors/PropertySiteProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/PropertySiteProcessor.cs
index 208d09b41..610984247 100644
--- a/Assets/Mirror/Editor/Weaver/Processors/PropertySiteProcessor.cs
+++ b/Assets/Mirror/Editor/Weaver/Processors/PropertySiteProcessor.cs
@@ -188,8 +188,7 @@ static void ProcessInstructionMethod(MethodDefinition md, Instruction instr, Met
             // so the earlier instruction that loads the event field is replaced with a Noop.
 
             // go backwards until find a ldfld instruction that matches ANY event
-            bool found = false;
-            while (iCount > 0 && !found)
+            while (iCount > 0)
             {
                 iCount -= 1;
                 Instruction inst = md.Body.Instructions[iCount];
@@ -200,11 +199,11 @@ static void ProcessInstructionMethod(MethodDefinition md, Instruction instr, Met
                     // find replaceEvent with matching name
                     // NOTE: original weaver compared .Name, not just the MethodDefinition,
                     //       that's why we use dict<string,method>.
-                    if (Weaver.WeaveLists.replaceEvents.TryGetValue(opField.Name, out MethodDefinition replacement))
+                    if (Weaver.WeaveLists.replaceEvents.TryGetValue(opField.FullName, out MethodDefinition replacement))
                     {
                         instr.Operand = replacement;
                         inst.OpCode = OpCodes.Nop;
-                        found = true;
+                        return;
                     }
                 }
             }
diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncEventProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncEventProcessor.cs
index 1325c17a8..92b5f57ca 100644
--- a/Assets/Mirror/Editor/Weaver/Processors/SyncEventProcessor.cs
+++ b/Assets/Mirror/Editor/Weaver/Processors/SyncEventProcessor.cs
@@ -146,7 +146,7 @@ public static void ProcessEvents(TypeDefinition td, List<EventDefinition> events
                     td.Methods.Add(eventCallFunc);
 
                     // original weaver compares .Name, not EventDefinition.
-                    Weaver.WeaveLists.replaceEvents[ed.Name] = eventCallFunc;
+                    Weaver.WeaveLists.replaceEvents[ed.FullName] = eventCallFunc;
 
                     Weaver.DLog(td, "  Event: " + ed.Name);
                     break;
diff --git a/Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs b/Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs
new file mode 100644
index 000000000..b8308fce2
--- /dev/null
+++ b/Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs
@@ -0,0 +1,68 @@
+using NUnit.Framework;
+
+
+namespace Mirror.Tests.RemoteAttrributeTest
+{
+    public delegate void SomeEventDelegate(int someNumber);
+
+    class EventBehaviour1 : NetworkBehaviour
+    {
+        [SyncEvent]
+        public event SomeEventDelegate EventWithName;
+
+        public void CallEvent(int i)
+        {
+            EventWithName.Invoke(i);
+        }
+    }
+
+    class EventBehaviour2 : NetworkBehaviour
+    {
+        [SyncEvent]
+        public event SomeEventDelegate EventWithName;
+
+        [SyncEvent]
+        public event SomeEventDelegate EventSecond;
+
+        public void CallEvent(int i)
+        {
+            EventWithName.Invoke(i);
+        }
+    }
+
+    public class SyncEventSameNameTest : RemoteTestBase
+    {
+        [Test]
+        public void EventsWithSameNameCanBothBeCalled()
+        {
+            EventBehaviour1 behaviour1 = CreateHostObject<EventBehaviour1>(true);
+            EventBehaviour2 behaviour2 = CreateHostObject<EventBehaviour2>(true);
+
+            const int someInt1 = 20;
+            const int someInt2 = 21;
+
+            int callCount1 = 0;
+            int callCount2 = 0;
+            behaviour1.EventWithName += incomingInt =>
+            {
+                callCount1++;
+                Assert.That(incomingInt, Is.EqualTo(someInt1));
+            };
+            behaviour2.EventWithName += incomingInt =>
+            {
+                callCount2++;
+                Assert.That(incomingInt, Is.EqualTo(someInt2));
+            };
+
+            behaviour1.CallEvent(someInt1);
+            ProcessMessages();
+            Assert.That(callCount1, Is.EqualTo(1));
+            Assert.That(callCount2, Is.EqualTo(0));
+
+            behaviour2.CallEvent(someInt2);
+            ProcessMessages();
+            Assert.That(callCount1, Is.EqualTo(1));
+            Assert.That(callCount2, Is.EqualTo(1));
+        }
+    }
+}
diff --git a/Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs.meta b/Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs.meta
new file mode 100644
index 000000000..2c7e6c2bd
--- /dev/null
+++ b/Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs.meta
@@ -0,0 +1,11 @@
+fileFormatVersion: 2
+guid: 730736e5ebe5ce149804eb62c0893732
+MonoImporter:
+  externalObjects: {}
+  serializedVersion: 2
+  defaultReferences: []
+  executionOrder: 0
+  icon: {instanceID: 0}
+  userData: 
+  assetBundleName: 
+  assetBundleVariant: