From d5fba47a4c4ffb07109d679a8c25e5812844caae Mon Sep 17 00:00:00 2001 From: Jon Keon Date: Mon, 1 May 2023 13:54:18 -0400 Subject: [PATCH 1/2] Detecting when we have a collection hidden inside. --- .../Lifecycle/Migration/MigrationUtil.cs | 55 +++++++++++++------ 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/Scripts/Runtime/Entities/Lifecycle/Migration/MigrationUtil.cs b/Scripts/Runtime/Entities/Lifecycle/Migration/MigrationUtil.cs index efb7c50e..6309b82f 100644 --- a/Scripts/Runtime/Entities/Lifecycle/Migration/MigrationUtil.cs +++ b/Scripts/Runtime/Entities/Lifecycle/Migration/MigrationUtil.cs @@ -1,5 +1,6 @@ using Anvil.CSharp.Logging; using System; +using System.Reflection; using Unity.Burst; using Unity.Collections; using Unity.Collections.LowLevel.Unsafe; @@ -36,14 +37,14 @@ private static void Init() SharedTypeOffsetInfo.REF.Data = s_TypeOffsetsLookup; UpdateSharedStatics(); } - + private static void CurrentDomain_OnDomainUnload(object sender, EventArgs e) { SharedTypeOffsetInfo.REF.Data = default; SharedEntityOffsetInfo.REF.Data = default; SharedBlobAssetRefInfo.REF.Data = default; SharedWeakAssetRefInfo.REF.Data = default; - + if (s_TypeOffsetsLookup.IsCreated) { s_TypeOffsetsLookup.Dispose(); @@ -61,7 +62,7 @@ private static void CurrentDomain_OnDomainUnload(object sender, EventArgs e) s_WeakAssetRefOffsetList.Dispose(); } } - + /// /// Registers the Type that may contain Entity references so that it can be used with /// to remap Entity references. @@ -72,7 +73,7 @@ public static void RegisterTypeForEntityPatching() { RegisterTypeForEntityPatching(typeof(T)); } - + /// /// /// Occurs when the Type is not a Value type. @@ -84,6 +85,8 @@ public static void RegisterTypeForEntityPatching(Type type) throw new InvalidOperationException($"Type {type.GetReadableName()} must be a value type in order to register for Entity Patching."); } + ScanForCollections(string.Empty, type); + long typeHash = BurstRuntime.GetHashCode64(type); //We've already added this type, no need to do so again if (s_TypeOffsetsLookup.ContainsKey(typeHash)) @@ -113,23 +116,43 @@ public static void RegisterTypeForEntityPatching(Type type) new TypeOffsetInfo( entityOffsetStartIndex, s_EntityOffsetList.Length)); - + //The size of the underlying data could have changed such that we re-allocated the memory, so we'll update //our shared statics UpdateSharedStatics(); } + private static void ScanForCollections(string parentPathString, Type type) + { + //One of these fields might be a collection or contain a collection, best way to tell is to scan it and see if it has a pointer + FieldInfo[] fields = type.GetFields(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); + foreach (FieldInfo field in fields) + { + Type fieldType = field.FieldType; + if (fieldType.IsPrimitive) + { + continue; + } + if (fieldType.IsPointer) + { + Debug.LogWarning($"{parentPathString}/{type.GetReadableName()} has a field named {field.Name} which is a pointer. This is probably a collection. As a result, we cannot automatically patch any entity references inside this collection. It would need to be handled manually until automatic work can handle in from https://github.com/decline-cookies/anvil-unity-dots/issues/233."); + continue; + } + ScanForCollections($"{parentPathString}/{type.GetReadableName()}", fieldType); + } + } + private static unsafe void UpdateSharedStatics() { SharedEntityOffsetInfo.REF.Data = new IntPtr(s_EntityOffsetList.GetUnsafePtr()); SharedBlobAssetRefInfo.REF.Data = new IntPtr(s_BlobAssetRefOffsetList.GetUnsafePtr()); SharedWeakAssetRefInfo.REF.Data = new IntPtr(s_WeakAssetRefOffsetList.GetUnsafePtr()); } - + //************************************************************************************************************* // BURST RUNTIME CALLS //************************************************************************************************************* - + /// /// Checks if the Entity was remapped by Unity during a world transfer. /// @@ -194,7 +217,7 @@ public static unsafe void PatchEntityReferences(this ref T instance, ref Nati *entityPtr = EntityRemapUtility.RemapEntity(ref remapArray, *entityPtr); } } - + //************************************************************************************************************* // HELPER TYPES //************************************************************************************************************* @@ -215,35 +238,33 @@ public TypeOffsetInfo(int entityOffsetStartIndex, int entityOffsetEndIndex) EntityOffsetEndIndex = entityOffsetEndIndex; } } - + //************************************************************************************************************* // SHARED STATIC REQUIREMENTS //************************************************************************************************************* - + // ReSharper disable once ConvertToStaticClass // ReSharper disable once ClassNeverInstantiated.Local private sealed class MigrationUtilContext { - private MigrationUtilContext() - { - } + private MigrationUtilContext() { } } - + private sealed class SharedTypeOffsetInfo { public static readonly SharedStatic> REF = SharedStatic>.GetOrCreate(); } - + private sealed class SharedEntityOffsetInfo { public static readonly SharedStatic REF = SharedStatic.GetOrCreate(); } - + private sealed class SharedBlobAssetRefInfo { public static readonly SharedStatic REF = SharedStatic.GetOrCreate(); } - + private sealed class SharedWeakAssetRefInfo { public static readonly SharedStatic REF = SharedStatic.GetOrCreate(); From 01a1d348d61675447171c36f74c49138ffa679f7 Mon Sep 17 00:00:00 2001 From: Jon Keon Date: Mon, 1 May 2023 16:29:20 -0400 Subject: [PATCH 2/2] PR comments --- .../Lifecycle/Migration/WorldEntityMigrationSystem.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Scripts/Runtime/Entities/Lifecycle/Migration/WorldEntityMigrationSystem.cs b/Scripts/Runtime/Entities/Lifecycle/Migration/WorldEntityMigrationSystem.cs index 5e958f24..66aa0b3c 100644 --- a/Scripts/Runtime/Entities/Lifecycle/Migration/WorldEntityMigrationSystem.cs +++ b/Scripts/Runtime/Entities/Lifecycle/Migration/WorldEntityMigrationSystem.cs @@ -1,6 +1,7 @@ using Anvil.CSharp.Logging; using System; using System.Collections.Generic; +using System.Diagnostics; using System.Reflection; using Unity.Burst; using Unity.Collections; @@ -8,6 +9,7 @@ using Unity.Entities; using Unity.Jobs; using UnityEngine; +using Debug = UnityEngine.Debug; namespace Anvil.Unity.DOTS.Entities { @@ -214,6 +216,8 @@ public static void RegisterForEntityPatching(Type type) UpdateSharedStatics(); } + //TODO: #233 - This likely won't be a safety function when fully implemented as we'll need to store the type offsets for patching + [Conditional("ANVIL_DEBUG_SAFETY")] private static void ScanForCollections(string parentPathString, Type type) { //One of these fields might be a collection or contain a collection, best way to tell is to scan it and see if it has a pointer @@ -227,7 +231,8 @@ private static void ScanForCollections(string parentPathString, Type type) } if (fieldType.IsPointer) { - Debug.LogWarning($"{parentPathString}/{type.GetReadableName()} has a field named {field.Name} which is a pointer. This is probably a collection. As a result, we cannot automatically patch any entity references inside this collection. It would need to be handled manually until automatic work can handle in from https://github.com/decline-cookies/anvil-unity-dots/issues/233."); + //TODO: #233 - Update instructions when implemented. + Debug.LogWarning($"{parentPathString}/{type.GetReadableName()} has a field named {field.Name} which is a pointer. This is probably a collection. As a result, we cannot automatically patch any entity references inside this collection."); continue; } ScanForCollections($"{parentPathString}/{type.GetReadableName()}", fieldType);