Skip to content

fix: restore native fixed string support #1961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ShadauxCat merged 8 commits into from
May 18, 2022
Merged

fix: restore native fixed string support #1961

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
14d5293
WIP
ShadauxCat May 7, 2022
e2c14ec
Finished implementation, fixed tests, added some new tests.
ShadauxCat May 11, 2022
9f349d1
Standards
ShadauxCat May 11, 2022
c260335
Merge branch 'develop' into fix/restore_native_FixedString_support
ShadauxCat May 11, 2022
9ee7de6
changelog
ShadauxCat May 11, 2022
a992d63
Added some requested comments.
ShadauxCat May 16, 2022
9771bd7
Added some comments.
ShadauxCat May 18, 2022
8a86af5
Merge branch 'develop' into fix/restore_native_FixedString_support
ShadauxCat May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions com.unity.netcode.gameobjects/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Additional documentation and release notes are available at [Multiplayer Documen
### Fixed

- Fixed endless dialog boxes when adding a NetworkBehaviour to a NetworkManager or vice-versa (#1947)
- `FixedString` types can now be used in NetworkVariables and RPCs again without requiring a `ForceNetworkSerializeByMemcpy<>` wrapper (#1961)
Comment thread
becksebenius-unity marked this conversation as resolved.

## [1.0.0-pre.9] - 2022-05-10

Expand Down
2 changes: 2 additions & 0 deletions com.unity.netcode.gameobjects/Editor/CodeGen/CodeGenHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Cecil.Rocks;
using Unity.Collections;
using Unity.CompilationPipeline.Common.Diagnostics;
using Unity.CompilationPipeline.Common.ILPostProcessing;
using UnityEngine;
Expand All @@ -28,6 +29,7 @@ internal static class CodeGenHelpers
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 IUTF8Bytes_FullName = typeof(IUTF8Bytes).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;
Expand Down
70 changes: 61 additions & 9 deletions com.unity.netcode.gameobjects/Editor/CodeGen/INetworkSerializableILPP.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Mono.Cecil;
using Mono.Cecil.Cil;
using Mono.Cecil.Rocks;
using Unity.Collections;
using Unity.CompilationPipeline.Common.Diagnostics;
using Unity.CompilationPipeline.Common.ILPostProcessing;
using ILPPInterface = Unity.CompilationPipeline.Common.ILPostProcessing.ILPostProcessor;
Expand Down Expand Up @@ -88,6 +89,9 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
var structTypes = mainModule.GetTypes()
.Where(t => t.Resolve().HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName) && !t.Resolve().IsAbstract && !t.Resolve().HasGenericParameters && t.Resolve().IsValueType)
.ToList();
var fixedStringTypes = mainModule.GetTypes()
.Where(t => t.Resolve().HasInterface(CodeGenHelpers.IUTF8Bytes_FullName) && t.HasInterface(m_INativeListBool_TypeRef.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();
Expand All @@ -102,6 +106,7 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
var networkSerializableTypesSet = new HashSet<TypeReference>(networkSerializableTypes);
var structTypesSet = new HashSet<TypeReference>(structTypes);
var enumTypesSet = new HashSet<TypeReference>(enumTypes);
var fixedStringTypesSet = new HashSet<TypeReference>(fixedStringTypes);
var typeStack = new List<TypeReference>();
foreach (var type in mainModule.GetTypes())
{
Expand Down Expand Up @@ -141,26 +146,56 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
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 it's generic. We'll also use this moment to catch
// any NetworkVariables with invalid T types at compile time.
if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializable_FullName))
{
// Then we pick the correct set to add it to and set it up
// for initialization.
if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializable_FullName))
if (underlyingType.IsGenericInstance)
{
networkSerializableTypesSet.Add(underlyingType);
}
}

if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName))
else if (underlyingType.HasInterface(CodeGenHelpers.INetworkSerializeByMemcpy_FullName))
{
if (underlyingType.IsGenericInstance)
{
structTypesSet.Add(underlyingType);
}
}
else if (underlyingType.HasInterface(m_INativeListBool_TypeRef.FullName) && underlyingType.HasInterface(CodeGenHelpers.IUTF8Bytes_FullName))
Comment thread
ShadauxCat marked this conversation as resolved.
{
if (underlyingType.IsGenericInstance)
{
fixedStringTypesSet.Add(underlyingType);
}
}

