Skip to content

Commit

Permalink
fix: sync events can not have the same name if they are in different …
Browse files Browse the repository at this point in the history
…classes (#2054)

* test for sync event with same name

* using full name instead of name

* fix test
  • Loading branch information
James-Frowen committed Jun 30, 2020
1 parent d0bb9d5 commit c91308f
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
68 changes: 68 additions & 0 deletions Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
11 changes: 11 additions & 0 deletions Assets/Mirror/Tests/Editor/SyncEventSameNameTest.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit c91308f

Please sign in to comment.