[email protected]

## [1.2.4] - 2024-08-14

* Debug proxies (used by external debuggers) were sometimes using invalid field offsets when inspecting structs in blob assets. This led to incorrect values being reported in debugger watch windows. In particular, this would be triggered by the use of bool fields in blob asset structs.
* Entity version numbers could go back to 1 after reallocation in some edge cases.
* When building a content update, a temporary path was getting created in the drive root instead of the Library folder.  This would also cause content update builds to grow in size every time they were built.  The folder is now created in the Library correctly.
* Error in build when sprites are contained in subscenes has been removed.
* Regression in compilation time with assemblies with lots of system methods.
* EntityComponentStore leaked memory during domain reload.

CHANGELOG.md

@@ -4,6 +4,17 @@ uid: changelog
 
 # Changelog
 
+
+## [1.2.4] - 2024-08-14
+
+* Debug proxies (used by external debuggers) were sometimes using invalid field offsets when inspecting structs in blob assets. This led to incorrect values being reported in debugger watch windows. In particular, this would be triggered by the use of bool fields in blob asset structs.
+* Entity version numbers could go back to 1 after reallocation in some edge cases.
+* When building a content update, a temporary path was getting created in the drive root instead of the Library folder.  This would also cause content update builds to grow in size every time they were built.  The folder is now created in the Library correctly.
+* Error in build when sprites are contained in subscenes has been removed.
+* Regression in compilation time with assemblies with lots of system methods.
+* EntityComponentStore leaked memory during domain reload.
+
+
 ## [1.2.3] - 2024-05-30
 
 ### Fixed
@@ -20,6 +31,7 @@ uid: changelog
 
 ## [1.2.1] - 2024-04-26
 
+
 ### Changed
 
 * Updated Burst dependency to version 1.8.13

Unity.Entities.Editor.Tests/Hierarchy/Model/HierarchyDataModeFilteringTests.cs

@@ -46,22 +46,21 @@ void BuildTestHierarchy(bool isSubSceneOpened, out HierarchyNodeStore.Immutable
             var emptyArchetype = m_World.EntityManager.CreateArchetype();
             var entities = m_World.EntityManager.CreateEntity(emptyArchetype, 11, Allocator.Temp);
 
-            // This test relies on entities having specific indices and versions.
+            // This test relies on entities having specific indices.
             // But it also requires those entities to effectively exists.
             // As long as it runs in an empty world, it should be fine.
             // The following loop makes sure of that.
             for (int i = 0; i < entities.Length; i++)
             {
                 Assert.AreEqual(entities[i].Index, i);
-                Assert.AreEqual(entities[i].Version, 1);
             }
 
-            var entityA = m_HierarchyNodeStore.AddNode(new HierarchyNodeHandle(NodeKind.Entity, 5, 1), subSceneNode);
-            var entityB = m_HierarchyNodeStore.AddNode(new HierarchyNodeHandle(NodeKind.Entity, 6, 1), subSceneNode);
-            var entityC = m_HierarchyNodeStore.AddNode(new HierarchyNodeHandle(NodeKind.Entity, 7, 1));
-            var entityD = m_HierarchyNodeStore.AddNode(new HierarchyNodeHandle(NodeKind.Entity, 8, 1));
+            var entityA = m_HierarchyNodeStore.AddNode(HierarchyNodeHandle.FromEntity(entities[5]), subSceneNode);
+            var entityB = m_HierarchyNodeStore.AddNode(HierarchyNodeHandle.FromEntity(entities[6]), subSceneNode);
+            var entityC = m_HierarchyNodeStore.AddNode(HierarchyNodeHandle.FromEntity(entities[7]));
+            var entityD = m_HierarchyNodeStore.AddNode(HierarchyNodeHandle.FromEntity(entities[8]));
             dynamicSubSceneNode = m_HierarchyNodeStore.AddNode(new HierarchyNodeHandle(NodeKind.SubScene, 9), HierarchyNodeHandle.Root);
-            m_HierarchyNodeStore.AddNode(new HierarchyNodeHandle(NodeKind.Entity, 10, 1), dynamicSubSceneNode);
+            m_HierarchyNodeStore.AddNode(HierarchyNodeHandle.FromEntity(entities[10]), dynamicSubSceneNode);
             m_HierarchyNodeStore.SetSortIndex(entityA, 3);
             m_HierarchyNodeStore.SetSortIndex(entityB, 4);
             m_HierarchyNodeStore.SetSortIndex(entityC, 2);

Unity.Entities.Editor.Tests/Hierarchy/Model/HierarchyNodeStoreTests.cs

@@ -19,19 +19,6 @@ public void SetUp()
         {
             m_World = new World(nameof(HierarchyNodeStoreTests));
             m_HierarchyNodeStore = new HierarchyNodeStore(Allocator.Persistent);
-
-            var emptyArchetype = m_World.EntityManager.CreateArchetype();
-            var entities = m_World.EntityManager.CreateEntity(emptyArchetype, 11, Allocator.Temp);
-
-            // Some of these tests rely on entities having specific indices and versions.
-            // But they also require those entities to effectively exist.
-            // As long as they run in an empty world, they should be fine.
-            // The following loop makes sure of that.
-            for (int i = 0; i < entities.Length; i++)
-            {
-                Assert.AreEqual(entities[i].Index, i);
-                Assert.AreEqual(entities[i].Version, 1);
-            }
         }
 
         [TearDown]

Unity.Entities.Tests/BlobDebugTests.cs

@@ -12,6 +12,8 @@
 using Assert = NUnit.Framework.Assert;
 using Unity.Mathematics;
 using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
 using Unity.Entities.DebugProxies;
 using Unity.Entities.Serialization;
 using Unity.Entities.LowLevel.Unsafe;
@@ -51,6 +53,13 @@ struct BlobWithStringNestedInArray
         public BlobArray<BlobString> ArrayOfStrings;
     }
 
