sparse_set/storage: move swap_only policy towards the base

src/entt/entity/sparse_set.hpp

@@ -200,9 +200,7 @@ class basic_sparse_set {
             std::uninitialized_fill(sparse[page], sparse[page] + traits_type::page_size, null);
         }
 
-        auto &elem = sparse[page][fast_mod(pos, traits_type::page_size)];
-        ENTT_ASSERT(elem == null, "Slot not available");
-        return elem;
+        return sparse[page][fast_mod(pos, traits_type::page_size)];
     }
 
     void release_sparse_pages() {
@@ -247,7 +245,7 @@ protected:
      * @param len The length to use.
      */
     void swap_only_length_temporary_function(const std::size_t len) {
-        ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatched");
+        ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatch");
         head = static_cast<underlying_type>(len);
     }
 
@@ -256,18 +254,10 @@ protected:
      * @param it An iterator to the element to pop.
      */
     void swap_only(const basic_iterator it) {
-        ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatched");
-
-        if(const auto pos = static_cast<underlying_type>(index(*it)); pos < head) {
-            bump(traits_type::next(*it));
-
-            if(const auto next = head - 1u; pos != next) {
-                swap_elements(packed[pos], packed[next]);
-            }
-
-            // partition check support in derived classes
-            --head;
-        }
+        ENTT_ASSERT(mode == deletion_policy::swap_only, "Deletion policy mismatch");
+        const auto pos = static_cast<underlying_type>(index(*it));
+        bump(traits_type::next(*it));
+        swap_at(pos, static_cast<size_type>(head -= (pos < head)));
     }
 
     /**
@@ -275,7 +265,7 @@ protected:
      * @param it An iterator to the element to pop.
      */
     void swap_and_pop(const basic_iterator it) {
-        ENTT_ASSERT(mode == deletion_policy::swap_and_pop, "Deletion policy mismatched");
+        ENTT_ASSERT(mode == deletion_policy::swap_and_pop, "Deletion policy mismatch");
         auto &self = sparse_ref(*it);
         const auto entt = traits_type::to_entity(self);
         sparse_ref(packed.back()) = traits_type::combine(entt, traits_type::to_integral(packed.back()));
@@ -292,7 +282,7 @@ protected:
      * @param it An iterator to the element to pop.
      */
     void in_place_pop(const basic_iterator it) {
-        ENTT_ASSERT(mode == deletion_policy::in_place, "Deletion policy mismatched");
+        ENTT_ASSERT(mode == deletion_policy::in_place, "Deletion policy mismatch");
         const auto entt = traits_type::to_entity(std::exchange(sparse_ref(*it), null));
         packed[static_cast<size_type>(entt)] = traits_type::combine(std::exchange(head, entt), tombstone);
     }
@@ -355,23 +345,38 @@ protected:
      * @return Iterator pointing to the emplaced element.
      */
     virtual basic_iterator try_emplace(const Entity entt, const bool force_back, const void * = nullptr) {
-        ENTT_ASSERT(!contains(entt), "Set already contains entity");
+        auto &elem = assure_at_least(entt);
+        auto pos = size();
 
-        switch(auto &elem = assure_at_least(entt); mode) {
+        switch(mode) {
         case deletion_policy::in_place:
             if(head != null && !force_back) {
-                const auto pos = static_cast<size_type>(head);
+                pos = static_cast<size_type>(head);
+                ENTT_ASSERT(elem == null, "Slot not available");
                 elem = traits_type::combine(head, traits_type::to_integral(entt));
                 head = traits_type::to_entity(std::exchange(packed[pos], entt));
-                return --(end() - pos);
+                break;
             }
             [[fallthrough]];
-        case deletion_policy::swap_only:
         case deletion_policy::swap_and_pop:
             packed.push_back(entt);
+            ENTT_ASSERT(elem == null, "Slot not available");
             elem = traits_type::combine(static_cast<typename traits_type::entity_type>(packed.size() - 1u), traits_type::to_integral(entt));
-            return begin();
+            break;
+        case deletion_policy::swap_only:
+            if(elem == null) {
+                packed.push_back(entt);
+                elem = traits_type::combine(static_cast<typename traits_type::entity_type>(packed.size() - 1u), traits_type::to_integral(entt));
+            } else {
+                ENTT_ASSERT(!(traits_type::to_entity(elem) < head), "Slot not available");
+                bump(entt);
+            }
+
+            swap_at(static_cast<size_type>(traits_type::to_entity(elem)), (pos = static_cast<size_type>(head++)));
+            break;
         }
+
+        return --(end() - pos);
     }
 
 public:

src/entt/entity/storage.hpp

@@ -1073,8 +1073,8 @@ public:
      */
     entity_type emplace() {
         const auto len = base_type::free_list();
-        base_type::swap_only_length_temporary_function(len + 1u);
-        return (len == base_type::size()) ? *base_type::try_emplace(entity_at(len), true) : base_type::operator[](len);
+        const auto entt = (len == base_type::size()) ? entity_at(len) : base_type::at(len);
+        return *base_type::try_emplace(entt, true);
     }
 
     /**
@@ -1093,21 +1093,19 @@ public:
             const auto pos = static_cast<size_type>(local_traits_type::to_entity(hint));
 
             while(!(pos < base_type::size())) {
-                base_type::try_emplace(entity_at(base_type::size()), true);
+                const auto it = base_type::try_emplace(entity_at(base_type::size()), true);
+                const auto entt = *it;
+
+                base_type::swap_only(it);
+                base_type::bump(entt);
             }
 
-            base_type::swap_elements(base_type::data()[pos], base_type::data()[base_type::free_list()]);
-            base_type::swap_only_length_temporary_function(base_type::free_list() + 1u);
+            return *base_type::try_emplace(hint, true);
         } else if(const auto idx = base_type::index(curr); idx < base_type::free_list()) {
             return emplace();
         } else {
-            base_type::swap_elements(base_type::data()[idx], base_type::data()[base_type::free_list()]);
-            base_type::swap_only_length_temporary_function(base_type::free_list() + 1u);
+            return *base_type::try_emplace(hint, true);
         }
-
-        base_type::bump(hint);
-
-        return hint;
     }
 
     /**
@@ -1130,13 +1128,12 @@ public:
      */
     template<typename It>
     void insert(It first, It last) {
-        for(const auto sz = base_type::size(); first != last && base_type::free_list() != sz; ++first, base_type::swap_only_length_temporary_function(base_type::free_list() + 1u)) {
-            *first = base_type::operator[](base_type::free_list());
+        for(const auto sz = base_type::size(); first != last && base_type::free_list() != sz; ++first) {
+            *first = *base_type::try_emplace(base_type::at(base_type::free_list()), true);
         }
 
         for(; first != last; ++first) {
             *first = *base_type::try_emplace(entity_at(base_type::free_list()), true);
-            base_type::swap_only_length_temporary_function(base_type::free_list() + 1u);
         }
     }
 

test/entt/entity/registry.cpp

@@ -555,8 +555,8 @@ TEST(Registry, CreateWithHint) {
     auto e3 = registry.create(entt::entity{3});
     auto e2 = registry.create(entt::entity{3});
 
-    ASSERT_EQ(e2, entt::entity{1});
-    ASSERT_FALSE(registry.valid(entt::entity{0}));
+    ASSERT_EQ(e2, entt::entity{0});
+    ASSERT_FALSE(registry.valid(entt::entity{1}));
     ASSERT_FALSE(registry.valid(entt::entity{2}));
     ASSERT_EQ(e3, entt::entity{3});
 
@@ -568,7 +568,7 @@ TEST(Registry, CreateWithHint) {
     e2 = registry.create();
     auto e1 = registry.create(entt::entity{2});
 
-    ASSERT_EQ(traits_type::to_entity(e2), 1u);
+    ASSERT_EQ(traits_type::to_entity(e2), 0u);
     ASSERT_EQ(traits_type::to_version(e2), 1u);
 
     ASSERT_EQ(traits_type::to_entity(e1), 2u);

test/entt/entity/sparse_set.cpp

@@ -559,6 +559,7 @@ TEST(SparseSet, StableErase) {
     set.erase(entity[1u]);
 
     ASSERT_EQ(set.size(), 3u);
+    ASSERT_EQ(set.free_list(), traits_type::entity_mask);
     ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entt::tombstone));
     ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entt::tombstone));
     ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entt::tombstone));
@@ -573,6 +574,7 @@ TEST(SparseSet, StableErase) {
     set.push(entt::entity{0});
 
     ASSERT_EQ(set.size(), 4u);
+    ASSERT_EQ(set.free_list(), traits_type::entity_mask);
     ASSERT_EQ(*set.begin(), entt::entity{0});
     ASSERT_EQ(set.at(0u), entity[1u]);
     ASSERT_EQ(set.at(1u), entity[0u]);
@@ -621,18 +623,51 @@ TEST(SparseSet, SwapOnlyErase) {
     set.erase(set.begin(), set.end());
 
     ASSERT_FALSE(set.empty());
-    ASSERT_EQ(set.size(), 3u);
     ASSERT_EQ(set.free_list(), 0u);
 
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+    entity[2u] = traits_type::next(entity[2u]);
+
+    ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u]));
+    ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u]));
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+
+    set.push(std::begin(entity), std::end(entity));
+    set.erase(entity, entity + 2u);
+
+    ASSERT_FALSE(set.empty());
+    ASSERT_EQ(set.free_list(), 1u);
+
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+
+    ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u]));
+    ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u]));
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+    ASSERT_EQ(*set.begin(), entity[0u]);
+
     set.erase(entity[2u]);
 
     ASSERT_FALSE(set.empty());
