Fix stupid iteration bug.

Iteration would terminate at ID size() rather than capacity(),
where size() is the number of allocated entities. Depending on where
deleted entities were, this would likely miss entities at the end of
the allocated vectors.

Fixes #10.

diff --git a/entityx/Entity.h b/entityx/Entity.h
index abb35528a4b53b2021517582bc636992d3a5607a..28c1b5d29678055a32d63b8c5c63384e1ef4abec 100644 (file)
--- a/entityx/Entity.h
+++ b/entityx/Entity.h
@@ -302,6 +302,7 @@ class EntityManager : boost::noncopyable, public enable_shared_from_this<EntityM
         while (i_ < capacity_ && !predicate()) {
           ++i_;
         }
+
         if (i_ < capacity_ && !unpackers_.empty()) {
           Entity::Id id = manager_->create_id(i_);
           for (auto &unpacker : unpackers_) {
@@ -333,9 +334,9 @@ class EntityManager : boost::noncopyable, public enable_shared_from_this<EntityM
     }
 
     Iterator begin() { return Iterator(manager_, predicates_, unpackers_, 0); }
-    Iterator end() { return Iterator(manager_, predicates_, unpackers_, manager_->size()); }
+    Iterator end() { return Iterator(manager_, predicates_, unpackers_, manager_->capacity()); }
     const Iterator begin() const { return Iterator(manager_, predicates_, unpackers_, 0); }
-    const Iterator end() const { return Iterator(manager_, predicates_, unpackers_, manager_->size()); }
+    const Iterator end() const { return Iterator(manager_, predicates_, unpackers_, manager_->capacity()); }
 
     template <typename A>
     View &unpack_to(ptr<A> &a) {

diff --git a/entityx/Entity_test.cc b/entityx/Entity_test.cc
index 701c15b207d3c673e268259597c725c209439279..bdc217ba7091287761705a9827cbe7b275500ffb 100644 (file)
--- a/entityx/Entity_test.cc
+++ b/entityx/Entity_test.cc
@@ -8,6 +8,7 @@
  * Author: Alec Thomas <alec@swapoff.org>
  */
 
+#include <algorithm>
 #include <iterator>
 #include <string>
 #include <utility>
@@ -369,3 +370,25 @@ TEST_F(EntityManagerTest, TestEntityDestroyAll) {
   ASSERT_FALSE(a.valid());
   ASSERT_FALSE(b.valid());
 }
+
+
+TEST_F(EntityManagerTest, TestEntityDestroyHole) {
+  std::vector<Entity> entities;
+
+  auto count = [this]() {
+    auto e = em->entities_with_components<Position>();
+    return std::count_if(e.begin(), e.end(), [] (const Entity &) { return true; });
+  };
+
+  for (int i = 0; i < 5000; i++) {
+    auto e = em->create();
+    e.assign<Position>();
+    entities.push_back(e);
+  }
+
+  ASSERT_EQ(count(), 5000);
+
+  entities[2500].destroy();
+
+  ASSERT_EQ(count(), 4999);
+}