if (underlyingType.Resolve().IsEnum)
else if (underlyingType.Resolve().IsEnum)
{
if (underlyingType.IsGenericInstance)
{
enumTypesSet.Add(underlyingType);
}
}
else if (!underlyingType.Resolve().IsPrimitive)
{
bool methodExists = false;
foreach (var method in m_FastBufferWriterType.Methods)
{
if (!method.HasGenericParameters && method.Parameters.Count == 1 && method.Parameters[0].ParameterType.Resolve() == underlyingType.Resolve())
{
methodExists = true;
break;
}
}

if (!methodExists)
{
m_Diagnostics.AddError($"{type}.{field.Name}: {underlyingType} is not valid for use in {typeof(NetworkVariable<>).Name} types. Types must either implement {nameof(INetworkSerializeByMemcpy)} or {nameof(INetworkSerializable)}. If this type is external and you are sure its memory layout makes it serializable by memcpy, you can replace {underlyingType} with {typeof(ForceNetworkSerializeByMemcpy<>).Name}<{underlyingType}>, or you can create extension methods for {nameof(FastBufferReader)}.{nameof(FastBufferReader.ReadValueSafe)}(this {nameof(FastBufferReader)}, out {underlyingType}) and {nameof(FastBufferWriter)}.{nameof(FastBufferWriter.WriteValueSafe)}(this {nameof(FastBufferWriter)}, in {underlyingType}) to define serialization for this type.");
Copy link
Copy Markdown
Contributor

@becksebenius-unity becksebenius-unity May 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but have you thought about having a short error message prefixing the detailed one here so that it's easy to refer to in community messaging?
"Unsupported Network Serialization Type. {type}.{field.Name}: {underlyingType} is not..."

Copy link
Copy Markdown
Collaborator Author

@ShadauxCat ShadauxCat May 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've got a follow-up PR that gets rid of this message again and moves the info elsewhere, so let's revisit the wording in that PR if it's still not good for you.

}
}

break;
}
Expand Down Expand Up @@ -194,7 +229,7 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
// 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());
CreateModuleInitializer(assemblyDefinition, networkSerializableTypesSet.ToList(), structTypesSet.ToList(), enumTypesSet.ToList(), fixedStringTypesSet.ToList());
}
else
{
Expand Down Expand Up @@ -232,12 +267,17 @@ public override ILPostProcessResult Process(ICompiledAssembly compiledAssembly)
private MethodReference m_InitializeDelegatesNetworkSerializable_MethodRef;
private MethodReference m_InitializeDelegatesStruct_MethodRef;
private MethodReference m_InitializeDelegatesEnum_MethodRef;
private MethodReference m_InitializeDelegatesFixedString_MethodRef;

private TypeDefinition m_NetworkVariableSerializationType;
private TypeDefinition m_FastBufferWriterType;

private TypeReference m_INativeListBool_TypeRef;

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 const string k_InitializeFixedStringMethodName = nameof(NetworkVariableHelper.InitializeDelegatesFixedString);

private bool ImportReferences(ModuleDefinition moduleDefinition)
{
Expand All @@ -256,9 +296,14 @@ private bool ImportReferences(ModuleDefinition moduleDefinition)
case k_InitializeEnumMethodName:
m_InitializeDelegatesEnum_MethodRef = moduleDefinition.ImportReference(methodInfo);
break;
case k_InitializeFixedStringMethodName:
m_InitializeDelegatesFixedString_MethodRef = moduleDefinition.ImportReference(methodInfo);
break;
}
}
m_NetworkVariableSerializationType = moduleDefinition.ImportReference(typeof(NetworkVariableSerialization<>)).Resolve();
m_NetworkVariableSerializationType = moduleDefinition.ImportReference(typeof(FastBufferWriter)).Resolve();
m_INativeListBool_TypeRef = moduleDefinition.ImportReference(typeof(INativeList<bool>));
return true;
}

