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.

diff --git a/src/EC/Manager.hpp b/src/EC/Manager.hpp
index 2adf0d2c9b8a998459e563f8b1cf973ae2205aef..cb6b9a74eec1de54588a045608c1dc3b0431f4ea 100644 (file)
--- a/src/EC/Manager.hpp
+++ b/src/EC/Manager.hpp
@@ -21,6 +21,7 @@
 #include <unordered_set>
 #include <algorithm>
 #include <thread>
+#include <queue>
 
 #include "Meta/Combine.hpp"
 #include "Meta/Matching.hpp"
@@ -282,21 +283,31 @@ namespace EC
             <b>This function should be called periodically to correctly handle
             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.
-            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,
-            and the size_t value refers to the new entity id if the entity
-            is still alive.
+
+            The first parameter in the tuple (bool) is true if the entity has
+            been relocated (entity id has changed) and is false if the entity
+            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 =
-            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 changedMap;
+            CleanupReturnType changedQueue;
             if(currentSize == 0)
             {
-                return changedMap;
+                return changedQueue;
             }
 
             std::size_t rhs = currentSize - 1;
@@ -309,10 +320,9 @@ namespace EC
                     if(rhs == 0)
                     {
                         currentSize = 0;
-                        return changedMap;
+                        return changedQueue;
                     }
-                    changedMap.insert(std::make_pair(rhs, std::make_pair(
-                        false, 0)));
+                    changedQueue.push(std::make_tuple(false, rhs, 0));
                     std::get<BitsetType>(entities[rhs]).reset();
                     --rhs;
                 }
@@ -325,10 +335,8 @@ namespace EC
                     // lhs is marked for deletion
 
                     // store deleted and changed id to map
-                    changedMap.insert(std::make_pair(lhs, std::make_pair(
-                        false, 0)));
-                    changedMap.insert(std::make_pair(rhs, std::make_pair(
-                        true, lhs)));
+                    changedQueue.push(std::make_tuple(false, lhs, 0));
+                    changedQueue.push(std::make_tuple(true, rhs, lhs));
 
                     // swap lhs entity with rhs entity
                     std::swap(entities[lhs], entities.at(rhs));
@@ -346,7 +354,7 @@ namespace EC
             }
             currentSize = rhs + 1;
 
-            return changedMap;
+            return changedQueue;
         }
 
         /*!

diff --git a/src/test/ECTest.cpp b/src/test/ECTest.cpp
index 8f8b8d109008245e5e50811308de06104a5f97ce..155e45e77c32d96f2e5d1bfa8a613551afdc6a5d 100644 (file)
--- a/src/test/ECTest.cpp
+++ b/src/test/ECTest.cpp
@@ -436,26 +436,35 @@ TEST(EC, DeletedEntityID)
     auto e0 = manager.addEntity();
     auto e1 = manager.addEntity();
     auto e2 = manager.addEntity();
+    auto e3 = manager.addEntity();
 
     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(std::get<0>(t));
+        }
+        else if(std::get<1>(t) == 1)
         {
-            EXPECT_FALSE(p.second.first);
+            EXPECT_FALSE(std::get<0>(t));
         }
-        else if(p.first == 2)
+        else
         {
-            EXPECT_TRUE(p.second.first);
-            EXPECT_EQ(0, p.second.second);
+            EXPECT_TRUE(std::get<0>(t));
         }
+        changedQueue.pop();
     }
 
     EXPECT_FALSE(manager.hasEntity(2));
+    EXPECT_FALSE(manager.hasEntity(3));
     EXPECT_TRUE(manager.hasEntity(0));
+    EXPECT_TRUE(manager.hasEntity(1));
 }
 
 TEST(EC, MultiThreaded)