Browse Source

storage: avoid emplacing extra entities when using a hint - close #1113

Michele Caini 2 years ago
parent
commit
91a095dba4

+ 9 - 11
src/entt/entity/storage.hpp

@@ -952,8 +952,14 @@ class basic_storage<Entity, Entity, Allocator>
     using underlying_iterator = typename underlying_type::basic_iterator;
     using underlying_iterator = typename underlying_type::basic_iterator;
 
 
     auto next() noexcept {
     auto next() noexcept {
-        ENTT_ASSERT(placeholder < underlying_type::traits_type::to_entity(null), "Invalid element");
-        return underlying_type::traits_type::combine(static_cast<typename underlying_type::traits_type::entity_type>(placeholder++), {});
+        entity_type entt = null;
+
+        do {
+            ENTT_ASSERT(placeholder < underlying_type::traits_type::to_entity(null), "Invalid element");
+            entt = underlying_type::traits_type::combine(static_cast<typename underlying_type::traits_type::entity_type>(placeholder++), {});
+        } while(base_type::current(entt) != underlying_type::traits_type::to_version(tombstone));
+
+        return entt;
     }
     }
 
 
 protected:
 protected:
@@ -1079,15 +1085,7 @@ public:
         if(hint == null || hint == tombstone) {
         if(hint == null || hint == tombstone) {
             return emplace();
             return emplace();
         } else if(const auto curr = underlying_type::traits_type::construct(underlying_type::traits_type::to_entity(hint), base_type::current(hint)); curr == tombstone) {
         } else if(const auto curr = underlying_type::traits_type::construct(underlying_type::traits_type::to_entity(hint), base_type::current(hint)); curr == tombstone) {
-            const auto pos = static_cast<size_type>(underlying_type::traits_type::to_entity(hint));
-            const auto entt = *base_type::try_emplace(hint, true);
-
-            for(; placeholder != pos;) {
-                base_type::try_emplace(next(), false);
-            }
-
-            placeholder = pos + 1u;
-            return entt;
+            return *base_type::try_emplace(hint, true);
         } else if(const auto idx = base_type::index(curr); idx < base_type::free_list()) {
         } else if(const auto idx = base_type::index(curr); idx < base_type::free_list()) {
             return emplace();
             return emplace();
         } else {
         } else {

+ 2 - 2
test/entt/entity/sigh_mixin.cpp

@@ -280,14 +280,14 @@ TEST(SighMixin, StorageEntity) {
 
 
     ASSERT_EQ(on_construct, 1u);
     ASSERT_EQ(on_construct, 1u);
     ASSERT_EQ(on_destroy, 0u);
     ASSERT_EQ(on_destroy, 0u);
-    ASSERT_EQ(pool.size(), 2u);
+    ASSERT_EQ(pool.size(), 1u);
     ASSERT_EQ(pool.free_list(), 1u);
     ASSERT_EQ(pool.free_list(), 1u);
 
 
     pool.erase(entt::entity{1});
     pool.erase(entt::entity{1});
 
 
     ASSERT_EQ(on_construct, 1u);
     ASSERT_EQ(on_construct, 1u);
     ASSERT_EQ(on_destroy, 1u);
     ASSERT_EQ(on_destroy, 1u);
-    ASSERT_EQ(pool.size(), 2u);
+    ASSERT_EQ(pool.size(), 1u);
     ASSERT_EQ(pool.free_list(), 0u);
     ASSERT_EQ(pool.free_list(), 0u);
 
 
     pool.push(traits_type::construct(0, 2));
     pool.push(traits_type::construct(0, 2));

+ 9 - 7
test/entt/entity/storage_entity.cpp

@@ -91,16 +91,16 @@ TEST(StorageEntity, Swap) {
     other.emplace(entt::entity{1});
     other.emplace(entt::entity{1});
     other.erase(entt::entity{2});
     other.erase(entt::entity{2});
 
 
-    ASSERT_EQ(pool.size(), 5u);
-    ASSERT_EQ(other.size(), 3u);
+    ASSERT_EQ(pool.size(), 1u);
+    ASSERT_EQ(other.size(), 2u);
 
 
     pool.swap(other);
     pool.swap(other);
 
 
     ASSERT_EQ(pool.type(), entt::type_id<void>());
     ASSERT_EQ(pool.type(), entt::type_id<void>());
     ASSERT_EQ(other.type(), entt::type_id<void>());
     ASSERT_EQ(other.type(), entt::type_id<void>());
 
 
-    ASSERT_EQ(pool.size(), 3u);
-    ASSERT_EQ(other.size(), 5u);
+    ASSERT_EQ(pool.size(), 2u);
+    ASSERT_EQ(other.size(), 1u);
 
 
     ASSERT_EQ(pool.index(entt::entity{1}), 0u);
     ASSERT_EQ(pool.index(entt::entity{1}), 0u);
     ASSERT_EQ(other.index(entt::entity{4}), 0u);
     ASSERT_EQ(other.index(entt::entity{4}), 0u);
@@ -152,7 +152,7 @@ TEST(StorageEntity, Emplace) {
     ASSERT_LT(pool.index(entt::entity{2}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{2}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{3}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{3}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{4}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{4}), pool.free_list());
-    ASSERT_GE(pool.index(entt::entity{5}), pool.free_list());
+    ASSERT_EQ(pool.current(entt::entity{5}), traits_type::to_version(entt::tombstone));
     ASSERT_LT(pool.index(traits_type::construct(6, 3)), pool.free_list());
     ASSERT_LT(pool.index(traits_type::construct(6, 3)), pool.free_list());
 
 
     ASSERT_EQ(pool.emplace(traits_type::construct(5, 2)), traits_type::construct(5, 2));
     ASSERT_EQ(pool.emplace(traits_type::construct(5, 2)), traits_type::construct(5, 2));
@@ -184,7 +184,7 @@ TEST(StorageEntity, TryEmplace) {
     ASSERT_LT(pool.index(entt::entity{1}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{1}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{2}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{2}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{3}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{3}), pool.free_list());
-    ASSERT_GE(pool.index(entt::entity{4}), pool.free_list());
+    ASSERT_EQ(pool.current(entt::entity{4}), traits_type::to_version(entt::tombstone));
     ASSERT_LT(pool.index(traits_type::construct(5, 3)), pool.free_list());
     ASSERT_LT(pool.index(traits_type::construct(5, 3)), pool.free_list());
 
 
     ASSERT_EQ(*pool.push(traits_type::construct(4, 2)), traits_type::construct(4, 2));
     ASSERT_EQ(*pool.push(traits_type::construct(4, 2)), traits_type::construct(4, 2));
@@ -265,9 +265,11 @@ TEST(StorageEntity, Insert) {
 
 
 TEST(StorageEntity, Pack) {
 TEST(StorageEntity, Pack) {
     entt::storage<entt::entity> pool;
     entt::storage<entt::entity> pool;
-    std::array entity{entt::entity{1}, entt::entity{3}, entt::entity{4}};
+    std::array entity{entt::entity{1}, entt::entity{3}, entt::entity{4}, entt::entity{2}};
 
 
     pool.push(entity.begin(), entity.end());
     pool.push(entity.begin(), entity.end());
+    pool.erase(entity[3u]);
+
     std::swap(entity[0u], entity[1u]);
     std::swap(entity[0u], entity[1u]);
 
 
     const auto to = pool.sort_as(entity.begin() + 1u, entity.end());
     const auto to = pool.sort_as(entity.begin() + 1u, entity.end());