From da09e715761cf41551ec89e53b0063c0d3d0df47 Mon Sep 17 00:00:00 2001 From: vis2k Date: Sat, 29 Jan 2022 23:19:13 +0800 Subject: [PATCH] fix: Duplicate IL2CPP hashes when building to WebGL (#3061) (#3072) * fix: Duplicate IL2CPP hashes when building to WebGL (#3061) * change to tryget * Update DistanceInterestManagement.cs * Update MethodProcessor.cs * GenerateMethodName * Update MethodProcessor.cs * Update Assets/Mirror/Editor/Weaver/Weaver.cs * Update Weaver.cs Co-authored-by: vis2k * fix callers to replacement rpcs which used to call the deadlock * improve generated name Co-authored-by: cooper <60411087+cxxpxr@users.noreply.github.com> --- .../Weaver/Processors/CommandProcessor.cs | 4 +- .../Weaver/Processors/MethodProcessor.cs | 54 ++++++++++++------- .../Editor/Weaver/Processors/RpcProcessor.cs | 8 +-- .../Weaver/Processors/TargetRpcProcessor.cs | 4 +- Assets/Mirror/Editor/Weaver/Weaver.cs | 23 ++++++++ 5 files changed, 68 insertions(+), 25 deletions(-) diff --git a/Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs index 0818ac90c..35ba2533f 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/CommandProcessor.cs @@ -78,7 +78,9 @@ protected static void InvokeCmdCmdThrust(NetworkBehaviour obj, NetworkReader rea */ public static MethodDefinition ProcessCommandInvoke(WeaverTypes weaverTypes, Readers readers, Logger Log, TypeDefinition td, MethodDefinition method, MethodDefinition cmdCallFunc, ref bool WeavingFailed) { - MethodDefinition cmd = new MethodDefinition(Weaver.InvokeRpcPrefix + method.Name, + string cmdName = Weaver.GenerateMethodName(Weaver.InvokeRpcPrefix, method); + + MethodDefinition cmd = new MethodDefinition(cmdName, MethodAttributes.Family | MethodAttributes.Static | MethodAttributes.HideBySig, weaverTypes.Import(typeof(void))); diff --git a/Assets/Mirror/Editor/Weaver/Processors/MethodProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/MethodProcessor.cs index d06c96ed2..8a4c581ec 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/MethodProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/MethodProcessor.cs @@ -17,7 +17,8 @@ public static class MethodProcessor // FixRemoteCallToBaseMethod replaces them afterwards. public static MethodDefinition SubstituteMethod(Logger Log, TypeDefinition td, MethodDefinition md, ref bool WeavingFailed) { - string newName = RpcPrefix + md.Name; + string newName = Weaver.GenerateMethodName(RpcPrefix, md); + MethodDefinition cmd = new MethodDefinition(newName, md.Attributes, md.ReturnType); // force the substitute method to be protected. @@ -79,28 +80,43 @@ public static void FixRemoteCallToBaseMethod(Logger Log, TypeDefinition type, Me foreach (Instruction instruction in method.Body.Instructions) { - // if call to base.CmdDoSomething within this.CallCmdDoSomething - if (IsCallToMethod(instruction, out MethodDefinition calledMethod) && - calledMethod.Name == baseRemoteCallName) + // is this instruction a Call to a method? + // if yes, output the method so we can check it. + if (IsCallToMethod(instruction, out MethodDefinition calledMethod)) { - TypeDefinition baseType = type.BaseType.Resolve(); - MethodDefinition baseMethod = baseType.GetMethodInBaseType(callName); - - if (baseMethod == null) + // when considering if 'calledMethod' is a call to 'method', + // we originally compared .Name. + // + // to fix IL2CPP build bugs with overloaded Rpcs, we need to + // generated rpc names like + // RpcTest(string value) => RpcTestString(strig value) + // RpcTest(int value) => RpcTestInt(int value) + // to make them unique. + // + // calledMethod.Name is still "RpcTest", so we need to + // convert this to the generated name as well before comparing. + string calledMethodName_Generated = Weaver.GenerateMethodName("", calledMethod); + if (calledMethodName_Generated == baseRemoteCallName) { - Log.Error($"Could not find base method for {callName}", method); - WeavingFailed = true; - return; - } + TypeDefinition baseType = type.BaseType.Resolve(); + MethodDefinition baseMethod = baseType.GetMethodInBaseType(callName); - if (!baseMethod.IsVirtual) - { - Log.Error($"Could not find base method that was virtual {callName}", method); - WeavingFailed = true; - return; - } + if (baseMethod == null) + { + Log.Error($"Could not find base method for {callName}", method); + WeavingFailed = true; + return; + } - instruction.Operand = baseMethod; + if (!baseMethod.IsVirtual) + { + Log.Error($"Could not find base method that was virtual {callName}", method); + WeavingFailed = true; + return; + } + + instruction.Operand = baseMethod; + } } } } diff --git a/Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs index 5d804acbb..7413ce5b0 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/RpcProcessor.cs @@ -8,10 +8,10 @@ public static class RpcProcessor { public static MethodDefinition ProcessRpcInvoke(WeaverTypes weaverTypes, Writers writers, Readers readers, Logger Log, TypeDefinition td, MethodDefinition md, MethodDefinition rpcCallFunc, ref bool WeavingFailed) { - MethodDefinition rpc = new MethodDefinition( - Weaver.InvokeRpcPrefix + md.Name, - MethodAttributes.Family | MethodAttributes.Static | MethodAttributes.HideBySig, - weaverTypes.Import(typeof(void))); + string rpcName = Weaver.GenerateMethodName(Weaver.InvokeRpcPrefix, md); + + MethodDefinition rpc = new MethodDefinition(rpcName, MethodAttributes.Family | MethodAttributes.Static | MethodAttributes.HideBySig, + weaverTypes.Import(typeof(void))); ILProcessor worker = rpc.Body.GetILProcessor(); Instruction label = worker.Create(OpCodes.Nop); diff --git a/Assets/Mirror/Editor/Weaver/Processors/TargetRpcProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/TargetRpcProcessor.cs index 6f3c76517..fa2e2a28b 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/TargetRpcProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/TargetRpcProcessor.cs @@ -15,7 +15,9 @@ public static bool HasNetworkConnectionParameter(MethodDefinition md) public static MethodDefinition ProcessTargetRpcInvoke(WeaverTypes weaverTypes, Readers readers, Logger Log, TypeDefinition td, MethodDefinition md, MethodDefinition rpcCallFunc, ref bool WeavingFailed) { - MethodDefinition rpc = new MethodDefinition(Weaver.InvokeRpcPrefix + md.Name, MethodAttributes.Family | + string trgName = Weaver.GenerateMethodName(Weaver.InvokeRpcPrefix, md); + + MethodDefinition rpc = new MethodDefinition(trgName, MethodAttributes.Family | MethodAttributes.Static | MethodAttributes.HideBySig, weaverTypes.Import(typeof(void))); diff --git a/Assets/Mirror/Editor/Weaver/Weaver.cs b/Assets/Mirror/Editor/Weaver/Weaver.cs index b748964bf..0a1b7f957 100644 --- a/Assets/Mirror/Editor/Weaver/Weaver.cs +++ b/Assets/Mirror/Editor/Weaver/Weaver.cs @@ -31,6 +31,29 @@ internal class Weaver // multi threaded logging. public Logger Log; + // remote actions now support overloads, + // -> but IL2CPP doesnt like it when two generated methods + // -> have the same signature, + // -> so, append the signature to the generated method name, + // -> to create a unique name + // Example: + // RpcTeleport(Vector3 position) -> InvokeUserCode_RpcTeleport__Vector3() + // RpcTeleport(Vector3 position, Quaternion rotation) -> InvokeUserCode_RpcTeleport__Vector3Quaternion() + // fixes https://github.com/vis2k/Mirror/issues/3060 + public static string GenerateMethodName(string initialPrefix, MethodDefinition md) + { + initialPrefix += md.Name; + + for (int i = 0; i < md.Parameters.Count; ++i) + { + // with __ so it's more obvious that this is the parameter suffix. + // otherwise RpcTest(int) => RpcTestInt(int) which is not obvious. + initialPrefix += $"__{md.Parameters[i].ParameterType.Name}"; + } + + return initialPrefix; + } + public Weaver(Logger Log) { this.Log = Log;