-    ASSERT_EQ(set.size(), 3u);
     ASSERT_EQ(set.free_list(), 0u);
 
-    ASSERT_EQ(set.at(0u), entity[0u]);
-    ASSERT_EQ(set.at(1u), entity[1u]);
-    ASSERT_EQ(set.at(2u), entity[2u]);
+    entity[2u] = traits_type::next(entity[2u]);
+
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+
+    set.push(std::begin(entity), std::end(entity));
+    std::swap(entity[1u], entity[2u]);
+    set.erase(entity, entity + 2u);
+
+    ASSERT_FALSE(set.empty());
+    ASSERT_EQ(set.free_list(), 1);
+
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+    ASSERT_EQ(*set.begin(), entity[0u]);
 }
 
 ENTT_DEBUG_TEST(SparseSetDeathTest, SwapOnlyErase) {
@@ -646,6 +681,8 @@ ENTT_DEBUG_TEST(SparseSetDeathTest, SwapOnlyErase) {
 }
 
 TEST(SparseSet, CrossSwapOnlyErase) {
+    using traits_type = entt::entt_traits<entt::entity>;
+
     entt::sparse_set set{entt::deletion_policy::swap_only};
     entt::sparse_set other{entt::deletion_policy::swap_only};
     entt::entity entity[2u]{entt::entity{3}, entt::entity{42}};
@@ -653,6 +690,7 @@ TEST(SparseSet, CrossSwapOnlyErase) {
     set.push(std::begin(entity), std::end(entity));
     other.push(entity[1u]);
     set.erase(other.begin(), other.end());
+    entity[1u] = traits_type::next(entity[1u]);
 
     ASSERT_TRUE(set.contains(entity[0u]));
     ASSERT_TRUE(set.contains(entity[1u]));
@@ -891,22 +929,73 @@ TEST(SparseSet, SwapOnlyRemove) {
 
     set.push(std::begin(entity), std::end(entity));
 
-    ASSERT_EQ(set.remove(set.begin(), set.end()), 3u);
     ASSERT_EQ(set.remove(set.begin(), set.end()), 3u);
     ASSERT_FALSE(set.empty());
-    ASSERT_EQ(set.size(), 3u);
+
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+    entity[2u] = traits_type::next(entity[2u]);
+
+    ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u]));
+    ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u]));
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+
+    set.push(std::begin(entity), std::end(entity));
+
+    ASSERT_EQ(set.remove(entity, entity + 2u), 2u);
+    ASSERT_FALSE(set.empty());
+
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+
+    ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u]));
+    ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u]));
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+    ASSERT_EQ(*set.begin(), entity[0u]);
 
     ASSERT_TRUE(set.remove(entity[2u]));
