feat: Weaver can now automatically create Reader/Writer for types in a different assembly (#1708)

* fix: serialize structs in other assemblies

fixes #1570

* removing check for type in another assembly

* fixing call to default constructor

* removing old tests that not longer apply

* fixing typo

* adding error when creating writer for unity base classes

* Test for Generating Reader and Writer functions

* removing old function

Co-authored-by: Paul Pacheco <paulpach@gmail.com>
This commit is contained in:
James Frowen 2020-04-16 09:50:49 +01:00 committed by GitHub
parent 58219c8f72
commit b1644ae481
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
36 changed files with 545 additions and 97 deletions

View File

@ -294,11 +294,6 @@ static MethodDefinition GenerateClassOrStructReadFunction(TypeReference variable
return null;
}
if (!Weaver.IsValidTypeToGenerate(variable.Resolve()))
{
return null;
}
string functionName = "_Read" + variable.Name + "_";
if (variable.DeclaringType != null)
{
@ -357,10 +352,12 @@ private static void CreateNew(TypeReference variable, ILProcessor worker, TypeDe
MethodDefinition ctor = Resolvers.ResolveDefaultPublicCtor(variable);
if (ctor == null)
{
Weaver.Error($"{variable} can't be deserialized because i has no default constructor");
Weaver.Error($"{variable} can't be deserialized because it has no default constructor");
}
worker.Append(worker.Create(OpCodes.Newobj, ctor));
MethodReference ctorRef = Weaver.CurrentAssembly.MainModule.ImportReference(ctor);
worker.Append(worker.Create(OpCodes.Newobj, ctorRef));
worker.Append(worker.Create(OpCodes.Stloc_0));
}
}
@ -387,8 +384,9 @@ private static void DeserializeFields(TypeReference variable, int recursionCount
{
Weaver.Error($"{field} has an unsupported type");
}
FieldReference fieldRef = Weaver.CurrentAssembly.MainModule.ImportReference(field);
worker.Append(worker.Create(OpCodes.Stfld, field));
worker.Append(worker.Create(OpCodes.Stfld, fieldRef));
fields++;
}
if (fields == 0)

View File

@ -73,6 +73,7 @@ internal static class Weaver
public static MethodReference ReadyConnectionReference;
public static TypeReference ComponentType;
public static TypeReference ObjectType;
public static TypeReference CmdDelegateReference;
public static MethodReference CmdDelegateConstructor;
@ -294,6 +295,7 @@ static void SetupTargetTypes()
RecycleWriterReference = Resolvers.ResolveMethod(NetworkWriterPoolType, CurrentAssembly, "Recycle");
ComponentType = UnityAssembly.MainModule.GetType("UnityEngine.Component");
ObjectType = UnityAssembly.MainModule.GetType("UnityEngine.Object");
ClientSceneType = NetAssembly.MainModule.GetType("Mirror.ClientScene");
ReadyConnectionReference = Resolvers.ResolveMethod(ClientSceneType, CurrentAssembly, "get_readyConnection");
@ -329,15 +331,6 @@ public static bool IsNetworkBehaviour(TypeDefinition td)
return td.IsDerivedFrom(NetworkBehaviourType);
}
public static bool IsValidTypeToGenerate(TypeDefinition variable)
{
// a valid type is a simple class or struct. so we generate only code for types we dont know, and if they are not inside
// this assembly it must mean that we are trying to serialize a variable outside our scope. and this will fail.
// no need to report an error here, the caller will report a better error
string assembly = CurrentAssembly.MainModule.Name;
return variable.Module.Name == assembly;
}
static void CheckMonoBehaviour(TypeDefinition td)
{
if (td.IsDerivedFrom(MonoBehaviourType))

View File

@ -58,6 +58,16 @@ public static MethodReference GetWriteFunc(TypeReference variable, int recursion
Weaver.Error($"Cannot generate writer for component type {variable}. Use a supported type or provide a custom writer");
return null;
}
if (variable.FullName == Weaver.ObjectType.FullName)
{
Weaver.Error($"Cannot generate writer for {variable}. Use a supported type or provide a custom writer");
return null;
}
if (variable.FullName == Weaver.ScriptableObjectType.FullName)
{
Weaver.Error($"Cannot generate writer for {variable}. Use a supported type or provide a custom writer");
return null;
}
if (td.HasGenericParameters && !td.FullName.StartsWith("System.ArraySegment`1", System.StringComparison.Ordinal))
{
Weaver.Error($"Cannot generate writer for generic type {variable}. Use a concrete type or provide a custom writer");
@ -108,11 +118,6 @@ static MethodDefinition GenerateClassOrStructWriterFunction(TypeReference variab
return null;
}
if (!Weaver.IsValidTypeToGenerate(variable.Resolve()))
{
return null;
}
string functionName = "_Write" + variable.Name + "_";
if (variable.DeclaringType != null)
{
@ -143,10 +148,12 @@ static MethodDefinition GenerateClassOrStructWriterFunction(TypeReference variab
MethodReference writeFunc = GetWriteFunc(field.FieldType, recursionCount + 1);
if (writeFunc != null)
{
FieldReference fieldRef = Weaver.CurrentAssembly.MainModule.ImportReference(field);
fields++;
worker.Append(worker.Create(OpCodes.Ldarg_0));
worker.Append(worker.Create(OpCodes.Ldarg_1));
worker.Append(worker.Create(OpCodes.Ldfld, field));
worker.Append(worker.Create(OpCodes.Ldfld, fieldRef));
worker.Append(worker.Create(OpCodes.Call, writeFunc));
}
else

View File

@ -0,0 +1,8 @@
fileFormatVersion: 2
guid: c7ea1d20d9a008f479d65016372c9af9
folderAsset: yes
DefaultImporter:
externalObjects: {}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,3 @@
We need this extra Assembly when testing multiple assembles.
Weaver is currently unable to use reference to assembles that are not in `CompilationPipeline.GetAssemblies()`

View File

@ -0,0 +1,7 @@
fileFormatVersion: 2
guid: 4020f17b5b2fe4842bea9d1e7e6102d8
TextScriptImporter:
externalObjects: {}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,7 @@
namespace Mirror.Weaver.Tests.Extra
{
public struct SomeData
{
public int usefulNumber;
}
}

View File

@ -0,0 +1,11 @@
fileFormatVersion: 2
guid: 502dcd3fe9ec0e04185f2afd167c7aad
MonoImporter:
externalObjects: {}
serializedVersion: 2
defaultReferences: []
executionOrder: 0
icon: {instanceID: 0}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,7 @@
namespace Mirror.Weaver.Tests.Extra
{
public class SomeDataClass
{
public int usefulNumber;
}
}

View File

@ -0,0 +1,11 @@
fileFormatVersion: 2
guid: 13374a84968c7684782a54136703257d
MonoImporter:
externalObjects: {}
serializedVersion: 2
defaultReferences: []
executionOrder: 0
icon: {instanceID: 0}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,12 @@
namespace Mirror.Weaver.Tests.Extra
{
public class SomeDataClassWithConstructor
{
public int usefulNumber;
public SomeDataClassWithConstructor()
{
// empty
}
}
}

View File

@ -0,0 +1,11 @@
fileFormatVersion: 2
guid: 1e73b5c53b6d21b47ab1442e5ad2265a
MonoImporter:
externalObjects: {}
serializedVersion: 2
defaultReferences: []
executionOrder: 0
icon: {instanceID: 0}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,19 @@
namespace Mirror.Weaver.Tests.Extra
{
public struct SomeDataWithWriter
{
public int usefulNumber;
}
public static class ReadWrite
{
public static void WriteSomeData(this NetworkWriter writer, SomeDataWithWriter itemData)
{
writer.WriteInt32(itemData.usefulNumber);
}
public static SomeDataWithWriter ReadSomeData(this NetworkReader reader)
{
return new SomeDataWithWriter { usefulNumber = reader.ReadInt32() };
}
}
}

View File

@ -0,0 +1,11 @@
fileFormatVersion: 2
guid: 15ed55793bf39a04f9ef337575c3c6e8
MonoImporter:
externalObjects: {}
serializedVersion: 2
defaultReferences: []
executionOrder: 0
icon: {instanceID: 0}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,16 @@
{
"name": "WeaverTestExtraAssembly",
"references": [
"Mirror"
],
"optionalUnityReferences": [],
"includePlatforms": [
"Editor"
],
"excludePlatforms": [],
"allowUnsafeCode": false,
"overrideReferences": false,
"precompiledReferences": [],
"autoReferenced": true,
"defineConstraints": []
}

View File

@ -0,0 +1,7 @@
fileFormatVersion: 2
guid: 4887344a35d890449b35f8b1b0a3355a
AssemblyDefinitionImporter:
externalObjects: {}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -0,0 +1,14 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CanUseCustomReadWriteForTypesFromDifferentAssemblies : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeDataWithWriter data)
{
// empty
}
}
}

View File

@ -0,0 +1,19 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CreatesForClass : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeOtherData data)
{
// empty
}
}
public class SomeOtherData
{
public int usefulNumber;
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CreatesForClassFromDifferentAssemblies : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeDataClass data)
{
// empty
}
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CreatesForClassFromDifferentAssembliesWithValidConstructor : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeDataClassWithConstructor data)
{
// empty
}
}
}