+    struct BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout
+    {
+        public bool a;
+        public bool b;
+        public int c;
+    }
+
     [Test]
     public void BlobAssetReferenceDebugProxy_CanDisplayInvalidValues()
     {
@@ -192,4 +201,55 @@ public void BlobAssetReferenceDebugProxy_WorksWithStringNestedInArray()
         var arr = (BlobArrayDebug<BlobString>)bs.Members[1].Value;
         CollectionAssert.AreEqual(new []{"One", "Two", "Three"}, arr.Entries);
     }
+
+    [Test]
+    public void BlobAssetReferenceDebugProxy_WorksWithTypesHavingADifferentMarshallingLayout()
+    {
+        // The struct contains the following fields:
+        // a : bool
+        // b : bool
+        // c : int
+        // In memory, boolean values might take only one byte, so the layout would be:
+        // a : size 1 offset 0
+        // b : size 1 offset 1
+        // c : size 4 offset 4
+        // The full size of the struct being 8 bytes, with two bytes of padding between b and c.
+
+        // That exact memory layout is not guaranteed as far as I can tell.
+
+        // But the marshalling struct uses 4 bytes for booleans, giving the following layout:
+        // a : size 4 offset 0
+        // b : size 4 offset 4
+        // c : size 4 offset 8
+        // The full size being now 12 bytes, with no padding.
+
+        // This test validates that debugging proxies for blobs using such a struct will display the
+        // right data. We used to mistakenly have a mix of marshalled and non-marshalled field offsets in there.
+
+        unsafe
+        {
+            var actualSize = sizeof(BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout);
+            var marshallSize = Marshal.SizeOf<BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout>();
+
+            // Making sure that we are testing the right stuff, the marshalled struct shouldn't be identical to the raw struct.
+            // If this test fails, I guess a more complicated struct will be required.
+            Assert.AreNotEqual(actualSize, marshallSize);
+        }
+
+        var builder = new BlobBuilder(Allocator.Temp);
+
+        {
+            ref var v = ref builder.ConstructRoot<BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout>();
+            v.a = true;
+            v.b = false;
+            v.c = 123;
+        }
+
+        var bar = builder.CreateBlobAssetReference<BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout>(Allocator.Temp);
+        var proxy = new BlobAssetReferenceProxy<BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout>(bar);
+        var bs = (BlobStruct<BlobStructWithALayoutThatDoesntMatchItsMarshallingLayout>) proxy.Value;
+
+        var array = bs.Members.Select(m => $"{m.Key}:{m.Value}").ToArray();
+        CollectionAssert.AreEquivalent(new []{ $"a:{true}", $"b:{false}", $"c:{123}" }, array);
+    }
 }

Unity.Entities/BlobsDebug.cs