+    ASSERT_FALSE(set.remove(entity[2u]));
+
+    entity[2u] = traits_type::next(entity[2u]);
+
     ASSERT_TRUE(set.remove(entity[2u]));
+    ASSERT_FALSE(set.remove(entity[2u]));
     ASSERT_FALSE(set.empty());
-    ASSERT_EQ(set.size(), 3u);
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(traits_type::next(entity[2u])));
 
-    ASSERT_EQ(set.at(0u), entity[0u]);
-    ASSERT_EQ(set.at(1u), entity[1u]);
-    ASSERT_EQ(set.at(2u), entity[2u]);
+    set.push(entity, entity + 2u);
+
+    ASSERT_EQ(set.remove(std::begin(entity), std::end(entity)), 2u);
+
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+    entity[2u] = traits_type::next(entity[2u]);
+
+    ASSERT_EQ(set.current(entity[0u]), traits_type::to_version(entity[0u]));
+    ASSERT_EQ(set.current(entity[1u]), traits_type::to_version(entity[1u]));
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+    ASSERT_FALSE(set.empty());
+
+    set.push(std::begin(entity), std::end(entity));
+    std::swap(entity[1u], entity[2u]);
+
+    ASSERT_EQ(set.remove(entity, entity + 2u), 2u);
+
+    entity[0u] = traits_type::next(entity[0u]);
+    entity[1u] = traits_type::next(entity[1u]);
+
+    ASSERT_EQ(set.current(entity[2u]), traits_type::to_version(entity[2u]));
+    ASSERT_FALSE(set.empty());
+    ASSERT_EQ(*set.begin(), entity[0u]);
+
+    ASSERT_FALSE(set.remove(traits_type::construct(9, 0)));
+    ASSERT_FALSE(set.remove(entt::tombstone));
+    ASSERT_FALSE(set.remove(entt::null));
 }
 
 TEST(SparseSet, CrossSwapOnlyRemove) {
+    using traits_type = entt::entt_traits<entt::entity>;
+
     entt::sparse_set set{entt::deletion_policy::swap_only};
     entt::sparse_set other{entt::deletion_policy::swap_only};
     entt::entity entity[2u]{entt::entity{3}, entt::entity{42}};
@@ -914,6 +1003,7 @@ TEST(SparseSet, CrossSwapOnlyRemove) {
     set.push(std::begin(entity), std::end(entity));
     other.push(entity[1u]);
     set.remove(other.begin(), other.end());
+    entity[1u] = traits_type::next(entity[1u]);
 
     ASSERT_TRUE(set.contains(entity[0u]));
     ASSERT_TRUE(set.contains(entity[1u]));