Expand Down Expand Up @@ -287,7 +332,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<TypeReference> networkSerializableTypes, List<TypeReference> structTypes, List<TypeReference> enumTypes)
private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeReference> networkSerializableTypes, List<TypeReference> structTypes, List<TypeReference> enumTypes, List<TypeReference> fixedStringTypes)
{
foreach (var typeDefinition in assembly.MainModule.Types)
{
Expand Down Expand Up @@ -320,6 +365,13 @@ private void CreateModuleInitializer(AssemblyDefinition assembly, List<TypeRefer
instructions.Add(processor.Create(OpCodes.Call, method));
}

foreach (var type in fixedStringTypes)
{
var method = new GenericInstanceMethod(m_InitializeDelegatesFixedString_MethodRef);
method.GenericArguments.Add(type);
instructions.Add(processor.Create(OpCodes.Call, method));
}

instructions.ForEach(instruction => processor.Body.Instructions.Insert(processor.Body.Instructions.Count - 1, instruction));
break;
}
Expand Down
20 changes: 18 additions & 2 deletions com.unity.netcode.gameobjects/Editor/CodeGen/NetworkBehaviourILPP.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -614,6 +614,14 @@ private MethodReference GetFastBufferWriterWriteMethod(string name, TypeReferenc
#endif

var resolvedConstraintName = resolvedConstraint.FullNameWithGenericParameters(new[] { method.GenericParameters[0] }, new[] { checkType });
if (constraint.IsGenericInstance)
{
var genericConstraint = (GenericInstanceType)constraint;
if (genericConstraint.HasGenericArguments && genericConstraint.GenericArguments[0].Resolve() != null)
{
resolvedConstraintName = constraint.FullName;
}
}
if ((resolvedConstraint.IsInterface && !checkType.HasInterface(resolvedConstraintName)) ||
(resolvedConstraint.IsClass && !checkType.Resolve().IsSubclassOf(resolvedConstraintName)) ||
(resolvedConstraint.Name == "ValueType" && !checkType.IsValueType))
Expand Down Expand Up @@ -749,6 +757,14 @@ private MethodReference GetFastBufferReaderReadMethod(string name, TypeReference


var resolvedConstraintName = resolvedConstraint.FullNameWithGenericParameters(new[] { method.GenericParameters[0] }, new[] { checkType });
if (constraint.IsGenericInstance)
{
var genericConstraint = (GenericInstanceType)constraint;
if (genericConstraint.HasGenericArguments && genericConstraint.GenericArguments[0].Resolve() != null)
{
resolvedConstraintName = constraint.FullName;
}
}

if ((resolvedConstraint.IsInterface && !checkType.HasInterface(resolvedConstraintName)) ||
(resolvedConstraint.IsClass && !checkType.Resolve().IsSubclassOf(resolvedConstraintName)) ||
Expand Down Expand Up @@ -1142,7 +1158,7 @@ private void InjectWriteAndCallBlocks(MethodDefinition methodDefinition, CustomA
}
else
{
m_Diagnostics.AddError(methodDefinition, $"Don't know how to serialize {paramType.Name} - implement {nameof(INetworkSerializable)}, tag memcpyable struct with {nameof(INetworkSerializeByMemcpy)}, or add an extension method for {nameof(FastBufferWriter)}.{k_WriteValueMethodName} to define serialization.");
m_Diagnostics.AddError(methodDefinition, $"{methodDefinition.Name} - Don't know how to serialize {paramType.Name}. RPC parameter types must either implement {nameof(INetworkSerializeByMemcpy)} or {nameof(INetworkSerializable)}. If this type is external and you are sure its memory layout makes it serializable by memcpy, you can replace {paramType} with {typeof(ForceNetworkSerializeByMemcpy<>).Name}<{paramType}>, or you can create extension methods for {nameof(FastBufferReader)}.{nameof(FastBufferReader.ReadValueSafe)}(this {nameof(FastBufferReader)}, out {paramType}) and {nameof(FastBufferWriter)}.{nameof(FastBufferWriter.WriteValueSafe)}(this {nameof(FastBufferWriter)}, in {paramType}) to define serialization for this type.");
continue;
}

Expand Down Expand Up @@ -1456,7 +1472,7 @@ private MethodDefinition GenerateStaticHandler(MethodDefinition methodDefinition
}
else
{
m_Diagnostics.AddError(methodDefinition, $"Don't know how to serialize {paramType.Name} - implement {nameof(INetworkSerializable)}, tag memcpyable struct with {nameof(INetworkSerializeByMemcpy)}, or add an extension method for {nameof(FastBufferWriter)}.{k_WriteValueMethodName} to define serialization.");
m_Diagnostics.AddError(methodDefinition, $"{methodDefinition.Name} - Don't know how to serialize {paramType.Name}. RPC parameter types must either implement {nameof(INetworkSerializeByMemcpy)} or {nameof(INetworkSerializable)}. If this type is external and you are sure its memory layout makes it serializable by memcpy, you can replace {paramType} with {typeof(ForceNetworkSerializeByMemcpy<>).Name}<{paramType}>, or you can create extension methods for {nameof(FastBufferReader)}.{nameof(FastBufferReader.ReadValueSafe)}(this {nameof(FastBufferReader)}, out {paramType}) and {nameof(FastBufferWriter)}.{nameof(FastBufferWriter.WriteValueSafe)}(this {nameof(FastBufferWriter)}, in {paramType}) to define serialization for this type.");
continue;
}

Expand Down
5 changes: 3 additions & 2 deletions com.unity.netcode.gameobjects/Editor/CodeGen/com.unity.netcode.editor.codegen.asmdef
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"name": "Unity.Netcode.Editor.CodeGen",
"rootNamespace": "Unity.Netcode.Editor.CodeGen",
"references": [
"Unity.Netcode.Runtime"
"Unity.Netcode.Runtime",
"Unity.Collections"
],
"includePlatforms": [
"Editor"
Expand All @@ -26,4 +27,4 @@
}
],
"noEngineReferences": false
}
}
12 changes: 12 additions & 0 deletions com.unity.netcode.gameobjects/Runtime/NetworkVariable/NetworkVariableHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using Unity.Collections;
using UnityEngine;

namespace Unity.Netcode
Expand Down Expand Up @@ -36,6 +37,11 @@ internal static void InitializeDelegatesPrimitive<T>() where T : unmanaged, ICom
NetworkVariableSerialization<T>.SetWriteDelegate(NetworkVariableSerialization<T>.WritePrimitive);
NetworkVariableSerialization<T>.SetReadDelegate(NetworkVariableSerialization<T>.ReadPrimitive);
}
internal static void InitializeDelegatesFixedString<T>() where T : unmanaged, INativeList<byte>, IUTF8Bytes
{
NetworkVariableSerialization<T>.SetWriteDelegate(NetworkVariableSerialization<T>.WriteFixedString);
NetworkVariableSerialization<T>.SetReadDelegate(NetworkVariableSerialization<T>.ReadFixedString);
}

internal static void InitializeAllBaseDelegates()
{
Expand All @@ -54,6 +60,12 @@ internal static void InitializeAllBaseDelegates()
InitializeDelegatesPrimitive<long>();
InitializeDelegatesPrimitive<ulong>();

InitializeDelegatesFixedString<FixedString32Bytes>();
InitializeDelegatesFixedString<FixedString64Bytes>();
InitializeDelegatesFixedString<FixedString128Bytes>();
InitializeDelegatesFixedString<FixedString512Bytes>();
InitializeDelegatesFixedString<FixedString4096Bytes>();

// Built-in Unity types, serialized with specific overloads because they're structs without ISerializeByMemcpy attached
NetworkVariableSerialization<Vector2>.SetWriteDelegate((FastBufferWriter writer, in Vector2 value) => { writer.WriteValueSafe(value); });
NetworkVariableSerialization<Vector3>.SetWriteDelegate((FastBufferWriter writer, in Vector3 value) => { writer.WriteValueSafe(value); });
Expand Down
Loading