@@ -138,7 +138,7 @@ void Init()
             m_Members = new BlobMember[fields.Length];
             for (int i = 0; i < m_Members.Length; i++)
             {
-                var offset = (byte*) m_BasePtr + Marshal.OffsetOf<T>(fields[i].Name).ToInt32();
+                var offset = (byte*) m_BasePtr + UnsafeUtility.GetFieldOffset(fields[i]);
                 m_Members[i] = new BlobMember
                 {
                     Key = fields[i].Name,
@@ -195,6 +195,11 @@ public object Value
 
     static class BlobProxy
     {
+        static unsafe object GetPrimitive<T>(IntPtr basePtr) where T : unmanaged
+        {
+            return *(T*)basePtr.ToPointer();
+        }
+
         static unsafe string UnpackString(void* basePtr)
         {
             // can't get the BlobString itself because that will trigger the Blob asset safety system
@@ -227,7 +232,8 @@ internal static unsafe object UnpackValue(void* basePtr, Type type)
                 return Activator.CreateInstance(structType, new IntPtr(basePtr));
             }
 
-            return Marshal.PtrToStructure(new IntPtr(basePtr), type);
+            var makePrimitive = typeof(BlobProxy).GetMethod("GetPrimitive", BindingFlags.NonPublic | BindingFlags.Static)!.MakeGenericMethod(type);
+            return makePrimitive.Invoke(null, new object[] { new IntPtr(basePtr) });
         }
     }
 }

Unity.Entities/DefaultWorldInitialization.cs

@@ -91,6 +91,10 @@ internal static void DomainUnloadOrPlayModeChangeShutdown()
 
             World.DisposeAllWorlds();
 
+#if!ENTITY_STORE_V1
+            EntityComponentStore.s_entityStore.Data.Dispose();
+#endif
+
 #if (UNITY_EDITOR || DEVELOPMENT_BUILD) && !DISABLE_ENTITIES_JOURNALING
             EntitiesJournaling.Shutdown();
 #endif
@@ -181,7 +185,6 @@ public static void AddSystemsToRootLevelSystemGroups(World world, IReadOnlyList<
             }
             AddSystemToRootLevelSystemGroupsInternal(world, systemTypeIndices);
         }
-
 
         /// <summary>
         /// Adds the collection of systems to the world by injecting them into the root level system groups
@@ -199,12 +202,11 @@ public static void AddSystemsToRootLevelSystemGroups(World world, params Type[]
             }
             AddSystemToRootLevelSystemGroupsInternal(world, indices);
         }
-
 
         /// <summary>
         /// Adds the collection of systems to the world by injecting them into the root level system groups
         /// (InitializationSystemGroup, SimulationSystemGroup and PresentationSystemGroup). This version avoids
-        /// unnecessary reflection. 
+        /// unnecessary reflection.
         /// </summary>
         /// <param name="world">The World in which the root-level system groups should be created.</param>
         /// <param name="systemTypes">The system types to create and add.</param>
@@ -243,7 +245,7 @@ internal static unsafe void AddSystemToRootLevelSystemGroupsInternal<T>(World wo
                     throw new InvalidOperationException("Bad type");
                 if (stype.IsManaged)
                     managedTypes.Add(stype);
-                else 
+                else
                     unmanagedTypes.Add(stype);
             }
 
