fix!: NativeList<T> causes crashes when used in RPCs [NCCBUG-106]

com.unity.netcode.gameobjects/CHANGELOG.md

@@ -11,6 +11,8 @@ Additional documentation and release notes are available at [Multiplayer Documen
 
 ### Changed
 
+- `unmanaged` structs are no longer universally accepted as RPC parameters because some structs (i.e., structs with pointers in them, such as `NativeList<T>`) can't be supported by the default memcpy struct serializer. Structs that are intended to be serialized across the network must add `INetworkSerializeByMemcpy` to the interface list (i.e., `struct Foo : INetworkSerializeByMemcpy`). This interface is empty and just serves to mark the struct as compatible with memcpy serialization. For external structs you can't edit, you can pass them to RPCs by wrapping them in `ForceNetworkSerializeByMemcpy<T>`. (#1901)
+
 ### Removed
 
 ### Fixed
@@ -19,6 +21,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
 - Fixed client throwing an exception if it has messages in the outbound queue when processing the `NetworkEvent.Disconnect` event and is using UTP. (#1884)
 - Fixed issue during client synchronization if 'ValidateSceneBeforeLoading' returned false it would halt the client synchronization process resulting in a client that was approved but not synchronized or fully connected with the server. (#1883)
 - Fixed an issue where UNetTransport.StartServer would return success even if the underlying transport failed to start (#854)
+- Passing generic types to RPCs no longer causes a native crash (#1901)
 
 ## [Unreleased]
 

com.unity.netcode.gameobjects/Editor/CodeGen/CodeGenHelpers.cs

@@ -27,6 +27,7 @@ internal static class CodeGenHelpers
         public static readonly string ServerRpcSendParams_FullName = typeof(ServerRpcSendParams).FullName;
         public static readonly string ServerRpcReceiveParams_FullName = typeof(ServerRpcReceiveParams).FullName;
         public static readonly string INetworkSerializable_FullName = typeof(INetworkSerializable).FullName;
+        public static readonly string INetworkSerializeByMemcpy_FullName = typeof(INetworkSerializeByMemcpy).FullName;
         public static readonly string UnityColor_FullName = typeof(Color).FullName;
         public static readonly string UnityColor32_FullName = typeof(Color32).FullName;
         public static readonly string UnityVector2_FullName = typeof(Vector2).FullName;
@@ -77,6 +78,35 @@ public static bool IsSubclassOf(this TypeDefinition typeDefinition, string class
             return false;
         }
 
+        public static string FullNameWithGenericParameters(this TypeReference typeReference, GenericParameter[] contextGenericParameters, TypeReference[] contextGenericParameterTypes)
+        {
+            var name = typeReference.FullName;
+            if (typeReference.HasGenericParameters)
+            {
+                name += "<";
+                for (var i = 0; i < typeReference.Resolve().GenericParameters.Count; ++i)
+                {
+                    if (i != 0)
+                    {
+                        name += ", ";
+                    }
+
+                    for (var j = 0; j < contextGenericParameters.Length; ++j)
+                    {
+                        if (typeReference.GenericParameters[i].FullName == contextGenericParameters[i].FullName)
+                        {
+                            name += contextGenericParameterTypes[i].FullName;
+                            break;
+                        }
+                    }
+                }
+
+                name += ">";
+            }
+
+            return name;
+        }
+
         public static bool HasInterface(this TypeReference typeReference, string interfaceTypeFullName)
         {
             if (typeReference.IsArray)

com.unity.netcode.gameobjects/Editor/CodeGen/INetworkSerializableILPP.cs

@@ -24,14 +24,37 @@ public override bool WillProcess(ICompiledAssembly compiledAssembly) =>
 
         private readonly List<DiagnosticMessage> m_Diagnostics = new List<DiagnosticMessage>();
 
+        private TypeReference ResolveGenericType(TypeReference type, List<TypeReference> typeStack)
+        {
+            var genericName = type.Name;
+            var lastType = (GenericInstanceType)typeStack[typeStack.Count - 1];
+            var resolvedType = lastType.Resolve();
+            typeStack.RemoveAt(typeStack.Count - 1);
+            for (var i = 0; i < resolvedType.GenericParameters.Count; ++i)
+            {
+                var parameter = resolvedType.GenericParameters[i];
+                if (parameter.Name == genericName)
+                {
+                    var underlyingType = lastType.GenericArguments[i];
+                    if (underlyingType.Resolve() == null)
+                    {
+                        return ResolveGenericType(underlyingType, typeStack);
+                    }
+
+                    return underlyingType;
+                }
+            }
+
+            return null;
+        }
+
         public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
         {
             if (!WillProcess(compiledAssembly))
             {
                 return null;
             }
 
-
             m_Diagnostics.Clear();
 
             // read
@@ -50,16 +73,124 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
                 {
                     if (ImportReferences(mainModule))
                     {
-                        var types = mainModule.GetTypes()
-                            .Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializable_FullName) && !t.Resolve().IsAbstract && t.Resolve().IsValueType)
+                        // Initialize all the delegates for various NetworkVariable types to ensure they can be serailized
+
+                        // Find all types we know we're going to want to serialize.
+                        // The list of these types includes:
+                        // - Non-generic INetworkSerializable types
+                        // - Non-Generic INetworkSerializeByMemcpy types
+                        // - Enums that are not declared within generic types
+                        // We can't process generic types because, to initialize a generic, we need a value
+                        // for `T` to initialize it with.
+                        var networkSerializableTypes = mainModule.GetTypes()
+                            .Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializable_FullName) && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
+                            .ToList();
+                        var structTypes = mainModule.GetTypes()
+                            .Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName) && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
+                            .ToList();
+                        var enumTypes = mainModule.GetTypes()
+                            .Where(t => t.Resolve().IsEnum && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
                             .ToList();
-                        // process `INetworkMessage` types
-                        if (types.Count == 0)
+
+                        // Now, to support generics, we have to do an extra pass.
+                        // We look for any type that's a NetworkBehaviour type
+                        // Then we look through all the fields in that type, finding any field whose type is
+                        // descended from `NetworkVariableSerialization`. Then we check `NetworkVariableSerialization`'s
+                        // `T` value, and if it's a generic, then we know it was missed in the above sweep and needs
+                        // to be initialized. Now we have a full generic instance rather than a generic definition,
+                        // so we can validly generate an initializer for that particular instance of the generic type.
+                        var networkSerializableTypesSet = new HashSet<TypeReference>(networkSerializableTypes);
+                        var structTypesSet = new HashSet<TypeReference>(structTypes);
+                        var enumTypesSet = new HashSet<TypeReference>(enumTypes);
+                        var typeStack = new List<TypeReference>();
+                        foreach (var type in mainModule.GetTypes())
+                        {
+                            // Check if it's a NetworkBehaviour
+                            if (type.IsSubclassOf(CodeGenHelpers.NetworkBehaviour_FullName))
+                            {
+                                // Iterate fields looking for NetworkVariableSerialization fields
+                                foreach (var field in type.Fields)
+                                {
+                                    // Get the field type and its base type
+                                    var fieldType = field.FieldType;
+                                    var baseType = fieldType.Resolve().BaseType;
+                                    // This type stack is used for resolving NetworkVariableSerialization's T value
+                                    // When looking at base types, we get the type definition rather than the
+                                    // type reference... which means that we get the generic definition with an
+                                    // undefined T rather than the instance with the type filled in.
+                                    // We then have to walk backward back down the type stack to resolve what T
+                                    // is.
+                                    typeStack.Clear();
+                                    typeStack.Add(fieldType);
+                                    // Iterate through the base types until we get to Object.
+                                    // Object is the base for everything so we'll stop when we hit that.
+                                    while (baseType.Name != mainModule.TypeSystem.Object.Name)
+                                    {
+                                        // If we've found a NetworkVariableSerialization type...
+                                        if (baseType.IsGenericInstance && baseType.Resolve() == m_NetworkVariableSerializationType)
+                                        {
+                                            // Then we need to figure out what T is
+                                            var genericType = (GenericInstanceType)baseType;
+                                            var underlyingType = genericType.GenericArguments[0];
+                                            if (underlyingType.Resolve() == null)
+                                            {
+                                                underlyingType = ResolveGenericType(underlyingType, typeStack);
+                                            }
+
+                                            // If T is generic...
+                                            if (underlyingType.IsGenericInstance)
+                                            {
+                                                // Then we pick the correct set to add it to and set it up
+                                                // for initialization.
+                                                if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializable_FullName))
+                                                {
+                                                    networkSerializableTypesSet.Add(underlyingType);
+                                                }
+
+                                                if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName))
+                                                {
+                                                    structTypesSet.Add(underlyingType);
+                                                }
+
+                                                if (underlyingType.Resolve().IsEnum)
+                                                {
+                                                    enumTypesSet.Add(underlyingType);
+                                                }
+                                            }
+
+                                            break;
+                                        }
+
+                                        typeStack.Add(baseType);
+                                        baseType = baseType.Resolve().BaseType;
+                                    }
+                                }
+                            }
+                            // We'll also avoid some confusion by ensuring users only choose one of the two
+                            // serialization schemes - by method OR by memcpy, not both. We'll also do a cursory
+                            // check that INetworkSerializeByMemcpy types are unmanaged.
+                            else if (type.HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName))
+                            {
+                                if (type.HasInterface(CodeGenHelpers.INetworkSerializable_FullName))
+                                {
+                                    m_Diagnostics.AddError($"{nameof(INetworkSerializeByMemcpy)} types may not implement {nameof(INetworkSerializable)} - choose one or the other.");
+                                }
+                                if (!type.IsValueType)
+                                {
+                                    m_Diagnostics.AddError($"{nameof(INetworkSerializeByMemcpy)} types must be unmanaged types.");
+                                }
+                            }
+                        }
+
+                        if (networkSerializableTypes.Count + structTypes.Count + enumTypes.Count == 0)
                         {
                             return null;
                         }
 
-                        CreateModuleInitializer(assemblyDefinition, types);
+                        // Finally we add to the module initializer some code to initialize the delegates in
+                        // NetworkVariableSerialization<T> for all necessary values of T, by calling initialization
+                        // methods in NetworkVariableHelpers.
+                        CreateModuleInitializer(assemblyDefinition, networkSerializableTypesSet.ToList(), structTypesSet.ToList(), enumTypesSet.ToList());
                     }
                     else
                     {
@@ -94,9 +225,15 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
             return new ILPostProcessResult(new InMemoryAssembly(pe.ToArray(), pdb.ToArray()), m_Diagnostics);
         }
 
-        private MethodReference m_InitializeDelegates_MethodRef;
+        private MethodReference m_InitializeDelegatesNetworkSerializable_MethodRef;
+        private MethodReference m_InitializeDelegatesStruct_MethodRef;
+        private MethodReference m_InitializeDelegatesEnum_MethodRef;
 
-        private const string k_InitializeMethodName = nameof(NetworkVariableHelper.InitializeDelegates);
+        private TypeDefinition m_NetworkVariableSerializationType;
+
+        private const string k_InitializeNetworkSerializableMethodName = nameof(NetworkVariableHelper.InitializeDelegatesNetworkSerializable);
+        private const string k_InitializeStructMethodName = nameof(NetworkVariableHelper.InitializeDelegatesStruct);
+        private const string k_InitializeEnumMethodName = nameof(NetworkVariableHelper.InitializeDelegatesEnum);
 
         private bool ImportReferences(ModuleDefinition moduleDefinition)
         {
@@ -106,11 +243,18 @@ private bool ImportReferences(ModuleDefinition moduleDefinition)
             {
                 switch (methodInfo.Name)
                 {
-                    case k_InitializeMethodName:
-                        m_InitializeDelegates_MethodRef = moduleDefinition.ImportReference(methodInfo);
+                    case k_InitializeNetworkSerializableMethodName:
+                        m_InitializeDelegatesNetworkSerializable_MethodRef = moduleDefinition.ImportReference(methodInfo);
+                        break;
+                    case k_InitializeStructMethodName:
+                        m_InitializeDelegatesStruct_MethodRef = moduleDefinition.ImportReference(methodInfo);
+                        break;
+                    case k_InitializeEnumMethodName:
+                        m_InitializeDelegatesEnum_MethodRef = moduleDefinition.ImportReference(methodInfo);
                         break;
                 }
             }
+            m_NetworkVariableSerializationType = moduleDefinition.ImportReference(typeof(NetworkVariableSerialization<>)).Resolve();
             return true;
         }
 
@@ -139,7 +283,7 @@ private MethodDefinition GetOrCreateStaticConstructor(TypeDefinition typeDefinit
         // C# (that attribute doesn't exist in Unity, but the static module constructor still works)
         // https://docs.microsoft.com/en-us/dotnet/api/system.runtime.compilerservices.moduleinitializerattribute?view=net-5.0
         // https://web.archive.org/web/20100212140402/http://blogs.msdn.com/junfeng/archive/2005/11/19/494914.aspx
-        private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeDefinition> networkSerializableTypes)
+        private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeReference> networkSerializableTypes, List<TypeReference> structTypes, List<TypeReference> enumTypes)
         {
             foreach (var typeDefinition in assembly.MainModule.Types)
             {
@@ -151,9 +295,23 @@ private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeDefin
 
                     var instructions = new List<Instruction>();
 
+                    foreach (var type in structTypes)
+                    {
+                        var method = new GenericInstanceMethod(m_InitializeDelegatesStruct_MethodRef);
+                        method.GenericArguments.Add(type);
+                        instructions.Add(processor.Create(OpCodes.Call, method));
+                    }
+
                     foreach (var type in networkSerializableTypes)
                     {
-                        var method = new GenericInstanceMethod(m_InitializeDelegates_MethodRef);
+                        var method = new GenericInstanceMethod(m_InitializeDelegatesNetworkSerializable_MethodRef);
+                        method.GenericArguments.Add(type);
+                        instructions.Add(processor.Create(OpCodes.Call, method));
+                    }
+
+                    foreach (var type in enumTypes)
+                    {
+                        var method = new GenericInstanceMethod(m_InitializeDelegatesEnum_MethodRef);
                         method.GenericArguments.Add(type);
                         instructions.Add(processor.Create(OpCodes.Call, method));
                     }