Fix cleanup function's return value

Unordered map does not preserve order in which items were inserted.
This is a problem because an entity id relocated to a previously deleted
one could be read as deleted when iterating through the unordered map.
Thus, it has been replaced with a queue.
This commit is contained in:
Stephen Seo 2017-10-31 13:25:27 +09:00
parent 2523831097
commit 1d2fbf7b52
2 changed files with 40 additions and 23 deletions

View file

@ -21,6 +21,7 @@
#include <unordered_set> #include <unordered_set>
#include <algorithm> #include <algorithm>
#include <thread> #include <thread>
#include <queue>
#include "Meta/Combine.hpp" #include "Meta/Combine.hpp"
#include "Meta/Matching.hpp" #include "Meta/Matching.hpp"
@ -282,21 +283,31 @@ namespace EC
<b>This function should be called periodically to correctly handle <b>This function should be called periodically to correctly handle
deletion of entities.</b> deletion of entities.</b>
The map returned by this function lists all entities that have The queue returned by this function lists all entities that have
changed as a result of calling this function. changed as a result of calling this function.
The size_t key refers to the original entity id of the entity,
the bool value refers to whether or not the entity is still alive, The first parameter in the tuple (bool) is true if the entity has
and the size_t value refers to the new entity id if the entity been relocated (entity id has changed) and is false if the entity
is still alive. has been deleted.
The second parameter (size_t) is the original entity id of the
entity. If the previous parameter was false for deleted, then this
refers to the id of that deleted entity. If the previous parameter
was true for relocated, then this refers to the previous id of the
relocated entity
The third parameter (size_t) is the new entity id of the entity if
the entity was relocated. If the entity was deleted then this
parameter is undefined as it is not used in that case.
*/ */
using CleanupReturnType = using CleanupReturnType =
std::unordered_map<std::size_t, std::pair<bool, std::size_t> >; std::queue<std::tuple<bool, std::size_t, std::size_t> >;
CleanupReturnType cleanup() CleanupReturnType cleanup()
{ {
CleanupReturnType changedMap; CleanupReturnType changedQueue;
if(currentSize == 0) if(currentSize == 0)
{ {
return changedMap; return changedQueue;
} }
std::size_t rhs = currentSize - 1; std::size_t rhs = currentSize - 1;
@ -309,10 +320,9 @@ namespace EC
if(rhs == 0) if(rhs == 0)
{ {
currentSize = 0; currentSize = 0;
return changedMap; return changedQueue;
} }
changedMap.insert(std::make_pair(rhs, std::make_pair( changedQueue.push(std::make_tuple(false, rhs, 0));
false, 0)));
std::get<BitsetType>(entities[rhs]).reset(); std::get<BitsetType>(entities[rhs]).reset();
--rhs; --rhs;
} }
@ -325,10 +335,8 @@ namespace EC
// lhs is marked for deletion // lhs is marked for deletion
// store deleted and changed id to map // store deleted and changed id to map
changedMap.insert(std::make_pair(lhs, std::make_pair( changedQueue.push(std::make_tuple(false, lhs, 0));
false, 0))); changedQueue.push(std::make_tuple(true, rhs, lhs));
changedMap.insert(std::make_pair(rhs, std::make_pair(
true, lhs)));
// swap lhs entity with rhs entity // swap lhs entity with rhs entity
std::swap(entities[lhs], entities.at(rhs)); std::swap(entities[lhs], entities.at(rhs));
@ -346,7 +354,7 @@ namespace EC
} }
currentSize = rhs + 1; currentSize = rhs + 1;
return changedMap; return changedQueue;
} }
/*! /*!

View file

@ -436,26 +436,35 @@ TEST(EC, DeletedEntityID)
auto e0 = manager.addEntity(); auto e0 = manager.addEntity();
auto e1 = manager.addEntity(); auto e1 = manager.addEntity();
auto e2 = manager.addEntity(); auto e2 = manager.addEntity();
auto e3 = manager.addEntity();
manager.deleteEntity(e0); manager.deleteEntity(e0);
manager.deleteEntity(e1);
auto changedMap = manager.cleanup(); auto changedQueue = manager.cleanup();
for(decltype(changedMap)::value_type& p : changedMap) while(!changedQueue.empty())
{ {
if(p.first == 0) auto t = changedQueue.front();
if(std::get<1>(t) == 0)
{ {
EXPECT_FALSE(p.second.first); EXPECT_FALSE(std::get<0>(t));
} }
else if(p.first == 2) else if(std::get<1>(t) == 1)
{ {
EXPECT_TRUE(p.second.first); EXPECT_FALSE(std::get<0>(t));
EXPECT_EQ(0, p.second.second);
} }
else
{
EXPECT_TRUE(std::get<0>(t));
}
changedQueue.pop();
} }
EXPECT_FALSE(manager.hasEntity(2)); EXPECT_FALSE(manager.hasEntity(2));
EXPECT_FALSE(manager.hasEntity(3));
EXPECT_TRUE(manager.hasEntity(0)); EXPECT_TRUE(manager.hasEntity(0));
EXPECT_TRUE(manager.hasEntity(1));
} }
TEST(EC, MultiThreaded) TEST(EC, MultiThreaded)