From e28eb2a344450bfeb5fb1551de647340a3cc3da1 Mon Sep 17 00:00:00 2001 From: Kitty Draper <284434+ShadauxCat@users.noreply.github.com> Date: Tue, 23 Aug 2022 18:20:50 -0500 Subject: [PATCH] fix: Adding FastBuffer extension methods for multiple instantiations of a generic type causes runtime errors [MTT-3063] (#2142) Fixed RPC codegen failing to choose the correct extension methods for FastBufferReader and FastBufferWriter when the parameters were a generic type (i.e., List) and extensions for multiple instantiations of that type have been defined (i.e., List and List) --- com.unity.netcode.gameobjects/CHANGELOG.md | 6 + .../Editor/CodeGen/NetworkBehaviourILPP.cs | 7 +- .../Runtime/RpcUserSerializableTypesTest.cs | 302 +++++++++++++++++- 3 files changed, 295 insertions(+), 20 deletions(-) diff --git a/com.unity.netcode.gameobjects/CHANGELOG.md b/com.unity.netcode.gameobjects/CHANGELOG.md index 627975e120..9ab71d3968 100644 --- a/com.unity.netcode.gameobjects/CHANGELOG.md +++ b/com.unity.netcode.gameobjects/CHANGELOG.md @@ -7,6 +7,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) Additional documentation and release notes are available at [Multiplayer Documentation](https://docs-multiplayer.unity3d.com). +## [Unreleased] + +### Fixed + +- Fixed RPC codegen failing to choose the correct extension methods for FastBufferReader and FastBufferWriter when the parameters were a generic type (i.e., List) and extensions for multiple instantiations of that type have been defined (i.e., List and List) (#2142) + ## [1.0.1] - 2022-08-23 ### Changed diff --git a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs index 8f8305c1de..01a2e88d10 100644 --- a/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs +++ b/com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs @@ -669,7 +669,7 @@ private bool GetWriteMethodForParameter(TypeReference paramType, out MethodRefer { if (parameters[1].IsIn) { - if (parameters[1].ParameterType.Resolve() == paramType.MakeByReferenceType().Resolve() && + if (((ByReferenceType)parameters[1].ParameterType).ElementType.FullName == paramType.FullName && ((ByReferenceType)parameters[1].ParameterType).ElementType.IsArray == paramType.IsArray) { methodRef = method; @@ -679,8 +679,7 @@ private bool GetWriteMethodForParameter(TypeReference paramType, out MethodRefer } else { - - if (parameters[1].ParameterType.Resolve() == paramType.Resolve() && + if (parameters[1].ParameterType.FullName == paramType.FullName && parameters[1].ParameterType.IsArray == paramType.IsArray) { methodRef = method; @@ -813,7 +812,7 @@ private bool GetReadMethodForParameter(TypeReference paramType, out MethodRefere var parameters = method.Resolve().Parameters; if (method.Name == k_ReadValueMethodName && parameters[1].IsOut && - parameters[1].ParameterType.Resolve() == paramType.MakeByReferenceType().Resolve() && + ((ByReferenceType)parameters[1].ParameterType).ElementType.FullName == paramType.FullName && ((ByReferenceType)parameters[1].ParameterType).ElementType.IsArray == paramType.IsArray) { methodRef = method; diff --git a/testproject/Assets/Tests/Runtime/RpcUserSerializableTypesTest.cs b/testproject/Assets/Tests/Runtime/RpcUserSerializableTypesTest.cs index b39bf0f675..01deda38b4 100644 --- a/testproject/Assets/Tests/Runtime/RpcUserSerializableTypesTest.cs +++ b/testproject/Assets/Tests/Runtime/RpcUserSerializableTypesTest.cs @@ -225,19 +225,24 @@ public IEnumerator ExtensionMethodRpcTest() var obj = new MyObject(256); var obj2 = new MySharedObjectReferencedById(256); var obj3 = new MyObjectPassedWithThisRef(256); + var intList = new List { 5, 10, 15, 5, 1 }; + var strList = new List { "foo", "bar", "baz", "qux" }; bool clientMyObjCalled = false; bool clientMyObjPassedWithThisRefCalled = false; - bool clientMySharedObjCalled = true; + bool clientMySharedObjCalled = false; bool serverMyObjCalled = false; bool serverMyObjPassedWithThisRefCalled = false; - bool serverMySharedObjCalled = true; + bool serverMySharedObjCalled = false; + bool serverIntListCalled = false; + bool serverStrListCalled = false; clientSideNetworkBehaviourClass.OnMyObjectUpdated = (receivedObj) => { Assert.AreEqual(obj.I, receivedObj.I); Assert.AreNotSame(obj, receivedObj); clientMyObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; serverSideNetworkBehaviourClass.OnMyObjectUpdated = (receivedObj) => { @@ -245,7 +250,8 @@ public IEnumerator ExtensionMethodRpcTest() Assert.AreNotSame(obj, receivedObj); serverMyObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; clientSideNetworkBehaviourClass.OnMyObjectPassedWithThisRefUpdated = (receivedObj) => { @@ -253,7 +259,8 @@ public IEnumerator ExtensionMethodRpcTest() Assert.AreNotSame(obj, receivedObj); clientMyObjPassedWithThisRefCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; serverSideNetworkBehaviourClass.OnMyObjectPassedWithThisRefUpdated = (receivedObj) => { @@ -261,26 +268,55 @@ public IEnumerator ExtensionMethodRpcTest() Assert.AreNotSame(obj, receivedObj); serverMyObjPassedWithThisRefCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; clientSideNetworkBehaviourClass.OnMySharedObjectReferencedByIdUpdated = (receivedObj) => { Assert.AreSame(obj2, receivedObj); clientMySharedObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; serverSideNetworkBehaviourClass.OnMySharedObjectReferencedByIdUpdated = (receivedObj) => { Assert.AreSame(obj2, receivedObj); serverMySharedObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; + }; + serverSideNetworkBehaviourClass.OnIntListUpdated = (receivedList) => + { + Assert.AreEqual(intList.Count, receivedList.Count); + for (var i = 0; i < receivedList.Count; ++i) + { + Assert.AreEqual(intList[i], receivedList[i]); + } + serverIntListCalled = true; + m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; + }; + serverSideNetworkBehaviourClass.OnStringListUpdated = (receivedList) => + { + Assert.AreEqual(strList.Count, receivedList.Count); + for (var i = 0; i < receivedList.Count; ++i) + { + Assert.AreEqual(strList[i], receivedList[i]); + } + serverStrListCalled = true; + m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; clientSideNetworkBehaviourClass.SendMyObjectServerRpc(obj); clientSideNetworkBehaviourClass.SendMySharedObjectReferencedByIdServerRpc(obj2); clientSideNetworkBehaviourClass.SendMyObjectPassedWithThisRefServerRpc(obj3); + clientSideNetworkBehaviourClass.SendIntListServerRpc(intList); + clientSideNetworkBehaviourClass.SendStringListServerRpc(strList); // Wait until the test has finished or we time out var timeOutPeriod = Time.realtimeSinceStartup + 5; @@ -329,12 +365,16 @@ public IEnumerator ExtensionMethodArrayRpcTest() var objs = new[] { new MyObject(256), new MyObject(512) }; var objs2 = new[] { new MySharedObjectReferencedById(256), new MySharedObjectReferencedById(512) }; var objs3 = new[] { new MyObjectPassedWithThisRef(256), new MyObjectPassedWithThisRef(512) }; + var intList = new[] { new List { 5, 10, 15 }, new List { 5, 1 } }; + var strList = new[] { new List { "foo", "bar" }, new List { "baz", "qux" }, new List { "quuz" } }; bool clientMyObjCalled = false; bool clientMyObjPassedWithThisRefCalled = false; - bool clientMySharedObjCalled = true; + bool clientMySharedObjCalled = false; bool serverMyObjCalled = false; bool serverMyObjPassedWithThisRefCalled = false; - bool serverMySharedObjCalled = true; + bool serverMySharedObjCalled = false; + bool serverIntListCalled = false; + bool serverStrListCalled = false; clientSideNetworkBehaviourClass.OnMyObjectUpdated = (receivedObjs) => { Assert.AreEqual(receivedObjs.Length, objs2.Length); @@ -345,7 +385,8 @@ public IEnumerator ExtensionMethodArrayRpcTest() } clientMyObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; serverSideNetworkBehaviourClass.OnMyObjectUpdated = (receivedObjs) => { @@ -357,7 +398,8 @@ public IEnumerator ExtensionMethodArrayRpcTest() } serverMyObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; clientSideNetworkBehaviourClass.OnMyObjectPassedWithThisRefUpdated = (receivedObjs) => { @@ -369,7 +411,8 @@ public IEnumerator ExtensionMethodArrayRpcTest() } clientMyObjPassedWithThisRefCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; serverSideNetworkBehaviourClass.OnMyObjectPassedWithThisRefUpdated = (receivedObjs) => { @@ -381,7 +424,8 @@ public IEnumerator ExtensionMethodArrayRpcTest() } serverMyObjPassedWithThisRefCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; clientSideNetworkBehaviourClass.OnMySharedObjectReferencedByIdUpdated = (receivedObjs) => { @@ -392,7 +436,8 @@ public IEnumerator ExtensionMethodArrayRpcTest() } clientMySharedObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; serverSideNetworkBehaviourClass.OnMySharedObjectReferencedByIdUpdated = (receivedObjs) => { @@ -403,12 +448,47 @@ public IEnumerator ExtensionMethodArrayRpcTest() } serverMySharedObjCalled = true; m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && - serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled; + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; + }; + serverSideNetworkBehaviourClass.OnIntListUpdated = (receivedLists) => + { + Assert.AreEqual(receivedLists.Length, intList.Length); + for (var i = 0; i < receivedLists.Length; ++i) + { + Assert.AreEqual(receivedLists[i].Count, intList[i].Count); + for (var j = 0; j < receivedLists[i].Count; ++j) + { + Assert.AreEqual(intList[i][j], receivedLists[i][j]); + } + } + serverIntListCalled = true; + m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; + }; + serverSideNetworkBehaviourClass.OnStringListUpdated = (receivedLists) => + { + Assert.AreEqual(receivedLists.Length, strList.Length); + for (var i = 0; i < receivedLists.Length; ++i) + { + Assert.AreEqual(receivedLists[i].Count, strList[i].Count); + for (var j = 0; j < receivedLists[i].Count; ++j) + { + Assert.AreEqual(strList[i][j], receivedLists[i][j]); + } + } + serverStrListCalled = true; + m_FinishedTest = clientMyObjCalled && clientMySharedObjCalled && clientMyObjPassedWithThisRefCalled && + serverMyObjCalled && serverMySharedObjCalled && serverMyObjPassedWithThisRefCalled && + serverIntListCalled && serverStrListCalled; }; clientSideNetworkBehaviourClass.SendMyObjectServerRpc(objs); clientSideNetworkBehaviourClass.SendMySharedObjectReferencedByIdServerRpc(objs2); clientSideNetworkBehaviourClass.SendMyObjectPassedWithThisRefServerRpc(objs3); + clientSideNetworkBehaviourClass.SendIntListServerRpc(intList); + clientSideNetworkBehaviourClass.SendStringListServerRpc(strList); // Wait until the test has finished or we time out var timeOutPeriod = Time.realtimeSinceStartup + 5; @@ -736,6 +816,11 @@ public class TestSerializationComponent : NetworkBehaviour public delegate void OnMyObjectUpdatedDelgateHandler(MyObject obj); public OnMyObjectUpdatedDelgateHandler OnMyObjectUpdated; + public delegate void OnIntListUpdatedDelgateHandler(List lst); + public OnIntListUpdatedDelgateHandler OnIntListUpdated; + public delegate void OnStringListUpdatedDelgateHandler(List lst); + public OnStringListUpdatedDelgateHandler OnStringListUpdated; + public delegate void OnMyObjectPassedWithThisRefUpdatedDelgateHandler(MyObjectPassedWithThisRef obj); public OnMyObjectPassedWithThisRefUpdatedDelgateHandler OnMyObjectPassedWithThisRefUpdated; @@ -934,6 +1019,22 @@ public void SendMyObjectClientRpc(MyObject obj) } } [ClientRpc] + public void SendIntListClientRpc(List lst) + { + if (OnIntListUpdated != null) + { + OnIntListUpdated.Invoke(lst); + } + } + [ClientRpc] + public void SendStringListClientRpc(List lst) + { + if (OnStringListUpdated != null) + { + OnStringListUpdated.Invoke(lst); + } + } + [ClientRpc] public void SendMyObjectPassedWithThisRefClientRpc(MyObjectPassedWithThisRef obj) { if (OnMyObjectPassedWithThisRefUpdated != null) @@ -961,6 +1062,26 @@ public void SendMyObjectServerRpc(MyObject obj) SendMyObjectClientRpc(obj); } + [ServerRpc] + public void SendIntListServerRpc(List lst) + { + if (OnIntListUpdated != null) + { + OnIntListUpdated.Invoke(lst); + } + SendIntListClientRpc(lst); + } + + [ServerRpc] + public void SendStringListServerRpc(List lst) + { + if (OnStringListUpdated != null) + { + OnStringListUpdated.Invoke(lst); + } + SendStringListClientRpc(lst); + } + [ServerRpc] public void SendMyObjectPassedWithThisRefServerRpc(MyObjectPassedWithThisRef obj) { @@ -998,6 +1119,10 @@ public class TestCustomTypesArrayComponent : NetworkBehaviour public delegate void OnMyObjectUpdatedDelgateHandler(MyObject[] obj); public OnMyObjectUpdatedDelgateHandler OnMyObjectUpdated; + public delegate void OnIntListUpdatedDelgateHandler(List[] obj); + public OnIntListUpdatedDelgateHandler OnIntListUpdated; + public delegate void OnStringListUpdatedDelgateHandler(List[] obj); + public OnStringListUpdatedDelgateHandler OnStringListUpdated; public delegate void OnMyObjectPassedWithThisRefUpdatedDelgateHandler(MyObjectPassedWithThisRef[] obj); public OnMyObjectPassedWithThisRefUpdatedDelgateHandler OnMyObjectPassedWithThisRefUpdated; @@ -1093,6 +1218,24 @@ public void SendMyObjectClientRpc(MyObject[] objs) } } + [ClientRpc] + public void SendIntListClientRpc(List[] lists) + { + if (OnIntListUpdated != null) + { + OnIntListUpdated.Invoke(lists); + } + } + + [ClientRpc] + public void SendStringListClientRpc(List[] lists) + { + if (OnStringListUpdated != null) + { + OnStringListUpdated.Invoke(lists); + } + } + [ClientRpc] public void SendMyObjectPassedWithThisRefClientRpc(MyObjectPassedWithThisRef[] objs) { @@ -1121,6 +1264,26 @@ public void SendMyObjectServerRpc(MyObject[] objs) SendMyObjectClientRpc(objs); } + [ServerRpc] + public void SendIntListServerRpc(List[] lists) + { + if (OnIntListUpdated != null) + { + OnIntListUpdated.Invoke(lists); + } + SendIntListClientRpc(lists); + } + + [ServerRpc] + public void SendStringListServerRpc(List[] lists) + { + if (OnStringListUpdated != null) + { + OnStringListUpdated.Invoke(lists); + } + SendStringListClientRpc(lists); + } + [ServerRpc] public void SendMyObjectPassedWithThisRefServerRpc(MyObjectPassedWithThisRef[] objs) { @@ -1259,6 +1422,7 @@ public static void WriteValueSafe(this FastBufferWriter writer, MySharedObjectRe { writer.WriteValueSafe(value.I); } + public static void ReadValueSafe(this FastBufferReader reader, out MyObject[] values) { reader.ReadValueSafe(out int length); @@ -1269,6 +1433,112 @@ public static void ReadValueSafe(this FastBufferReader reader, out MyObject[] va } } + public static void ReadValueSafe(this FastBufferReader reader, out List value) + { + reader.ReadValueSafe(out int length); + value = new List(); + for (var i = 0; i < length; ++i) + { + reader.ReadValueSafe(out int val); + value.Add(val); + } + } + + //Serialization write for a List of ints + public static void WriteValueSafe(this FastBufferWriter writer, in List value) + { + writer.WriteValueSafe(value.Count); + foreach (var item in value) + { + writer.WriteValueSafe(item); + } + } + + //Serialization read for a List of strings + public static void ReadValueSafe(this FastBufferReader reader, out List value) + { + reader.ReadValueSafe(out int length); + value = new List(); + for (var i = 0; i < length; ++i) + { + reader.ReadValueSafe(out string val); + value.Add(val); + } + } + + //Serialization write for a List of strings + public static void WriteValueSafe(this FastBufferWriter writer, in List value) + { + writer.WriteValueSafe(value.Count); + foreach (var item in value) + { + writer.WriteValueSafe(item); + } + } + + public static void ReadValueSafe(this FastBufferReader reader, out List[] value) + { + reader.ReadValueSafe(out int length); + value = new List[length]; + for (var i = 0; i < length; ++i) + { + reader.ReadValueSafe(out int oneLength); + + value[i] = new List(); + for (var j = 0; j < oneLength; ++j) + { + reader.ReadValueSafe(out int val); + value[i].Add(val); + } + } + } + + //Serialization write for a List of ints + public static void WriteValueSafe(this FastBufferWriter writer, in List[] value) + { + writer.WriteValueSafe(value.Length); + foreach (var item in value) + { + writer.WriteValueSafe(item.Count); + foreach (var subItem in item) + { + writer.WriteValueSafe(subItem); + } + } + } + + //Serialization read for a List of strings + public static void ReadValueSafe(this FastBufferReader reader, out List[] value) + { + reader.ReadValueSafe(out int length); + value = new List[length]; + for (var i = 0; i < length; ++i) + { + reader.ReadValueSafe(out int oneLength); + + value[i] = new List(); + for (var j = 0; j < oneLength; ++j) + { + reader.ReadValueSafe(out string val); + value[i].Add(val); + } + } + } + + //Serialization write for a List of strings + public static void WriteValueSafe(this FastBufferWriter writer, in List[] value) + { + writer.WriteValueSafe(value.Length); + foreach (var item in value) + { + writer.WriteValueSafe(item.Count); + foreach (var subItem in item) + { + writer.WriteValueSafe(subItem); + } + } + } + public static void WriteValueSafe(this FastBufferWriter writer, in MyObject[] values) { writer.WriteValueSafe(values.Length);