sparse_set/storage:

* refine the try_erase to make it accept an iterator and a count (easier to work with at the call site)
* added a test for the stable clear
* added a note for future improvements

TODO

@@ -4,9 +4,7 @@
 * add examples (and credits) from @alanjfs :)
 
 WIP:
-* investigate: meta as_type_t<T> for getters and the like
-* investigate: add the possibility of disabling entities without deleting components thanks to the new full check
-* fast-contains for sparse sets (low prio but nice-to-have)
+* reintroduce swap-and-pop vs in-place-pop difference for perf reasons
 * runtime events (dispatcher/emitter), runtime context variables...
 * registry: remove reference to basic_sparse_set<E>
 * dedicated entity storage, in-place O(1) release/destroy for non-orphaned entities, out-of-sync model
@@ -16,10 +14,8 @@ WIP:
 * customizable any_vtable, sfinae-friendly definition and op::custom for user-def
 * resource, forward the id to the loader from the cache and if constexpr the call to load, update doc and describe customization points
 * add user data to type_info
-* headless (sparse set only) view
 * write documentation for custom storages and views!!
 * make runtime views use opaque storage and therefore return also elements.
-* add exclude-only views to combine with packs
 * entity-aware observer, add observer functions aside observer class
 * deprecate non-owning groups in favor of owning views and view packs, introduce lazy owning views
 * snapshot: support for range-based archives

src/entt/entity/sparse_set.hpp

@@ -234,26 +234,26 @@ protected:
 
     /**
      * @brief Erases an entity from a sparse set.
-     * @param first An iterator to the first element of the range of entities.
-     * @param last An iterator past the last element of the range of entities.
+     * @param it Iterator to the first element to erase.
+     * @param count Number of elements to erase.
      */
-    virtual void try_erase(const basic_iterator first, const basic_iterator last) {
-        for(auto it = first; it != last; ++it) {
-            const auto pos = static_cast<size_type>(it.index());
-            auto &ref = sparse_ref(*it);
-
-            if(mode == deletion_policy::in_place) {
-                packed[pos] = std::exchange(free_list, entity_traits::combine(static_cast<typename entity_traits::entity_type>(pos), entity_traits::reserved));
-            } else {
-                packed[pos] = packed.back();
-                sparse_ref(packed.back()) = entity_traits::combine(static_cast<typename entity_traits::entity_type>(pos), entity_traits::to_integral(packed.back()));
+    virtual void try_erase(basic_iterator it, const std::size_t count) {
+        if(mode == deletion_policy::in_place) {
+            for(const auto last = it + count; it != last; ++it) {
+                packed[it.index()] = std::exchange(free_list, entity_traits::combine(static_cast<typename entity_traits::entity_type>(it.index()), entity_traits::reserved));
+                sparse_ref(*it) = null;
+            }
+        } else {
+            for(const auto last = it + count; it != last; ++it) {
+                auto &ref = sparse_ref(*it);
+                packed[it.index()] = packed.back();
+                sparse_ref(packed.back()) = entity_traits::combine(static_cast<typename entity_traits::entity_type>(it.index()), entity_traits::to_integral(packed.back()));
                 // unnecessary but it helps to detect nasty bugs
                 ENTT_ASSERT((packed.back() = tombstone, true), "");
                 packed.pop_back();
+                // lazy self-assignment guard
+                ref = null;
             }
-
-            // lazy self-assignment guard
-            ref = null;
         }
     }
 
@@ -696,8 +696,7 @@ public:
      * @param entt A valid identifier.
      */
     void erase(const entity_type entt) {
-        const auto it = --(end() - index(entt));
-        try_erase(it, it + 1u);
+        try_erase(--(end() - index(entt)), 1u);
     }
 
     /**
@@ -712,7 +711,7 @@ public:
     template<typename It>
     void erase(It first, It last) {
         if constexpr(std::is_same_v<It, basic_iterator>) {
-            try_erase(first, last);
+            try_erase(first, static_cast<size_type>(std::distance(first, last)));
         } else {
             for(; first != last; ++first) {
                 erase(*first);
@@ -909,7 +908,7 @@ public:
                 remove(entity);
             }
         } else {
-            try_erase(begin(), end());
+            try_erase(begin(), size());
         }
     }
 

src/entt/entity/storage.hpp

@@ -277,7 +277,7 @@ class basic_storage: public basic_sparse_set<Entity, typename std::allocator_tra
             alloc_traits::construct(packed.second(), to_address(elem), std::forward<Args>(args)...);
         }
         ENTT_CATCH {
-            base_type::try_erase(it, it + 1u);
+            base_type::try_erase(it, 1u);
             ENTT_THROW;
         }
 
@@ -326,16 +326,16 @@ private:
 protected:
     /**
      * @brief Erases an element from a storage.
-     * @param first An iterator to the first element of the range of entities.
-     * @param last An iterator past the last element of the range of entities.
+     * @param it Iterator to the first element to erase.
+     * @param count Number of elements to erase.
      */
-    void try_erase(const typename underlying_type::basic_iterator first, const typename underlying_type::basic_iterator last) override {
-        for(auto it = first; it != last; ++it) {
+    void try_erase(typename underlying_type::basic_iterator it, const std::size_t count) override {
+        for(const auto last = it + count; it != last; ++it) {
             const auto pos = static_cast<size_type>(it.index());
             auto &elem = element_at(comp_traits::in_place_delete ? pos : (base_type::size() - 1u));
             [[maybe_unused]] auto unused = std::exchange(element_at(pos), std::move(elem));
             std::destroy_at(std::addressof(elem));
-            base_type::try_erase(it, it + 1u);
+            base_type::try_erase(it, 1u);
         }
     }
 
@@ -895,14 +895,13 @@ public:
  */
 template<typename Type>
 class sigh_storage_mixin final: public Type {
-    void try_erase(const typename Type::basic_iterator first, const typename Type::basic_iterator last) override {
+    void try_erase(typename Type::basic_iterator it, const std::size_t count) override {
         ENTT_ASSERT(owner != nullptr, "Invalid pointer to registry");
 
-        for(auto it = first; it != last; ++it) {
+        for(const auto last = it + count; it != last; ++it) {
             const auto entt = *it;
             destruction.publish(*owner, entt);
-            const auto curr = Type::find(entt);
-            Type::try_erase(curr, curr + 1u);
+            Type::try_erase(Type::find(entt), 1u);
         }
     }
 

test/entt/entity/sparse_set.cpp

@@ -811,6 +811,26 @@ TEST(SparseSet, Clear) {
     set.clear();
 
     ASSERT_TRUE(set.empty());
+    ASSERT_EQ(set.size(), 0u);
+}
+
+TEST(SparseSet, StableClear) {
+    entt::sparse_set set{entt::deletion_policy::in_place};
+
+    set.emplace(entt::entity{3});
+    set.emplace(entt::entity{42});
+    set.emplace(entt::entity{9});
+
+    ASSERT_FALSE(set.empty());
+
+    set.clear();
+
+    ASSERT_FALSE(set.empty());
+    ASSERT_EQ(set.size(), 3u);
+
+    ASSERT_EQ(set.find(entt::entity{3}), set.end());
+    ASSERT_EQ(set.find(entt::entity{42}), set.end());
+    ASSERT_EQ(set.find(entt::entity{9}), set.end());
 }
 
 TEST(SparseSet, Iterator) {