Skip to content

Commit

Permalink
fix: calling base method when the first base class did not have the v…
Browse files Browse the repository at this point in the history
…irtual method (#2014)

* adding test for error

* adding test with multiple overrides

* calling methods in base type

* adding test for more than 1 override
  • Loading branch information
James-Frowen committed Jun 18, 2020
1 parent 5032ceb commit 4af72c3
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 3 deletions.
26 changes: 26 additions & 0 deletions Assets/Mirror/Editor/Weaver/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,32 @@ public static List<MethodDefinition> GetMethods(this TypeDefinition td, string m
return methods;
}

public static MethodDefinition GetMethodInBaseType(this TypeDefinition td, string methodName)
{
TypeDefinition typedef = td;
while (typedef != null)
{
foreach (MethodDefinition md in typedef.Methods)
{
if (md.Name == methodName)
return md;
}

try
{
TypeReference parent = typedef.BaseType;
typedef = parent?.Resolve();
}
catch (AssemblyResolutionException)
{
// this can happen for pluins.
break;
}
}

return null;
}

/// <summary>
///
/// </summary>
Expand Down
14 changes: 13 additions & 1 deletion Assets/Mirror/Editor/Weaver/Processors/MethodProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,19 @@ public static void FixRemoteCallToBaseMethod(TypeDefinition type, MethodDefiniti
calledMethod.Name == baseRemoteCallName)
{
TypeDefinition baseType = type.BaseType.Resolve();
MethodDefinition baseMethod = baseType.GetMethod(callName);
MethodDefinition baseMethod = baseType.GetMethodInBaseType(callName);

if (baseMethod == null)
{
Weaver.Error($"Could not find base method for {callName}", method);
return;
}

if (!baseMethod.IsVirtual)
{
Weaver.Error($"Could not find base method that was virutal {callName}", method);
return;
}

instruction.Operand = baseMethod;

Expand Down
49 changes: 49 additions & 0 deletions Assets/Mirror/Tests/Editor/CommandOverrideTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ public override void CmdSendInt(int someInt)
}
}

/// <summary>
/// test for 2 overrides
/// </summary>
class VirtualOverrideCommandWithBase2 : VirtualOverrideCommandWithBase
{
public event Action<int> onOverrideSendInt2;

[Command]
public override void CmdSendInt(int someInt)
{
base.CmdSendInt(someInt);
onOverrideSendInt2?.Invoke(someInt);
}
}

public class CommandOverrideTest : RemoteTestBase
{
[Test]
Expand Down Expand Up @@ -132,5 +147,39 @@ public void OverrideVirtualWithBaseCallsBothVirtualAndBase()
Assert.That(virtualCallCount, Is.EqualTo(1));
Assert.That(overrideCallCount, Is.EqualTo(1));
}

[Test]
public void OverrideVirtualWithBaseCallsAllMethodsThatCallBase()
{
VirtualOverrideCommandWithBase2 hostBehaviour = CreateHostObject<VirtualOverrideCommandWithBase2>(true);

const int someInt = 20;

int virtualCallCount = 0;
int overrideCallCount = 0;
int override2CallCount = 0;
hostBehaviour.onVirtualSendInt += incomingInt =>
{
virtualCallCount++;
Assert.That(incomingInt, Is.EqualTo(someInt));
};
hostBehaviour.onOverrideSendInt += incomingInt =>
{
overrideCallCount++;
Assert.That(incomingInt, Is.EqualTo(someInt));
};
hostBehaviour.onOverrideSendInt2 += incomingInt =>
{
override2CallCount++;
Assert.That(incomingInt, Is.EqualTo(someInt));
};


hostBehaviour.CmdSendInt(someInt);
ProcessMessages();
Assert.That(virtualCallCount, Is.EqualTo(1));
Assert.That(overrideCallCount, Is.EqualTo(1));
Assert.That(override2CallCount, Is.EqualTo(1));
}
}
}
2 changes: 2 additions & 0 deletions Assets/Mirror/Tests/Editor/Weaver/.WeaverTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
<Compile Include="WeaverCommandTests~\CommandWithArguments.cs" />
<Compile Include="WeaverCommandTests~\OverrideAbstractCommand.cs" />
<Compile Include="WeaverCommandTests~\OverrideVirtualCallBaseCommand.cs" />
<Compile Include="WeaverCommandTests~\OverrideVirtualCallsBaseCommandWithMultipleBaseClasses.cs" />
<Compile Include="WeaverCommandTests~\OverrideVirtualCallsBaseCommandWithOverride.cs" />
<Compile Include="WeaverCommandTests~\OverrideVirtualCommand.cs" />
<Compile Include="WeaverCommandTests~\VirtualCommand.cs" />
<Compile Include="WeaverCommandTests~\CommandWithSenderConnectionAndOtherArgs.cs" />
Expand Down
12 changes: 12 additions & 0 deletions Assets/Mirror/Tests/Editor/Weaver/WeaverCommandTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ public void OverrideVirtualCallBaseCommand()
Assert.That(weaverErrors, Is.Empty);
}

[Test]
public void OverrideVirtualCallsBaseCommandWithMultipleBaseClasses()
{
Assert.That(weaverErrors, Is.Empty);
}

[Test]
public void OverrideVirtualCallsBaseCommandWithOverride()
{
Assert.That(weaverErrors, Is.Empty);
}

[Test]
public void AbstractCommand()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Mirror;


namespace WeaverCommandTests.OverrideVirtualCallBaseCommand
{
class OverrideVirtualCallBaseCommand : baseBehaviour
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
using Mirror;

namespace WeaverCommandTests.OverrideVirtualCallsBaseCommandWithMultipleBaseClasses
{
class OverrideVirtualCallsBaseCommandWithMultipleBaseClasses : middleBehaviour
{
[Command]
protected override void CmdDoSomething()
{
// do somethin
base.CmdDoSomething();
}
}

class middleBehaviour : baseBehaviour
{
}

class baseBehaviour : NetworkBehaviour
{
[Command]
protected virtual void CmdDoSomething()
{
// do more stuff
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using Mirror;

namespace WeaverCommandTests.OverrideVirtualCallsBaseCommandWithOverride
{
class OverrideVirtualCallsBaseCommandWithOverride : middleBehaviour
{
[Command]
protected override void CmdDoSomething()
{
// do something
base.CmdDoSomething();
}
}


class middleBehaviour : baseBehaviour
{
[Command]
protected override void CmdDoSomething()
{
// do something else
base.CmdDoSomething();
}
}

class baseBehaviour : NetworkBehaviour
{
[Command]
protected virtual void CmdDoSomething()
{
// do more stuff
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Mirror;


namespace WeaverCommandTests.OverrideVirtualCommand
{
class OverrideVirtualCommand : baseBehaviour
Expand Down

0 comments on commit 4af72c3

Please sign in to comment.