@@ -297,7 +299,7 @@ internal static void AddSystemToRootLevelSystemGroupsInternal(World world, Nativ
         private static ComponentSystemGroup FindGroup(World world, SystemTypeIndex systemType, TypeManager.SystemAttribute attr)
         {
             var groupTypeIndex = attr.TargetSystemTypeIndex;
-            
+
             if (!TypeManager.IsSystemTypeIndex(groupTypeIndex) || !groupTypeIndex.IsGroup)
             {
                 throw new InvalidOperationException($"Invalid [{nameof(UpdateInGroupAttribute)}] attribute for {systemType}: target group must be derived from {nameof(ComponentSystemGroup)}.");
@@ -360,7 +362,7 @@ public static void DefaultLazyEditModeInitialize()
 
         /// <summary>
         /// Calculates a list of all systems filtered with WorldSystemFilterFlags, [DisableAutoCreation] etc. Prefer
-        /// GetAllSystemTypeIndices where possible to avoid extra reflection. 
+        /// GetAllSystemTypeIndices where possible to avoid extra reflection.
         /// </summary>
         /// <param name="filterFlags">The filter flags to search for.</param>
         /// <param name="requireExecuteInEditor">Optionally require that [WorldSystemFilter(WorldSystemFilterFlags.Editor)] is present on the system. This is used when creating edit mode worlds.</param>
@@ -377,7 +379,7 @@ public static IReadOnlyList<Type> GetAllSystems(WorldSystemFilterFlags filterFla
 
             return ret;
         }
-        
+
         /// <summary>
         /// Calculates a list of all systems filtered with WorldSystemFilterFlags, [DisableAutoCreation] etc.
         /// Prefer this over GetAllSystems if possible, to avoid extra reflection usage.

Unity.Entities/EntityComponentStoreEntities.cs

@@ -49,18 +49,15 @@ public void IntegrityCheck(int blockIndex)
 
                 var block = (DataBlock*)m_DataBlocks[blockIndex];
 
-                var emptyA = block == null;
-                var emptyB = m_EntityCount[blockIndex] == 0;
-
-                if (emptyA != emptyB)
+                if(block == null)
                 {
-                    Debug.Log($"block index {blockIndex} pointer {(ulong)block:X16} count {m_EntityCount[blockIndex]}");
-                }
+                    Assert.AreEqual(0, m_EntityCount[blockIndex]);
 
-                Assert.AreEqual(emptyA, emptyB);
+                    if(m_EntityCount[blockIndex] != 0)
+                    {
+                        Debug.Log($"block index {blockIndex} pointer {(ulong)block:X16} count {m_EntityCount[blockIndex]}");
+                    }
 
-                if (block == null)
-                {
                     return;
                 }
 
@@ -85,7 +82,7 @@ public void IntegrityCheck()
                 }
             }
 
-            public int EntityCount
+            public int DebugOnlyThreadUnsafeEntityCount
             {
                 get
                 {
@@ -127,13 +124,8 @@ internal Entity GetEntityByEntityIndex(int index)
                 var block = (DataBlock*)m_DataBlocks[blockIndex];
                 if (block == null) return Entity.Null;
 
-                var bitfield = block->allocated[indexInBlock / 64];
-                var mask = 1UL << (indexInBlock % 64);
-
-                if ((bitfield & mask) == 0) return Entity.Null;
-
                 var version = block->versions[indexInBlock];
-                return new Entity { Index = index, Version = version };
+                return ((uint)version & 1) == 0 ? Entity.Null : new Entity { Index = index, Version = version };
             }
 
             internal bool Exists(Entity entity)
@@ -144,14 +136,7 @@ internal bool Exists(Entity entity)
                 var block = (DataBlock*)m_DataBlocks[blockIndex];
                 if (block == null) return false;
 
-                var bitfield = block->allocated[indexInBlock / 64];
-                var mask = 1UL << (indexInBlock % 64);
-
-                if ((bitfield & mask) == 0) return false;
-
-                var version = block->versions[indexInBlock];
-                if (version != entity.Version) return false;
-
+                if (((uint)entity.Version & 1) == 0 || block->versions[indexInBlock] != entity.Version) return false;
                 return true;
             }
 
@@ -234,18 +219,16 @@ internal void AllocateEntities(Entity* entities, int totalCount, ChunkIndex chun
                         continue;
                     }
 
-                    DataBlock* block;
+                    DataBlock* block = (DataBlock*)m_DataBlocks[i];
 
-                    if (blockCount == 0)
+                    // Be careful that the block might exist even if the count is zero, checking the pointer
+                    // for null is the only valid way to tell if the block exists or not.
+                    if (block == null)
                     {
                         block = (DataBlock*)Memory.Unmanaged.Allocate(k_BlockSize, 8, Allocator.Persistent);
                         UnsafeUtility.MemClear(block, k_BlockSize);
                         m_DataBlocks[i] = (ulong)block;
                     }
-                    else
-                    {
-                        block = (DataBlock*)m_DataBlocks[i];
-                    }
 
                     int remainingCount = math.min(blockAvailable, count);
                     var allocated = block->allocated;
@@ -432,11 +415,7 @@ internal void DeallocateEntities(Entity* entities, int count)
                         }
                     }
 
-                    if (blockCount == 0)
-                    {
-                        Memory.Unmanaged.Free(block, Allocator.Persistent);
-                        m_DataBlocks[blockIndex] = 0;
-                    }
+                    // Do not deallocate the block even if it's empty. Versions should be preserved.
 
                     {
                         var resultCheck = Interlocked.CompareExchange(ref m_EntityCount[blockIndex], blockCount, k_BlockBusy);

Unity.Entities/SourceGenerators/Source~/SystemGenerator.Common/SystemDescription.cs

@@ -83,13 +83,14 @@ public string GetOrCreateEntityCommandBufferSystemField(ITypeSymbol ecbSystemTyp
             if (containingMember is MethodDeclarationSyntax { Body: not null } methodDeclarationSyntax)
             {
                 var methodStatements = methodDeclarationSyntax.Body.DescendantNodes().OfType<StatementSyntax>();
+                StatementSyntax lastStatement = null;
                 foreach (var statement in methodStatements)
                 {
                     lineDirectiveStatements.Add(statement);
-                    if (statement == methodStatements.Last())
-                        hiddenDirectiveStatements.Add(statement);
-
+                    lastStatement = statement;
                 }
+                if (lastStatement != null)
+                    hiddenDirectiveStatements.Add(lastStatement);
             }
         }