View File

@ -0,0 +1,24 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CreatesForClassWithValidConstructor : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeOtherData data)
{
// empty
}
}
public class SomeOtherData
{
public int usefulNumber;
public SomeOtherData()
{
// empty
}
}
}

View File

@ -0,0 +1,20 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
using UnityEngine;
namespace MirrorTest
{
public class CreatesForInheritedFromScriptableObject : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(DataScriptableObject data)
{
// empty
}
}
public class DataScriptableObject :ScriptableObject
{
public int usefulNumber;
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CreatesForStructFromDifferentAssemblies : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeData data)
{
// empty
}
}
}

View File

@ -0,0 +1,19 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class CreatesForStructs : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeOtherData data)
{
// empty
}
}
public struct SomeOtherData
{
public int usefulNumber;
}
}

View File

@ -0,0 +1,24 @@
using Mirror;
using Mirror.Weaver.Tests.Extra;
namespace MirrorTest
{
public class GivesErrorForClassWithNoValidConstructor : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(SomeOtherData data)
{
// empty
}
}
public class SomeOtherData
{
public int usefulNumber;
public SomeOtherData(int usefulNumber)
{
this.usefulNumber = usefulNumber;
}
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using UnityEngine;
namespace MirrorTest
{
public class GivesErrorWhenUsingMonoBehaviour : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(MonoBehaviour behaviour)
{
// empty
}
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using UnityEngine;
namespace MirrorTest
{
public class GivesErrorWhenUsingObject : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(Object obj)
{
// empty
}
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using UnityEngine;
namespace MirrorTest
{
public class GivesErrorWhenUsingScriptableObject : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(ScriptableObject obj)
{
// empty
}
}
}

View File

@ -0,0 +1,19 @@
using Mirror;
using UnityEngine;
namespace MirrorTest
{
public class GivesErrorWhenUsingTypeInheritedFromMonoBehaviour : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(MyBehaviour behaviour)
{
// empty
}
}
public class MyBehaviour : MonoBehaviour
{
public int usefulNumber;
}
}

View File

@ -0,0 +1,14 @@
using Mirror;
using UnityEngine;
namespace MirrorTest
{
public class GivesErrorForClassWithNoValidConstructor : NetworkBehaviour
{
[ClientRpc]
public void RpcDoSomething(Material material)
{
// empty
}
}
}

View File

@ -0,0 +1,141 @@
using NUnit.Framework;
namespace Mirror.Weaver.Tests
{
public class WeaverGeneratedReaderWriterTests : WeaverTests
{
protected void BuildAndWeaveTestAssembly(string testScript)
{
const string folderName = "GeneratedReaderWriter";
BuildAndWeaveTestAssembly(folderName, testScript);
}
[SetUp]
public void TestSetup()
{
WeaverAssembler.AddReferencesByAssemblyName(new string[] { "WeaverTestExtraAssembly.dll" });
BuildAndWeaveTestAssembly(TestContext.CurrentContext.Test.Name);
}
[Test]
public void CreatesForStructs()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void CreatesForClass()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void CreatesForClassWithValidConstructor()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void GivesErrorForClassWithNoValidConstructor()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Has.Some.Match(@"Mirror\.Weaver error: MirrorTest\.SomeOtherData can't be deserialized because it has no default constructor"));
}
[Test]
public void CreatesForInheritedFromScriptableObject()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void CreatesForStructFromDifferentAssemblies()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void CreatesForClassFromDifferentAssemblies()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void CreatesForClassFromDifferentAssembliesWithValidConstructor()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void CanUseCustomReadWriteForTypesFromDifferentAssemblies()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
public void GivesErrorWhenUsingUnityAsset()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Has.Some.Match(@"Mirror\.Weaver error: UnityEngine\.Material can't be deserialized because it has no default constructor"));
}
[Test]
public void GivesErrorWhenUsingObject()
{
// TODO: decide if we want to block sending of Object
// would only want to be send as an arg as a base type for an Inherited object
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Has.Some.Match(@"Cannot generate writer for UnityEngine\.Object\. Use a supported type or provide a custom writer"));
}
[Test]
public void GivesErrorWhenUsingScriptableObject()
{
// TODO: decide if we want to block sending of ScripableObject
// would only want to be send as an arg as a base type for an Inherited object
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Has.Some.Match(@"Cannot generate writer for UnityEngine\.ScriptableObject\. Use a supported type or provide a custom writer"));
}
[Test]
public void GivesErrorWhenUsingMonoBehaviour()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Has.Some.Match(@"Cannot generate writer for component type UnityEngine\.MonoBehaviour\. Use a supported type or provide a custom writer"));
}
[Test]
public void GivesErrorWhenUsingTypeInheritedFromMonoBehaviour()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Has.Some.Match(@"Cannot generate writer for component type MirrorTest\.MyBehaviour\. Use a supported type or provide a custom writer"));
}
[Test]
[Ignore("Not Implemented")]
public void ExcludesNonSerializedFields()
{
// we test this by having a not allowed type in the class, but mark it with NonSerialized
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
[Test]
[Ignore("Not Implemented")]
public void IncludesPrivateSerializeField()
{
// Need to read Assemably and check that fields are added to writer
Assert.That(CompilationFinishedHook.WeaveFailed, Is.False);
Assert.That(weaverErrors, Is.Empty);
}
}
}

