From 3d03b0143980dd350d03d11d32d79a4fa7366706 Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Wed, 3 Aug 2022 10:57:51 +0200 Subject: [PATCH] storage: full support for non-movable types (close #905) --- TODO | 1 - src/entt/entity/sparse_set.hpp | 8 +++++--- src/entt/entity/storage.hpp | 26 +++++++++++++++++--------- test/entt/entity/storage.cpp | 27 +++++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 13 deletions(-) diff --git a/TODO b/TODO index 4e088fed6..94f344be9 100644 --- a/TODO +++ b/TODO @@ -14,7 +14,6 @@ DOC: WIP: * any: assert-if-dynamic like tag to the constructor * no gh-pages, use main/docs or similar, see settings/pages on gh -* sparse set/storage support for move-only types, internal rework required * get rid of observers, storage based views made them pointless - document alternatives * add storage getter for filters to views and groups * exploit the tombstone mechanism to allow enabling/disabling entities (see bump, compact and clear for further details) diff --git a/src/entt/entity/sparse_set.hpp b/src/entt/entity/sparse_set.hpp index a7ae3d58a..96aa172d1 100644 --- a/src/entt/entity/sparse_set.hpp +++ b/src/entt/entity/sparse_set.hpp @@ -238,6 +238,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"); auto &self = sparse_ref(*it); const auto entt = entity_traits::to_entity(self); sparse_ref(packed.back()) = entity_traits::combine(entt, entity_traits::to_integral(packed.back())); @@ -254,6 +255,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"); const auto entt = entity_traits::to_entity(std::exchange(sparse_ref(*it), null)); packed[static_cast(entt)] = std::exchange(free_list, entity_traits::combine(entt, entity_traits::reserved)); } @@ -265,13 +267,13 @@ protected: * @param last An iterator past the last element of the range of entities. */ virtual void pop(basic_iterator first, basic_iterator last) { - if(mode == deletion_policy::in_place) { + if(mode == deletion_policy::swap_and_pop) { for(; first != last; ++first) { - in_place_pop(first); + swap_and_pop(first); } } else { for(; first != last; ++first) { - swap_and_pop(first); + in_place_pop(first); } } } diff --git a/src/entt/entity/storage.hpp b/src/entt/entity/storage.hpp index 3ac1868a0..db6d25043 100644 --- a/src/entt/entity/storage.hpp +++ b/src/entt/entity/storage.hpp @@ -232,14 +232,14 @@ template */ template class basic_storage: public basic_sparse_set::template rebind_alloc> { - static_assert(std::is_move_constructible_v && std::is_move_assignable_v, "The type must be at least move constructible/assignable"); - using alloc_traits = std::allocator_traits; static_assert(std::is_same_v, "Invalid value type"); using underlying_type = basic_sparse_set>; using container_type = std::vector>; using comp_traits = component_traits; + static constexpr bool is_pinned_type_v = !(std::is_move_constructible_v && std::is_move_assignable_v); + [[nodiscard]] auto &element_at(const std::size_t pos) const { return packed.first()[pos / comp_traits::page_size][fast_mod(pos, comp_traits::page_size)]; } @@ -309,15 +309,23 @@ private: return std::addressof(element_at(pos)); } - void swap_at(const std::size_t lhs, const std::size_t rhs) final { - using std::swap; - swap(element_at(lhs), element_at(rhs)); + void swap_at([[maybe_unused]] const std::size_t lhs, [[maybe_unused]] const std::size_t rhs) final { + if constexpr(is_pinned_type_v) { + ENTT_ASSERT(false, "Pinned type"); + } else { + using std::swap; + swap(element_at(lhs), element_at(rhs)); + } } - void move_element(const std::size_t from, const std::size_t to) final { - auto &elem = element_at(from); - entt::uninitialized_construct_using_allocator(to_address(assure_at_least(to)), packed.second(), std::move(elem)); - std::destroy_at(std::addressof(elem)); + void move_element([[maybe_unused]] const std::size_t from, [[maybe_unused]] const std::size_t to) final { + if constexpr(is_pinned_type_v) { + ENTT_ASSERT(false, "Pinned type"); + } else { + auto &elem = element_at(from); + entt::uninitialized_construct_using_allocator(to_address(assure_at_least(to)), packed.second(), std::move(elem)); + std::destroy_at(std::addressof(elem)); + } } protected: diff --git a/test/entt/entity/storage.cpp b/test/entt/entity/storage.cpp index c74a185e0..27c0df39d 100644 --- a/test/entt/entity/storage.cpp +++ b/test/entt/entity/storage.cpp @@ -9,10 +9,15 @@ #include #include #include +#include "../common/config.h" #include "../common/throwing_allocator.hpp" #include "../common/throwing_type.hpp" #include "../common/tracked_memory_resource.hpp" +struct pinned_type { + const int value{42}; +}; + struct empty_stable_type { static constexpr auto in_place_delete = true; }; @@ -1635,6 +1640,28 @@ TEST(Storage, MoveOnlyComponent) { [[maybe_unused]] entt::storage> pool; } +TEST(Storage, PinnedComponent) { + // the purpose is to ensure that non-movable components are always accepted + [[maybe_unused]] entt::storage pool; +} + +ENTT_DEBUG_TEST(StorageDeathTest, PinnedComponent) { + entt::storage pool; + const entt::entity entity{0}; + const entt::entity destroy{1}; + const entt::entity other{2}; + + pool.emplace(entity); + pool.emplace(destroy); + pool.emplace(other); + + pool.erase(destroy); + + ASSERT_DEATH(pool.swap_elements(entity, other), ""); + ASSERT_DEATH(pool.compact(), ""); + ASSERT_DEATH(pool.sort([&pool](auto &&lhs, auto &&rhs) { return lhs < rhs; }), ""); +} + TEST(Storage, UpdateFromDestructor) { static constexpr auto size = 10u;