From d8ed4ca3549195ebe8590803c1612d6eb00579fe Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Wed, 10 May 2023 10:47:11 +0200 Subject: [PATCH] registry: refine how entity storage is used internally --- src/entt/entity/registry.hpp | 89 +++++++++++++++++------------------ test/entt/entity/handle.cpp | 48 +++++-------------- test/entt/entity/registry.cpp | 36 +++++++------- test/example/entity_copy.cpp | 8 ++-- 4 files changed, 78 insertions(+), 103 deletions(-) diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp index ceec0f662..21f625201 100644 --- a/src/entt/entity/registry.hpp +++ b/src/entt/entity/registry.hpp @@ -248,41 +248,51 @@ class basic_registry { using group_container_type = dense_map, identity, std::equal_to, typename alloc_traits::template rebind_alloc>>>; template - [[nodiscard]] auto &assure(const id_type id = type_hash::value()) { - static_assert(std::is_same_v>, "Non-decayed types not allowed"); - auto &cpool = pools[id]; + [[nodiscard]] auto &assure([[maybe_unused]] const id_type id = type_hash::value()) { + if constexpr(std::is_same_v) { + return entities; + } else { + static_assert(std::is_same_v>, "Non-decayed types not allowed"); + auto &cpool = pools[id]; - if(!cpool) { - using storage_type = storage_for_type; - using alloc_type = typename storage_type::allocator_type; + if(!cpool) { + using storage_type = storage_for_type; + using alloc_type = typename storage_type::allocator_type; - if constexpr(std::is_same_v && !std::is_constructible_v) { - // std::allocator has no cross constructors (waiting for C++20) - cpool = std::allocate_shared(get_allocator(), alloc_type{}); - } else { - cpool = std::allocate_shared(get_allocator(), get_allocator()); + if constexpr(std::is_same_v && !std::is_constructible_v) { + // std::allocator has no cross constructors (waiting for C++20) + cpool = std::allocate_shared(get_allocator(), alloc_type{}); + } else { + cpool = std::allocate_shared(get_allocator(), get_allocator()); + } + + cpool->bind(forward_as_any(*this)); } - cpool->bind(forward_as_any(*this)); + ENTT_ASSERT(cpool->type() == type_id(), "Unexpected type"); + return static_cast &>(*cpool); } - - ENTT_ASSERT(cpool->type() == type_id(), "Unexpected type"); - return static_cast &>(*cpool); } template - [[nodiscard]] const auto *assure(const id_type id = type_hash::value()) const { - static_assert(std::is_same_v>, "Non-decayed types not allowed"); + [[nodiscard]] const auto *assure([[maybe_unused]] const id_type id = type_hash::value()) const { + if constexpr(std::is_same_v) { + return &entities; + } else { + static_assert(std::is_same_v>, "Non-decayed types not allowed"); - if(const auto it = pools.find(id); it != pools.cend()) { - ENTT_ASSERT(it->second->type() == type_id(), "Unexpected type"); - return static_cast *>(it->second.get()); + if(const auto it = pools.find(id); it != pools.cend()) { + ENTT_ASSERT(it->second->type() == type_id(), "Unexpected type"); + return static_cast *>(it->second.get()); + } + + return static_cast *>(nullptr); } - - return static_cast *>(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::value()] = std::shared_ptr{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 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 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().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 entities; - storage_for_type *shortcut; }; } // namespace entt diff --git a/test/entt/entity/handle.cpp b/test/entt/entity/handle.cpp index 529065233..777be0222 100644 --- a/test/entt/entity/handle.cpp +++ b/test/entt/entity/handle.cpp @@ -171,17 +171,10 @@ TEST(BasicHandle, Component) { ASSERT_TRUE(registry.storage().empty()); ASSERT_EQ(0u, (handle.remove())); - auto it = handle.storage().begin(); - - ASSERT_NE(it, handle.storage().end()); - ASSERT_EQ(it->first, entt::type_id().hash()); - ASSERT_TRUE(it->second.contains(handle.entity())); - - ASSERT_NE(++it, handle.storage().end()); - ASSERT_EQ(it->first, entt::type_id().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().hash()); + ASSERT_TRUE(pool.contains(handle.entity())); + } ASSERT_TRUE((handle.any_of())); ASSERT_FALSE((handle.all_of())); @@ -260,37 +253,20 @@ TEST(BasicHandle, Storage) { static_assert(std::is_same_v>); static_assert(std::is_same_v>); - 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(); registry.emplace(entity); - 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++, handle.storage().begin()); - ASSERT_NE(it, handle.storage().end()); - ASSERT_EQ(++it, handle.storage().end()); + ASSERT_EQ(++handle.storage().begin(), handle.storage().end()); + ASSERT_EQ(++chandle.storage().begin(), chandle.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_EQ(it->second.type(), entt::type_id()); - ASSERT_EQ((++it)->second.type(), entt::type_id()); - - ASSERT_EQ(cit->second.type(), entt::type_id()); - ASSERT_EQ((++cit)->second.type(), entt::type_id()); + ASSERT_EQ(handle.storage().begin()->second.type(), entt::type_id()); + ASSERT_EQ(chandle.storage().begin()->second.type(), entt::type_id()); } TEST(BasicHandle, HandleStorageIterator) { diff --git a/test/entt/entity/registry.cpp b/test/entt/entity/registry.cpp index 51fb16dfa..7eea24af6 100644 --- a/test/entt/entity/registry.cpp +++ b/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"_hs); storage.emplace(entity); - auto it = ++registry.storage().begin(); - auto cit = std::as_const(registry).storage().begin(); + for(auto [id, pool]: registry.storage()) { + static_assert(std::is_same_v); + static_assert(std::is_same_v); - static_assert(std::is_same_vfirst), entt::id_type>); - static_assert(std::is_same_vsecond), entt::sparse_set &>); + ASSERT_TRUE(pool.contains(entity)); + ASSERT_EQ(std::addressof(storage), std::addressof(pool)); + ASSERT_EQ(id, "int"_hs); + } - static_assert(std::is_same_vfirst), entt::id_type>); - static_assert(std::is_same_vsecond), const entt::sparse_set &>); + for(auto &&curr: std::as_const(registry).storage()) { + static_assert(std::is_same_v); + static_assert(std::is_same_v); - ASSERT_TRUE(it->second.contains(entity)); - ASSERT_EQ(std::addressof(storage), std::addressof(it->second)); - ASSERT_EQ(it->first, "int"_hs); - - ASSERT_TRUE(cit->second.contains(entity)); - ASSERT_NE(std::addressof(storage), std::addressof(cit->second)); - ASSERT_EQ(cit->first, entt::type_id().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(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().hash()); + ASSERT_EQ(begin[0u].first, entt::type_id().hash()); ASSERT_TRUE(begin[0u].second.contains(entity)); }; @@ -2213,6 +2216,7 @@ TEST(Registry, RegistryStorageIterator) { TEST(Registry, RegistryStorageIteratorConversion) { entt::registry registry; + registry.storage(); auto proxy = registry.storage(); auto cproxy = std::as_const(registry).storage(); @@ -2223,8 +2227,8 @@ TEST(Registry, RegistryStorageIteratorConversion) { static_assert(std::is_same_v>); static_assert(std::is_same_v>); - ASSERT_EQ(it->first, entt::type_id().hash()); - ASSERT_EQ((*it).second.type(), entt::type_id()); + ASSERT_EQ(it->first, entt::type_id().hash()); + ASSERT_EQ((*it).second.type(), entt::type_id()); ASSERT_EQ(it->first, cit->first); ASSERT_EQ((*it).second.type(), (*cit).second.type()); diff --git a/test/example/entity_copy.cpp b/test/example/entity_copy.cpp index 55adc2d6a..2c678aaf3 100644 --- a/test/example/entity_copy.cpp +++ b/test/example/entity_copy.cpp @@ -61,9 +61,9 @@ TEST(EntityCopy, SameRegistry) { ASSERT_TRUE((registry.all_of(src))); ASSERT_FALSE((registry.any_of(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(entity))); ASSERT_FALSE((dst.template all_of(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) {