From b88c9b2cecdef930be98eea368d2f29a84972067 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Leclercq?= Date: Mon, 13 May 2019 16:50:19 +0200 Subject: [PATCH] Sdk/World: Fix entity kill and invalidation bug --- ChangeLog.md | 1 + SDK/include/NDK/World.hpp | 10 ++++++++-- SDK/include/NDK/World.inl | 10 +++++----- SDK/src/NDK/World.cpp | 18 ++++++++++-------- tests/SDK/NDK/World.cpp | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 57 insertions(+), 15 deletions(-) diff --git a/ChangeLog.md b/ChangeLog.md index 6f9dfa1e3..c9628ed0c 100644 --- a/ChangeLog.md +++ b/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: diff --git a/SDK/include/NDK/World.hpp b/SDK/include/NDK/World.hpp index 98e6d87ac..891c3df11 100644 --- a/SDK/include/NDK/World.hpp +++ b/SDK/include/NDK/World.hpp @@ -98,6 +98,12 @@ namespace Ndk inline void InvalidateSystemOrder(); void ReorderSystems(); + struct DoubleBitset + { + Nz::Bitset front; + Nz::Bitset back; + }; + struct EntityBlock { EntityBlock(Entity&& e) : @@ -119,9 +125,9 @@ namespace Ndk std::vector> m_waitingEntities; EntityList m_aliveEntities; ProfilerData m_profilerData; - Nz::Bitset m_dirtyEntities; + DoubleBitset m_dirtyEntities; Nz::Bitset m_freeEntityIds; - Nz::Bitset m_killedEntities; + DoubleBitset m_killedEntities; bool m_orderedSystemsUpdated; bool m_isProfilerEnabled; }; diff --git a/SDK/include/NDK/World.inl b/SDK/include/NDK/World.inl index 860454a0d..1c3e446e1 100644 --- a/SDK/include/NDK/World.inl +++ b/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() diff --git a/SDK/src/NDK/World.cpp b/SDK/src/NDK/World.cpp index 579985297..d8a1ec0e3 100644 --- a/SDK/src/NDK/World.cpp +++ b/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(j)); removedComponents.Reset(); @@ -262,7 +264,7 @@ namespace Ndk } } } - m_dirtyEntities.Reset(); + m_dirtyEntities.back.Clear(); } /*! diff --git a/tests/SDK/NDK/World.cpp b/tests/SDK/NDK/World.cpp index 185717609..f98a7e2e1 100644 --- a/tests/SDK/NDK/World.cpp +++ b/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()); + } + } + } + } }