Skip to content

fix: NetworkVariable subclasses not generating serialization code #2421

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 3 commits into from
Mar 9, 2023
Merged

fix: NetworkVariable subclasses not generating serialization code #2421

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
458975e
fix: NetworkVariable subclasses not generating serialization code
ShadauxCat Feb 22, 2023
4b8e2b0
Merge branch 'develop' into fix/networkvariable_subclasses_throwing_e…
ShadauxCat Mar 3, 2023
ce4eae8
Merge branch 'develop' into fix/networkvariable_subclasses_throwing_e…
ShadauxCat Mar 9, 2023
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
80 changes: 80 additions & 0 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 @@ -837,6 +837,58 @@ private void GetAllFieldsAndResolveGenerics(TypeDefinition type, ref List<TypeRe
GetAllFieldsAndResolveGenerics(resolved, ref fieldTypes, genericParams);
}

private void GetAllBaseTypesAndResolveGenerics(TypeDefinition type, ref List<TypeReference> baseTypes, Dictionary<string, TypeReference> genericParameters)
{

if (type.BaseType == null || type.BaseType.Name == "Object")
{
return;
}

var baseType = type.BaseType;

var genericParams = new Dictionary<string, TypeReference>();

if (baseType.IsGenericInstance)
{
var genericType = (GenericInstanceType)baseType;
var newGenericType = new GenericInstanceType(baseType.Resolve());
for (var i = 0; i < genericType.GenericArguments.Count; ++i)
Copy link
Copy Markdown
Contributor

@samlucas-unity samlucas-unity Mar 1, 2023

Choose a reason for hiding this comment

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

More a question than a comment on the code :). Is there a reason you're not using foreach apart from this is your preferred way to iterate? I'm just wondering if there are reasons not to use it which I'm not aware of?

Copy link
Copy Markdown
Collaborator Author

@ShadauxCat ShadauxCat Mar 3, 2023

Choose a reason for hiding this comment

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

um... if you look at the code below, in the next loop, it's accessing two different arrays with the same index. I think when I originally wrote this loop, it was doing the same thing, so I was using C-style for to be able to have access to the index value for the other array. At some point I think I realized this loop doesn't need it and removed the other access, but didn't change the loop style.

{
var argument = genericType.GenericArguments[i];

if (genericParameters != null && genericParameters.ContainsKey(argument.Name))
{
newGenericType.GenericArguments.Add(genericParameters[argument.Name]);
genericParams[baseType.Resolve().GenericParameters[newGenericType.GenericArguments.Count - 1].Name] = genericParameters[argument.Name];
}
else
{
newGenericType.GenericArguments.Add(argument);
}
}
baseTypes.Add(newGenericType);
}
else
{
baseTypes.Add(baseType);
}

var resolved = type.BaseType.Resolve();
if (type.BaseType.IsGenericInstance)
{
var genericType = (GenericInstanceType)type.BaseType;
for (var i = 0; i < genericType.GenericArguments.Count; ++i)
{
if (!genericParams.ContainsKey(resolved.GenericParameters[i].Name))
{
genericParams[resolved.GenericParameters[i].Name] = genericType.GenericArguments[i];
}
}
}
GetAllBaseTypesAndResolveGenerics(resolved, ref baseTypes, genericParams);
}

private void ProcessNetworkBehaviour(TypeDefinition typeDefinition, string[] assemblyDefines)
{
var rpcHandlers = new List<(uint RpcMethodId, MethodDefinition RpcHandler)>();
Expand Down Expand Up @@ -898,6 +950,34 @@ private void ProcessNetworkBehaviour(TypeDefinition typeDefinition, string[] ass
}
}
}
{
var baseTypes = new List<TypeReference>();

var genericParams = new Dictionary<string, TypeReference>();
var resolved = type.Resolve();
if (type.IsGenericInstance)
{
var genericType = (GenericInstanceType)type;
for (var i = 0; i < genericType.GenericArguments.Count; ++i)
{
genericParams[resolved.GenericParameters[i].Name] = genericType.GenericArguments[i];
}
}

GetAllBaseTypesAndResolveGenerics(type.Resolve(), ref baseTypes, genericParams);
foreach (var baseType in baseTypes)
{
if (baseType.Resolve().Name == typeof(NetworkVariable<>).Name || baseType.Resolve().Name == typeof(NetworkList<>).Name)
{
var genericInstanceType = (GenericInstanceType)baseType;
var wrappedType = genericInstanceType.GenericArguments[0];
if (!m_WrappedNetworkVariableTypes.Contains(wrappedType))
{
m_WrappedNetworkVariableTypes.Add(wrappedType);
}
}
}
}
}
}

Expand Down
32 changes: 32 additions & 0 deletions com.unity.netcode.gameobjects/Tests/Runtime/NetworkVariableTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,22 @@ public class NetVarPermTestComp : NetworkBehaviour
public NetworkVariable<Vector3> OwnerReadWrite_Position = new NetworkVariable<Vector3>(Vector3.one, NetworkVariableReadPermission.Owner, NetworkVariableWritePermission.Owner);
}

public class NetworkVariableMiddleclass<TMiddleclassName> : NetworkVariable<TMiddleclassName>
{

}

public class NetworkVariableSubclass<TSubclassName> : NetworkVariableMiddleclass<TSubclassName>
{

}

public struct TemplatedValueOnlyReferencedByNetworkVariableSubclass<T> : INetworkSerializeByMemcpy
where T : unmanaged
{
public T Value;
}

// The ILPP code for NetworkVariables to determine how to serialize them relies on them existing as fields of a NetworkBehaviour to find them.
// Some of the tests below create NetworkVariables on the stack, so this class is here just to make sure the relevant types are all accounted for.
public class NetVarILPPClassForTests : NetworkBehaviour
Expand All @@ -25,6 +41,7 @@ public class NetVarILPPClassForTests : NetworkBehaviour
public NetworkVariable<ManagedNetworkSerializableType> ManagedNetworkSerializableTypeVar;
public NetworkVariable<string> StringVar;
public NetworkVariable<Guid> GuidVar;
public NetworkVariableSubclass<TemplatedValueOnlyReferencedByNetworkVariableSubclass<int>> SubclassVar;
}

public class TemplateNetworkBehaviourType<T> : NetworkBehaviour
Expand Down Expand Up @@ -1096,6 +1113,21 @@ public void TestUnsupportedUnmanagedTypesThrowExceptions()
});
}

[Test]
public void TestTypesReferencedInSubclassSerializeSuccessfully()
{
var variable = new NetworkVariableSubclass<TemplatedValueOnlyReferencedByNetworkVariableSubclass<int>>();
using var writer = new FastBufferWriter(1024, Allocator.Temp);
var value = new TemplatedValueOnlyReferencedByNetworkVariableSubclass<int> { Value = 12345 };
variable.Value = value;
variable.WriteField(writer);
variable.Value = new TemplatedValueOnlyReferencedByNetworkVariableSubclass<int> { Value = 54321 };

using var reader = new FastBufferReader(writer, Allocator.None);
variable.ReadField(reader);
Assert.AreEqual(value.Value, variable.Value.Value);
}

[Test]
public void TestUnsupportedUnmanagedTypesWithUserSerializationDoNotThrowExceptions()
{
Expand Down