View File

@ -0,0 +1,11 @@
fileFormatVersion: 2
guid: 0ee83eef537268344880fa60f2c7a059
MonoImporter:
externalObjects: {}
serializedVersion: 2
defaultReferences: []
executionOrder: 0
icon: {instanceID: 0}
userData:
assetBundleName:
assetBundleVariant:

View File

@ -1,4 +1,4 @@
using NUnit.Framework;
using NUnit.Framework;
namespace Mirror.Weaver.Tests
{
@ -18,21 +18,6 @@ public void MessageSelfReferencing()
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.PrefabClone has field $MirrorTest.PrefabClone MirrorTest.PrefabClone::selfReference that references itself"));
}
[Test]
public void MessageInvalidSerializeFieldType()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: System.AccessViolationException MirrorTest.PrefabClone::invalidField has unsupported type"));
}
[Test]
public void MessageInvalidDeserializeFieldType()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: System.AccessViolationException is not a supported type"));
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: System.AccessViolationException MirrorTest.PrefabClone::invalidField has unsupported type"));
}
[Test]
public void MessageMemberGeneric()
{

View File

@ -1,27 +0,0 @@
using System;
using System.Collections;
using UnityEngine;
using Mirror;
namespace MirrorTest
{
class PrefabClone : MessageBase
{
public uint netId;
public Guid assetId;
public Vector3 position;
public Quaternion rotation;
public AccessViolationException invalidField;
public byte[] payload;
// this will cause generate serialization to be skipped, testing generate deserialization
public override void Serialize(NetworkWriter writer)
{
writer.WritePackedUInt32(netId);
writer.WriteGuid(assetId);
writer.WriteVector3(position);
writer.WriteQuaternion(rotation);
writer.WriteBytesAndSize(payload);
}
}
}

View File

@ -1,27 +0,0 @@
using System;
using System.Collections;
using UnityEngine;
using Mirror;
namespace MirrorTest
{
class PrefabClone : MessageBase
{
public uint netId;
public Guid assetId;
public Vector3 position;
public Quaternion rotation;
public AccessViolationException invalidField;
public byte[] payload;
// this will guarantee only serialize will be generated (even though weaver does serialize first)
public override void Deserialize(NetworkReader reader)
{
netId = reader.ReadPackedUInt32();
assetId = reader.ReadGuid();
position = reader.ReadVector3();
rotation = reader.ReadQuaternion();
payload = reader.ReadBytesAndSize();
}
}
}

View File

@ -1,4 +1,4 @@
using NUnit.Framework;
using NUnit.Framework;
namespace Mirror.Weaver.Tests
{
@ -92,7 +92,7 @@ public void NetworkBehaviourTargetRpcParamRef()
public void NetworkBehaviourTargetRpcParamAbstract()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.MirrorTestPlayer/AbstractClass can't be deserialized because i has no default constructor"));
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.MirrorTestPlayer/AbstractClass can't be deserialized because it has no default constructor"));
}
[Test]
@ -162,7 +162,7 @@ public void NetworkBehaviourClientRpcParamRef()
public void NetworkBehaviourClientRpcParamAbstract()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.MirrorTestPlayer/AbstractClass can't be deserialized because i has no default constructor"));
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.MirrorTestPlayer/AbstractClass can't be deserialized because it has no default constructor"));
}
[Test]
@ -211,7 +211,7 @@ public void NetworkBehaviourCmdParamRef()
public void NetworkBehaviourCmdParamAbstract()
{
Assert.That(CompilationFinishedHook.WeaveFailed, Is.True);
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.MirrorTestPlayer/AbstractClass can't be deserialized because i has no default constructor"));
Assert.That(weaverErrors, Contains.Item("Mirror.Weaver error: MirrorTest.MirrorTestPlayer/AbstractClass can't be deserialized because it has no default constructor"));
}
[Test]