From 751d36a844d6c6fa19c4c38b14db2a409a3e22cb Mon Sep 17 00:00:00 2001
From: Evan Hemsley <evan@moonside.games>
Date: Wed, 10 Jul 2019 18:22:08 -0700
Subject: [PATCH] fix crash when removing a specific component from an entity
 the same frame the entity is destroyed

---
 encompass-cs/ComponentManager.cs | 14 ++++++-------
 encompass-cs/EntityManager.cs    |  8 +++----
 encompass-cs/WorldBuilder.cs     |  4 ++--
 test/EngineTest.cs               | 36 ++++++++++++++++++++++++++++++--
 4 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/encompass-cs/ComponentManager.cs b/encompass-cs/ComponentManager.cs
index d880e31..a9ff095 100644
--- a/encompass-cs/ComponentManager.cs
+++ b/encompass-cs/ComponentManager.cs
@@ -18,18 +18,18 @@ namespace Encompass
         private readonly List<Guid> activeComponents = new List<Guid>();
         private readonly List<Guid> inactiveComponents = new List<Guid>();
 
-        private readonly List<Guid> componentsToActivate = new List<Guid>();
-        private readonly List<Guid> componentsToDeactivate = new List<Guid>();
-        private readonly List<Guid> componentsToRemove = new List<Guid>();
+        private readonly HashSet<Guid> componentsToActivate = new HashSet<Guid>();
+        private readonly HashSet<Guid> componentsToDeactivate = new HashSet<Guid>();
+        private readonly HashSet<Guid> componentsToRemove = new HashSet<Guid>();
 
         //shared references with EntityManager
-        private readonly List<Guid> entitiesWithAddedComponents;
-        private readonly List<Guid> entitiesWithRemovedComponents;
+        private readonly HashSet<Guid> entitiesWithAddedComponents;
+        private readonly HashSet<Guid> entitiesWithRemovedComponents;
 
         public ComponentManager(
             DrawLayerManager drawLayerManager,
-            List<Guid> entitiesWithAddedComponents,
-            List<Guid> entitiesWithRemovedComponents
+            HashSet<Guid> entitiesWithAddedComponents,
+            HashSet<Guid> entitiesWithRemovedComponents
         )
         {
             this.drawLayerManager = drawLayerManager;
diff --git a/encompass-cs/EntityManager.cs b/encompass-cs/EntityManager.cs
index 4cab8bb..3f0d107 100644
--- a/encompass-cs/EntityManager.cs
+++ b/encompass-cs/EntityManager.cs
@@ -12,15 +12,15 @@ namespace Encompass
         private readonly Dictionary<Type, HashSet<IEntityTracker>> componentTypeToEntityTrackers = new Dictionary<Type, HashSet<IEntityTracker>>();
         private readonly Dictionary<Guid, HashSet<IEntityTracker>> entityToEntityTrackers = new Dictionary<Guid, HashSet<IEntityTracker>>();
 
-        private readonly List<Guid> entitiesWithAddedComponents;
-        private readonly List<Guid> entitiesWithRemovedComponents;
+        private readonly HashSet<Guid> entitiesWithAddedComponents;
+        private readonly HashSet<Guid> entitiesWithRemovedComponents;
 
         private readonly ComponentManager componentManager;
 
         public EntityManager(
             ComponentManager componentManager,
-            List<Guid> entitiesWithAddedComponents,
-            List<Guid> entitiesWithRemovedComponents
+            HashSet<Guid> entitiesWithAddedComponents,
+            HashSet<Guid> entitiesWithRemovedComponents
         )
         {
             this.componentManager = componentManager;
diff --git a/encompass-cs/WorldBuilder.cs b/encompass-cs/WorldBuilder.cs
index b4cd02f..849dfa4 100644
--- a/encompass-cs/WorldBuilder.cs
+++ b/encompass-cs/WorldBuilder.cs
@@ -22,8 +22,8 @@ namespace Encompass
 
         public WorldBuilder()
         {
-            var entitiesWithAddedComponents = new List<Guid>();
-            var entitiesWithRemovedComponents = new List<Guid>();
+            var entitiesWithAddedComponents = new HashSet<Guid>();
+            var entitiesWithRemovedComponents = new HashSet<Guid>();
             drawLayerManager = new DrawLayerManager();
             componentManager = new ComponentManager(drawLayerManager, entitiesWithAddedComponents, entitiesWithRemovedComponents);
             entityManager = new EntityManager(componentManager, entitiesWithAddedComponents, entitiesWithRemovedComponents);
diff --git a/test/EngineTest.cs b/test/EngineTest.cs
index f34bb65..d9397bc 100644
--- a/test/EngineTest.cs
+++ b/test/EngineTest.cs
@@ -435,8 +435,40 @@ namespace Tests
 
             world.Update(0.01f);
 
-            Assert.That(results, Does.Not.Contain(new KeyValuePair<Guid, MockComponent>(componentID, mockComponent)));
-            Assert.That(results, Does.Not.Contain(new KeyValuePair<Guid, MockComponent>(componentBID, mockComponent)));
+            Assert.That(results, Does.Not.Contain((componentID, mockComponent)));
+            Assert.That(results, Does.Not.Contain((componentBID, mockComponent)));
+        }
+
+        class DestroyAndAddComponentEngine : Engine
+        {
+            public override void Update(double dt)
+            {
+                foreach (var componentPair in ReadComponents<DestroyerComponent>())
+                {
+                    var componentID = componentPair.Item1;
+                    var entity = GetEntityByComponentID(componentID);
+                    var (id, _) = entity.GetComponent<MockComponent>();
+                    entity.RemoveComponent(id);
+                    Destroy(entity.id);
+                }
+            }
+        }
+
+        [Test]
+        public void DestroyEntityWhileRemovingComponent()
+        {
+            var worldBuilder = new WorldBuilder();
+            worldBuilder.AddEngine(new DestroyAndAddComponentEngine());
+            worldBuilder.AddEngine(new ReaderEngine());
+
+            var entity = worldBuilder.CreateEntity();
+
+            entity.AddComponent(new DestroyerComponent());
+            entity.AddComponent(new MockComponent());
+
+            var world = worldBuilder.Build();
+
+            Assert.DoesNotThrow(() => world.Update(0.01));
         }
 
         static Entity entityFromComponentIDResult;