registry: refine how entity storage is used internally

src/entt/entity/registry.hpp

@@ -248,41 +248,51 @@ class basic_registry {
     using group_container_type = dense_map<id_type, std::shared_ptr<internal::group_descriptor>, identity, std::equal_to<id_type>, typename alloc_traits::template rebind_alloc<std::pair<const id_type, std::shared_ptr<internal::group_descriptor>>>>;
 
     template<typename Type>
-    [[nodiscard]] auto &assure(const id_type id = type_hash<Type>::value()) {
-        static_assert(std::is_same_v<Type, std::decay_t<Type>>, "Non-decayed types not allowed");
-        auto &cpool = pools[id];
-
-        if(!cpool) {
-            using storage_type = storage_for_type<Type>;
-            using alloc_type = typename storage_type::allocator_type;
-
-            if constexpr(std::is_same_v<Type, void> && !std::is_constructible_v<alloc_type, allocator_type>) {
-                // std::allocator<void> has no cross constructors (waiting for C++20)
-                cpool = std::allocate_shared<storage_type>(get_allocator(), alloc_type{});
-            } else {
-                cpool = std::allocate_shared<storage_type>(get_allocator(), get_allocator());
+    [[nodiscard]] auto &assure([[maybe_unused]] const id_type id = type_hash<Type>::value()) {
+        if constexpr(std::is_same_v<Type, entity_type>) {
+            return entities;
+        } else {
+            static_assert(std::is_same_v<Type, std::decay_t<Type>>, "Non-decayed types not allowed");
+            auto &cpool = pools[id];
+
+            if(!cpool) {
+                using storage_type = storage_for_type<Type>;
+                using alloc_type = typename storage_type::allocator_type;
+
+                if constexpr(std::is_same_v<Type, void> && !std::is_constructible_v<alloc_type, allocator_type>) {
+                    // std::allocator<void> has no cross constructors (waiting for C++20)
+                    cpool = std::allocate_shared<storage_type>(get_allocator(), alloc_type{});
+                } else {
+                    cpool = std::allocate_shared<storage_type>(get_allocator(), get_allocator());
+                }
+
+                cpool->bind(forward_as_any(*this));
             }
 
-            cpool->bind(forward_as_any(*this));
+            ENTT_ASSERT(cpool->type() == type_id<Type>(), "Unexpected type");
+            return static_cast<storage_for_type<Type> &>(*cpool);
         }
-
-        ENTT_ASSERT(cpool->type() == type_id<Type>(), "Unexpected type");
-        return static_cast<storage_for_type<Type> &>(*cpool);
     }
 
     template<typename Type>
-    [[nodiscard]] const auto *assure(const id_type id = type_hash<Type>::value()) const {
-        static_assert(std::is_same_v<Type, std::decay_t<Type>>, "Non-decayed types not allowed");
+    [[nodiscard]] const auto *assure([[maybe_unused]] const id_type id = type_hash<Type>::value()) const {
+        if constexpr(std::is_same_v<Type, entity_type>) {
+            return &entities;
+        } else {
+            static_assert(std::is_same_v<Type, std::decay_t<Type>>, "Non-decayed types not allowed");
 
-        if(const auto it = pools.find(id); it != pools.cend()) {
-            ENTT_ASSERT(it->second->type() == type_id<Type>(), "Unexpected type");
-            return static_cast<const storage_for_type<Type> *>(it->second.get());
-        }
+            if(const auto it = pools.find(id); it != pools.cend()) {
+                ENTT_ASSERT(it->second->type() == type_id<Type>(), "Unexpected type");
+                return static_cast<const storage_for_type<Type> *>(it->second.get());
+            }
 
-        return static_cast<const storage_for_type<Type> *>(nullptr);
+            return static_cast<const storage_for_type<Type> *>(nullptr);
+        }
     }
 
     void rebind() {
+        entities.bind(forward_as_any(*this));
+
         for(auto &&curr: pools) {
             curr.second->bind(forward_as_any(*this));
         }
@@ -331,10 +341,8 @@ public:
         : vars{allocator},
           pools{allocator},
           groups{allocator},
-          entities{allocator},
-          shortcut{&entities} {
+          entities{allocator} {
         pools.reserve(count);
-        pools[type_hash<entity_type>::value()] = std::shared_ptr<base_type>{shortcut, [](const void *) {}};
         rebind();
     }
 
@@ -346,8 +354,7 @@ public:
         : vars{std::move(other.vars)},
           pools{std::move(other.pools)},
           groups{std::move(other.groups)},
-          entities{std::move(other.entities)},
-          shortcut{std::move(other.shortcut)} {
+          entities{std::move(other.entities)} {
         rebind();
     }
 
@@ -361,7 +368,6 @@ public:
         pools = std::move(other.pools);
         groups = std::move(other.groups);
         entities = std::move(other.entities);
-        shortcut = std::move(other.shortcut);
 
         rebind();
 
@@ -379,7 +385,6 @@ public:
         swap(pools, other.pools);
         swap(groups, other.groups);
         swap(entities, other.entities);
-        swap(shortcut, other.shortcut);
 
         rebind();
         other.rebind();
@@ -659,13 +664,11 @@ public:
      * @return The version of the recycled entity.
      */
     version_type destroy(const entity_type entt) {
-        ENTT_ASSERT(!pools.empty() && (pools.begin()->second.get() == shortcut), "Misplaced entity pool");
-        ENTT_ASSERT(entities.contains(entt), "Invalid entity");
-
         for(size_type pos = pools.size(); pos; --pos) {
             pools.begin()[pos - 1u].second->remove(entt);
         }
 
+        entities.erase(entt);
         return entities.current(entt);
     }
 
@@ -682,13 +685,7 @@ public:
      * @return The version actually assigned to the entity.
      */
     version_type destroy(const entity_type entt, const version_type version) {
-        ENTT_ASSERT(!pools.empty() && (pools.begin()->second.get() == shortcut), "Misplaced entity pool");
-        ENTT_ASSERT(entities.contains(entt), "Invalid entity");
-
-        for(size_type pos = pools.size(); pos; --pos) {
-            pools.begin()[pos - 1u].second->remove(entt);
-        }
-
+        destroy(entt);
         const auto elem = traits_type::construct(traits_type::to_entity(entt), version);
         return entities.bump((elem == tombstone) ? traits_type::next(elem) : elem);
     }
@@ -704,13 +701,14 @@ public:
      */
     template<typename It>
     void destroy(It first, It last) {
-        ENTT_ASSERT(!pools.empty() && (pools.begin()->second.get() == shortcut), "Misplaced entity pool");
         const auto from = entities.each().cbegin().base();
         const auto to = from + entities.pack(first, last);
 
         for(size_type pos = pools.size(); pos; --pos) {
             pools.begin()[pos - 1u].second->remove(from, to);
         }
+
+        entities.erase(from, to);
     }
 
     /**
@@ -1072,13 +1070,11 @@ public:
     template<typename... Type>
     void clear() {
         if constexpr(sizeof...(Type) == 0u) {
-            ENTT_ASSERT(!pools.empty() && (pools.begin()->second.get() == shortcut), "Misplaced entity pool");
-
-            for(size_type pos = pools.size() - 1u; pos; --pos) {
-                pools.begin()[pos].second->clear();
+            for(size_type pos = pools.size(); pos; --pos) {
+                pools.begin()[pos - 1u].second->clear();
             }
 
-            auto iterable = entities.each();
+            const auto iterable = entities.each();
             entities.erase(iterable.begin().base(), iterable.end().base());
         } else {
             (assure<Type>().clear(), ...);
@@ -1112,7 +1108,7 @@ public:
      * @return True if the entity has no components assigned, false otherwise.
      */
     [[nodiscard]] bool orphan(const entity_type entt) const {
-        return std::none_of(++pools.cbegin(), pools.cend(), [entt](auto &&curr) { return curr.second->contains(entt); });
+        return std::none_of(pools.cbegin(), pools.cend(), [entt](auto &&curr) { return curr.second->contains(entt); });
     }
 
     /**
@@ -1357,7 +1353,6 @@ private:
     pool_container_type pools;
     group_container_type groups;
     storage_for_type<entity_type> entities;
-    storage_for_type<entity_type> *shortcut;
 };
 
 } // namespace entt

test/entt/entity/handle.cpp

@@ -171,17 +171,10 @@ TEST(BasicHandle, Component) {
     ASSERT_TRUE(registry.storage<double>().empty());
     ASSERT_EQ(0u, (handle.remove<char, double>()));
 
-    auto it = handle.storage().begin();
-
-    ASSERT_NE(it, handle.storage().end());
-    ASSERT_EQ(it->first, entt::type_id<entt::entity>().hash());
-    ASSERT_TRUE(it->second.contains(handle.entity()));
-
-    ASSERT_NE(++it, handle.storage().end());
-    ASSERT_EQ(it->first, entt::type_id<int>().hash());
-    ASSERT_TRUE(it->second.contains(handle.entity()));
-
-    ASSERT_EQ(++it, handle.storage().end());
+    for(auto [id, pool]: handle.storage()) {
+        ASSERT_EQ(id, entt::type_id<int>().hash());
+        ASSERT_TRUE(pool.contains(handle.entity()));
+    }
 
     ASSERT_TRUE((handle.any_of<int, char, double>()));
     ASSERT_FALSE((handle.all_of<int, char, double>()));
@@ -260,37 +253,20 @@ TEST(BasicHandle, Storage) {
     static_assert(std::is_same_v<decltype(*handle.storage().begin()), std::pair<entt::id_type, entt::sparse_set &>>);
     static_assert(std::is_same_v<decltype(*chandle.storage().begin()), std::pair<entt::id_type, const entt::sparse_set &>>);
 
-    auto it = handle.storage().begin();
-    auto cit = chandle.storage().begin();
-
-    ASSERT_NE(it, handle.storage().end());
-    ASSERT_EQ(++it, handle.storage().end());
-
-    ASSERT_NE(cit, chandle.storage().end());
-    ASSERT_EQ(++cit, chandle.storage().end());
+    ASSERT_EQ(handle.storage().begin(), handle.storage().end());
+    ASSERT_EQ(chandle.storage().begin(), chandle.storage().end());
 
     registry.storage<double>();
     registry.emplace<int>(entity);
 
-    it = handle.storage().begin();
-    cit = chandle.storage().begin();
-
-    ASSERT_EQ(it++, handle.storage().begin());
-    ASSERT_NE(it, handle.storage().end());
-    ASSERT_EQ(++it, handle.storage().end());
-
-    ASSERT_EQ(cit++, chandle.storage().begin());
-    ASSERT_NE(cit, chandle.storage().end());
-    ASSERT_EQ(++cit, chandle.storage().end());
-
-    it = handle.storage().begin();
-    cit = chandle.storage().begin();
+    ASSERT_NE(handle.storage().begin(), handle.storage().end());
+    ASSERT_NE(chandle.storage().begin(), chandle.storage().end());
 
-    ASSERT_EQ(it->second.type(), entt::type_id<entt::entity>());
-    ASSERT_EQ((++it)->second.type(), entt::type_id<int>());
+    ASSERT_EQ(++handle.storage().begin(), handle.storage().end());
+    ASSERT_EQ(++chandle.storage().begin(), chandle.storage().end());
 
-    ASSERT_EQ(cit->second.type(), entt::type_id<entt::entity>());
-    ASSERT_EQ((++cit)->second.type(), entt::type_id<int>());
+    ASSERT_EQ(handle.storage().begin()->second.type(), entt::type_id<int>());
+    ASSERT_EQ(chandle.storage().begin()->second.type(), entt::type_id<int>());
 }
 
 TEST(BasicHandle, HandleStorageIterator) {

test/entt/entity/registry.cpp

@@ -2136,30 +2136,33 @@ TEST(Registry, Storage) {
 
     entt::registry registry;
     const auto entity = registry.create();
+
     auto &storage = registry.storage<int>("int"_hs);
     storage.emplace(entity);
 
-    auto it = ++registry.storage().begin();
-    auto cit = std::as_const(registry).storage().begin();
-
-    static_assert(std::is_same_v<decltype(it->first), entt::id_type>);
-    static_assert(std::is_same_v<decltype(it->second), entt::sparse_set &>);
+    for(auto [id, pool]: registry.storage()) {
+        static_assert(std::is_same_v<decltype(pool), entt::sparse_set &>);
+        static_assert(std::is_same_v<decltype(id), entt::id_type>);
 
-    static_assert(std::is_same_v<decltype(cit->first), entt::id_type>);
-    static_assert(std::is_same_v<decltype(cit->second), const entt::sparse_set &>);
+        ASSERT_TRUE(pool.contains(entity));
+        ASSERT_EQ(std::addressof(storage), std::addressof(pool));
+        ASSERT_EQ(id, "int"_hs);
+    }
 
-    ASSERT_TRUE(it->second.contains(entity));
-    ASSERT_EQ(std::addressof(storage), std::addressof(it->second));
-    ASSERT_EQ(it->first, "int"_hs);
+    for(auto &&curr: std::as_const(registry).storage()) {
+        static_assert(std::is_same_v<decltype(curr.second), const entt::sparse_set &>);
+        static_assert(std::is_same_v<decltype(curr.first), entt::id_type>);
 
-    ASSERT_TRUE(cit->second.contains(entity));
-    ASSERT_NE(std::addressof(storage), std::addressof(cit->second));
-    ASSERT_EQ(cit->first, entt::type_id<entt::entity>().hash());
+        ASSERT_TRUE(curr.second.contains(entity));
+        ASSERT_EQ(std::addressof(storage), std::addressof(curr.second));
+        ASSERT_EQ(curr.first, "int"_hs);
+    }
 }
 
 TEST(Registry, RegistryStorageIterator) {
     entt::registry registry;
     const auto entity = registry.create();
+    registry.emplace<int>(entity);
 
     auto test = [entity](auto iterable) {
         auto end{iterable.begin()};
@@ -2198,7 +2201,7 @@ TEST(Registry, RegistryStorageIterator) {
         ASSERT_GT(end, begin);
         ASSERT_GE(end, iterable.end());
 
-        ASSERT_EQ(begin[0u].first, entt::type_id<entt::entity>().hash());
+        ASSERT_EQ(begin[0u].first, entt::type_id<int>().hash());
         ASSERT_TRUE(begin[0u].second.contains(entity));
     };
 
@@ -2213,6 +2216,7 @@ TEST(Registry, RegistryStorageIterator) {
 
 TEST(Registry, RegistryStorageIteratorConversion) {
     entt::registry registry;
+    registry.storage<int>();
 
     auto proxy = registry.storage();
     auto cproxy = std::as_const(registry).storage();
@@ -2223,8 +2227,8 @@ TEST(Registry, RegistryStorageIteratorConversion) {
     static_assert(std::is_same_v<decltype(*it), std::pair<entt::id_type, entt::sparse_set &>>);
     static_assert(std::is_same_v<decltype(*cit), std::pair<entt::id_type, const entt::sparse_set &>>);
 
-    ASSERT_EQ(it->first, entt::type_id<entt::entity>().hash());
-    ASSERT_EQ((*it).second.type(), entt::type_id<entt::entity>());
+    ASSERT_EQ(it->first, entt::type_id<int>().hash());
+    ASSERT_EQ((*it).second.type(), entt::type_id<int>());
     ASSERT_EQ(it->first, cit->first);
     ASSERT_EQ((*it).second.type(), (*cit).second.type());
 

test/example/entity_copy.cpp

@@ -61,9 +61,9 @@ TEST(EntityCopy, SameRegistry) {
     ASSERT_TRUE((registry.all_of<int, char>(src)));
     ASSERT_FALSE((registry.any_of<int, char>(dst)));
 
-    for(auto it = ++registry.storage().begin(), last = registry.storage().end(); it != last; ++it) {
+    for(auto [id, storage]: registry.storage()) {
         // discard the custom storage because why not, this is just an example after all
-        if(auto [id, storage] = *it; id != "custom"_hs && storage.contains(src)) {
+        if(id != "custom"_hs && storage.contains(src)) {
             storage.push(dst, storage.value(src));
         }
     }
@@ -97,8 +97,8 @@ TYPED_TEST(EntityCopy, CrossRegistry) {
     ASSERT_TRUE((src.all_of<int, char>(entity)));
     ASSERT_FALSE((dst.template all_of<int, char>(copy)));
 
-    for(auto it = ++src.storage().begin(), last = src.storage().end(); it != last; ++it) {
-        if(auto [id, storage] = *it; storage.contains(entity)) {
+    for(auto [id, storage]: src.storage()) {
+        if(storage.contains(entity)) {
             auto *other = dst.storage(id);
 
             if(!other) {