Sdk/World: Fix entity kill and invalidation bug

ChangeLog.md

@@ -262,6 +262,7 @@ Nazara Development Kit:
 - Fixed GraphicsComponent not invalidating render queue on material change (causing crashes or visual errors)
 - Added CollisionComponent2D::SetGeomOffset and CollisionComponent2D::Recenter
 - Added LifetimeComponent and LifetimeSystem
+- Fixed a subtle bug regarding entities invalidation and kill (ex: if an entity #2 kills entity #1 during Entity::Destroy callbacks, entity #1 will survive destruction).
 
 # 0.4:
 

SDK/include/NDK/World.hpp

@@ -98,6 +98,12 @@ namespace Ndk
 			inline void InvalidateSystemOrder();
 			void ReorderSystems();
 
+			struct DoubleBitset
+			{
+				Nz::Bitset<Nz::UInt64> front;
+				Nz::Bitset<Nz::UInt64> back;
+			};
+
 			struct EntityBlock
 			{
 				EntityBlock(Entity&& e) :
@@ -119,9 +125,9 @@ namespace Ndk
 			std::vector<std::unique_ptr<EntityBlock>> m_waitingEntities;
 			EntityList m_aliveEntities;
 			ProfilerData m_profilerData;
-			Nz::Bitset<Nz::UInt64> m_dirtyEntities;
+			DoubleBitset m_dirtyEntities;
 			Nz::Bitset<Nz::UInt64> m_freeEntityIds;
-			Nz::Bitset<Nz::UInt64> m_killedEntities;
+			DoubleBitset m_killedEntities;
 			bool m_orderedSystemsUpdated;
 			bool m_isProfilerEnabled;
 	};

SDK/include/NDK/World.inl

@@ -308,7 +308,7 @@ namespace Ndk
 	inline void World::KillEntity(Entity* entity)
 	{
 		if (IsEntityValid(entity))
-			m_killedEntities.UnboundedSet(entity->GetId(), true);
+			m_killedEntities.front.UnboundedSet(entity->GetId(), true);
 	}
 
 	/*!
@@ -343,7 +343,7 @@ namespace Ndk
 	*/
 	inline bool World::IsEntityDying(EntityId id) const
 	{
-		return m_killedEntities.UnboundedTest(id);
+		return m_killedEntities.front.UnboundedTest(id);
 	}
 
 	/*!
@@ -467,13 +467,13 @@ namespace Ndk
 
 	inline void World::Invalidate()
 	{
-		m_dirtyEntities.Resize(m_entityBlocks.size(), false);
-		m_dirtyEntities.Set(true); // Activation of all bits
+		m_dirtyEntities.front.Resize(m_entityBlocks.size(), false);
+		m_dirtyEntities.front.Set(true); // Activation of all bits
 	}
 
 	inline void World::Invalidate(EntityId id)
 	{
-		m_dirtyEntities.UnboundedSet(id, true);
+		m_dirtyEntities.front.UnboundedSet(id, true);
 	}
 
 	inline void World::InvalidateSystemOrder()

SDK/src/NDK/World.cpp

@@ -135,9 +135,9 @@ namespace Ndk
 		m_waitingEntities.clear();
 
 		m_aliveEntities.Clear();
-		m_dirtyEntities.Clear();
+		m_dirtyEntities.front.Clear();
 		m_freeEntityIds.Clear();
-		m_killedEntities.Clear();
+		m_killedEntities.front.Clear();
 	}
 
 	/*!
@@ -210,7 +210,8 @@ namespace Ndk
 		}
 
 		// Handle killed entities before last call
-		for (std::size_t i = m_killedEntities.FindFirst(); i != m_killedEntities.npos; i = m_killedEntities.FindNext(i))
+		std::swap(m_killedEntities.front, m_killedEntities.back);
+		for (std::size_t i = m_killedEntities.back.FindFirst(); i != m_killedEntities.back.npos; i = m_killedEntities.back.FindNext(i))
 		{
 			NazaraAssert(i < m_entityBlocks.size(), "Entity index out of range");
 
@@ -220,12 +221,13 @@ namespace Ndk
 			entity->Destroy();
 
 			// Send back the identifier of the entity to the free queue
-			m_freeEntityIds.UnboundedSet(entity->GetId());
+			m_freeEntityIds.UnboundedSet(i);
 		}
-		m_killedEntities.Reset();
+		m_killedEntities.back.Clear();
 
 		// Handle of entities which need an update from the systems
-		for (std::size_t i = m_dirtyEntities.FindFirst(); i != m_dirtyEntities.npos; i = m_dirtyEntities.FindNext(i))
+		std::swap(m_dirtyEntities.front, m_dirtyEntities.back);
+		for (std::size_t i = m_dirtyEntities.back.FindFirst(); i != m_dirtyEntities.back.npos; i = m_dirtyEntities.back.FindNext(i))
 		{
 			NazaraAssert(i < m_entityBlocks.size(), "Entity index out of range");
 
@@ -236,7 +238,7 @@ namespace Ndk
 				continue;
 
 			Nz::Bitset<>& removedComponents = entity->GetRemovedComponentBits();
-			for (std::size_t j = removedComponents.FindFirst(); j != m_dirtyEntities.npos; j = removedComponents.FindNext(j))
+			for (std::size_t j = removedComponents.FindFirst(); j != m_dirtyEntities.back.npos; j = removedComponents.FindNext(j))
 				entity->DestroyComponent(static_cast<Ndk::ComponentIndex>(j));
 			removedComponents.Reset();
 
@@ -262,7 +264,7 @@ namespace Ndk
 				}
 			}
 		}
-		m_dirtyEntities.Reset();
+		m_dirtyEntities.back.Clear();
 	}
 
 	/*!

tests/SDK/NDK/World.cpp

@@ -126,4 +126,37 @@ SCENARIO("World", "[NDK][WORLD]")
 			}
 		}
 	}
+
+	GIVEN("An empty world")
+	{
+		Ndk::World world(false);
+
+		WHEN("We create two entities")
+		{
+			Ndk::EntityHandle a = world.CreateEntity();
+			REQUIRE(a->GetId() == 0);
+			Ndk::EntityHandle b = world.CreateEntity();
+			REQUIRE(b->GetId() == 1);
+
+			b->OnEntityDestruction.Connect([a](Ndk::Entity*)
+			{
+				REQUIRE(a.IsValid());
+				a->Kill();
+			});
+
+			THEN("We kill the second entity which will kill the first one")
+			{
+				b->Kill();
+				world.Refresh();
+
+				AND_THEN("Both entities should be dead next refresh")
+				{
+					world.Refresh();
+
+					REQUIRE_FALSE(a.IsValid());
+					REQUIRE_FALSE(b.IsValid());
+				}
+			}
+		}
+	}
 }