From 1af5b4ed2f81b4450881fb11fa9b4b7e198274cb Mon Sep 17 00:00:00 2001 From: James Frowen Date: Fri, 24 Apr 2020 08:30:18 +0100 Subject: [PATCH] fix: weaver syncLists now checks for SerializeItem in base class (#1768) * tests for override in base class * fixing overrides in base class * moving check up so that typedef cant be null for the check --- Assets/Mirror/Editor/Weaver/Extensions.cs | 36 +++++++++++++++ .../Processors/SyncDictionaryProcessor.cs | 4 +- .../Weaver/Processors/SyncListProcessor.cs | 9 ++-- .../Weaver/Processors/SyncObjectProcessor.cs | 44 +++++++++---------- .../Editor/Weaver/WeaverSyncListTests.cs | 7 +++ .../SyncListInheritanceWithOverrides.cs | 42 ++++++++++++++++++ 6 files changed, 113 insertions(+), 29 deletions(-) create mode 100644 Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests~/SyncListInheritanceWithOverrides.cs diff --git a/Assets/Mirror/Editor/Weaver/Extensions.cs b/Assets/Mirror/Editor/Weaver/Extensions.cs index 7ccd033a9..b188021ea 100644 --- a/Assets/Mirror/Editor/Weaver/Extensions.cs +++ b/Assets/Mirror/Editor/Weaver/Extensions.cs @@ -180,5 +180,41 @@ public static MethodDefinition GetMethod(this TypeDefinition td, string methodNa } return null; } + + /// + /// + /// + /// + /// + /// + /// + public static bool HasMethodInBaseType(this TypeDefinition td, string methodName, TypeReference stopAt) + { + TypeDefinition typedef = td; + while (typedef != null) + { + if (typedef.FullName == stopAt.FullName) + break; + + foreach (MethodDefinition md in typedef.Methods) + { + if (md.Name == methodName) + return true; + } + + try + { + TypeReference parent = typedef.BaseType; + typedef = parent?.Resolve(); + } + catch (AssemblyResolutionException) + { + // this can happen for pluins. + break; + } + } + + return false; + } } } diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs index b7ba67fc7..1588f4d0c 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs @@ -15,7 +15,7 @@ public static void Process(TypeDefinition td) if (resolver.GetGenericFromBaseClass(td, 0, Weaver.SyncDictionaryType, out TypeReference keyType)) { - SyncObjectProcessor.GenerateSerialization(td, keyType, "SerializeKey", "DeserializeKey"); + SyncObjectProcessor.GenerateSerialization(td, keyType, Weaver.SyncDictionaryType, "SerializeKey", "DeserializeKey"); } else { @@ -25,7 +25,7 @@ public static void Process(TypeDefinition td) if (resolver.GetGenericFromBaseClass(td, 1, Weaver.SyncDictionaryType, out TypeReference itemType)) { - SyncObjectProcessor.GenerateSerialization(td, itemType, "SerializeItem", "DeserializeItem"); + SyncObjectProcessor.GenerateSerialization(td, itemType, Weaver.SyncDictionaryType, "SerializeItem", "DeserializeItem"); } else { diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs index cff414344..89df84ab0 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs @@ -9,17 +9,18 @@ static class SyncListProcessor /// Generates serialization methods for synclists /// /// The synclist class - public static void Process(TypeDefinition td, TypeReference baseType) + /// the base SyncObject td inherits from + public static void Process(TypeDefinition td, TypeReference mirrorBaseType) { GenericArgumentResolver resolver = new GenericArgumentResolver(1); - if (resolver.GetGenericFromBaseClass(td, 0, baseType, out TypeReference itemType)) + if (resolver.GetGenericFromBaseClass(td, 0, mirrorBaseType, out TypeReference itemType)) { - SyncObjectProcessor.GenerateSerialization(td, itemType, "SerializeItem", "DeserializeItem"); + SyncObjectProcessor.GenerateSerialization(td, itemType, mirrorBaseType, "SerializeItem", "DeserializeItem"); } else { - Weaver.Error($"Could not find generic arguments for {baseType} using {td}"); + Weaver.Error($"Could not find generic arguments for {mirrorBaseType} using {td}"); } } } diff --git a/Assets/Mirror/Editor/Weaver/Processors/SyncObjectProcessor.cs b/Assets/Mirror/Editor/Weaver/Processors/SyncObjectProcessor.cs index f12286e05..948e5d36f 100644 --- a/Assets/Mirror/Editor/Weaver/Processors/SyncObjectProcessor.cs +++ b/Assets/Mirror/Editor/Weaver/Processors/SyncObjectProcessor.cs @@ -9,35 +9,33 @@ public static class SyncObjectProcessor /// Generates the serialization and deserialization methods for a specified generic argument /// /// The type of the class that needs serialization methods - /// Which generic argument to serialize, 0 is the first one - /// the type that has generic arguments + /// generic argument to serialize + /// the base SyncObject td inherits from /// The name of the serialize method /// The name of the deserialize method - public static void GenerateSerialization(TypeDefinition td, TypeReference itemType, string serializeMethod, string deserializeMethod) + public static void GenerateSerialization(TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType, string serializeMethod, string deserializeMethod) { Weaver.DLog(td, "SyncObjectProcessor Start item:" + itemType.FullName); - MethodReference writeItemFunc = GenerateSerialization(serializeMethod, td, itemType); + bool success = GenerateSerialization(serializeMethod, td, itemType, mirrorBaseType); if (Weaver.WeavingFailed) { return; } - MethodReference readItemFunc = GenerateDeserialization(deserializeMethod, td, itemType); + success |= GenerateDeserialization(deserializeMethod, td, itemType, mirrorBaseType); - if (readItemFunc == null || writeItemFunc == null) - return; - - Weaver.DLog(td, "SyncObjectProcessor Done"); + if (success) + Weaver.DLog(td, "SyncObjectProcessor Done"); } // serialization of individual element - static MethodReference GenerateSerialization(string methodName, TypeDefinition td, TypeReference itemType) + static bool GenerateSerialization(string methodName, TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType) { Weaver.DLog(td, " GenerateSerialization"); - MethodDefinition existing = td.GetMethod(methodName); - if (existing != null) - return existing; + bool existing = td.HasMethodInBaseType(methodName, mirrorBaseType); + if (existing) + return true; // this check needs to happen inside GenerateSerialization because @@ -45,7 +43,7 @@ static MethodReference GenerateSerialization(string methodName, TypeDefinition t if (itemType.IsGenericInstance) { Weaver.Error($"{td} Can not create Serialize or Deserialize for generic element. Override virtual methods with custom Serialize and Deserialize to use {itemType} in SyncList"); - return null; + return false; } MethodDefinition serializeFunc = new MethodDefinition(methodName, MethodAttributes.Public | @@ -68,27 +66,27 @@ static MethodReference GenerateSerialization(string methodName, TypeDefinition t else { Weaver.Error($"{td} cannot have item of type {itemType}. Use a type supported by mirror instead"); - return null; + return false; } serWorker.Append(serWorker.Create(OpCodes.Ret)); td.Methods.Add(serializeFunc); - return serializeFunc; + return true; } - static MethodReference GenerateDeserialization(string methodName, TypeDefinition td, TypeReference itemType) + static bool GenerateDeserialization(string methodName, TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType) { Weaver.DLog(td, " GenerateDeserialization"); - MethodDefinition existing = td.GetMethod(methodName); - if (existing != null) - return existing; + bool existing = td.HasMethodInBaseType(methodName, mirrorBaseType); + if (existing) + return true; // this check needs to happen inside GenerateDeserialization because // we need to check if user has made custom function above if (itemType.IsGenericInstance) { Weaver.Error($"{td} Can not create Serialize or Deserialize for generic element. Override virtual methods with custom Serialize and Deserialize to use {itemType} in SyncList"); - return null; + return false; } MethodDefinition deserializeFunction = new MethodDefinition(methodName, MethodAttributes.Public | @@ -111,11 +109,11 @@ static MethodReference GenerateDeserialization(string methodName, TypeDefinition else { Weaver.Error($"{td} cannot have item of type {itemType}. Use a type supported by mirror instead"); - return null; + return false; } td.Methods.Add(deserializeFunction); - return deserializeFunction; + return true; } } } diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests.cs index 8fb3fc7eb..d0171b72c 100644 --- a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests.cs +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests.cs @@ -175,6 +175,13 @@ public void SyncListInterfaceWithCustomMethods() Assert.That(weaverErrors, Is.Empty); } + [Test] + public void SyncListInheritanceWithOverrides() + { + Assert.That(CompilationFinishedHook.WeaveFailed, Is.False); + Assert.That(weaverErrors, Is.Empty); + } + [Test] public void SyncListErrorWhenUsingGenericListInNetworkBehaviour() { diff --git a/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests~/SyncListInheritanceWithOverrides.cs b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests~/SyncListInheritanceWithOverrides.cs new file mode 100644 index 000000000..2a1ed8605 --- /dev/null +++ b/Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests~/SyncListInheritanceWithOverrides.cs @@ -0,0 +1,42 @@ +using Mirror; +using UnityEngine; + +namespace MirrorTest +{ + class SyncListInheritanceWithOverrides : NetworkBehaviour + { + readonly SomeExtraList superSyncListString = new SomeExtraList(); + } + + // Type that cant have custom writer + public class MyBehaviourWithValue : NetworkBehaviour + { + public Vector3 target; + } + + public class SomeBaseList : SyncList + { + protected override void SerializeItem(NetworkWriter writer, MyBehaviourWithValue item) + { + writer.WriteUInt32(item.netId); + writer.WriteVector3(item.target); + } + protected override MyBehaviourWithValue DeserializeItem(NetworkReader reader) + { + NetworkIdentity item = NetworkIdentity.spawned[reader.ReadUInt32()]; + MyBehaviourWithValue behaviour = item.GetComponent(); + + behaviour.target = reader.ReadVector3(); + + return behaviour; + } + } + + // Sync List type is MyBehaviourWithValue + // MyBehaviourWithValue is an invalid type, so requires custom writers + // Custom writers exist in base class so SomeExtraList should work without errors + public class SomeExtraList : SomeBaseList + { + // do extra stuff here + } +}