From 91a095dba4c839ee2c58483ae116eea74a5c8bb1 Mon Sep 17 00:00:00 2001
From: Michele Caini <michele.caini@gmail.com>
Date: Wed, 21 Feb 2024 08:55:25 +0100
Subject: [PATCH] storage: avoid emplacing extra entities when using a hint -
 close #1113

---
 src/entt/entity/storage.hpp         | 20 +++++++++-----------
 test/entt/entity/sigh_mixin.cpp     |  4 ++--
 test/entt/entity/storage_entity.cpp | 16 +++++++++-------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp
index e6d9381c0..7e111e239 100644
--- a/src/entt/entity/storage.hpp
+++ b/src/entt/entity/storage.hpp
@@ -952,8 +952,14 @@ class basic_storage<Entity, Entity, Allocator>
     using underlying_iterator = typename underlying_type::basic_iterator;
 
     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:
@@ -1079,15 +1085,7 @@ public:
         if(hint == null || hint == tombstone) {
             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) {
-            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()) {
             return emplace();
         } else {
diff --git a/test/entt/entity/sigh_mixin.cpp b/test/entt/entity/sigh_mixin.cpp
index 10e808e51..06f7c7d21 100644
--- a/test/entt/entity/sigh_mixin.cpp
+++ b/test/entt/entity/sigh_mixin.cpp
@@ -280,14 +280,14 @@ TEST(SighMixin, StorageEntity) {
 
     ASSERT_EQ(on_construct, 1u);
     ASSERT_EQ(on_destroy, 0u);
-    ASSERT_EQ(pool.size(), 2u);
+    ASSERT_EQ(pool.size(), 1u);
     ASSERT_EQ(pool.free_list(), 1u);
 
     pool.erase(entt::entity{1});
 
     ASSERT_EQ(on_construct, 1u);
     ASSERT_EQ(on_destroy, 1u);
-    ASSERT_EQ(pool.size(), 2u);
+    ASSERT_EQ(pool.size(), 1u);
     ASSERT_EQ(pool.free_list(), 0u);
 
     pool.push(traits_type::construct(0, 2));
diff --git a/test/entt/entity/storage_entity.cpp b/test/entt/entity/storage_entity.cpp
index a0b76264a..e348ffea2 100644
--- a/test/entt/entity/storage_entity.cpp
+++ b/test/entt/entity/storage_entity.cpp
@@ -91,16 +91,16 @@ TEST(StorageEntity, Swap) {
     other.emplace(entt::entity{1});
     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);
 
     ASSERT_EQ(pool.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(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{3}), 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_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{2}), 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_EQ(*pool.push(traits_type::construct(4, 2)), traits_type::construct(4, 2));
@@ -265,9 +265,11 @@ TEST(StorageEntity, Insert) {
 
 TEST(StorageEntity, Pack) {
     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.erase(entity[3u]);
+
     std::swap(entity[0u], entity[1u]);
 
     const auto to = pool.sort_as(entity.begin() + 1u, entity.end());