1
0
Эх сурвалжийг харах

storage: full support for non-movable types (close #905)

Michele Caini 3 жил өмнө
parent
commit
3d03b01439

+ 0 - 1
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)

+ 5 - 3
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<size_type>(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);
             }
         }
     }

+ 17 - 9
src/entt/entity/storage.hpp

@@ -232,14 +232,14 @@ template<typename... CLhs, typename... CRhs>
  */
 template<typename Type, typename Entity, typename Allocator, typename>
 class basic_storage: public basic_sparse_set<Entity, typename std::allocator_traits<Allocator>::template rebind_alloc<Entity>> {
-    static_assert(std::is_move_constructible_v<Type> && std::is_move_assignable_v<Type>, "The type must be at least move constructible/assignable");
-
     using alloc_traits = std::allocator_traits<Allocator>;
     static_assert(std::is_same_v<typename alloc_traits::value_type, Type>, "Invalid value type");
     using underlying_type = basic_sparse_set<Entity, typename alloc_traits::template rebind_alloc<Entity>>;
     using container_type = std::vector<typename alloc_traits::pointer, typename alloc_traits::template rebind_alloc<typename alloc_traits::pointer>>;
     using comp_traits = component_traits<Type>;
 
+    static constexpr bool is_pinned_type_v = !(std::is_move_constructible_v<Type> && std::is_move_assignable_v<Type>);
+
     [[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:

+ 27 - 0
test/entt/entity/storage.cpp

@@ -9,10 +9,15 @@
 #include <gtest/gtest.h>
 #include <entt/entity/component.hpp>
 #include <entt/entity/storage.hpp>
+#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<std::unique_ptr<int>> pool;
 }
 
+TEST(Storage, PinnedComponent) {
+    // the purpose is to ensure that non-movable components are always accepted
+    [[maybe_unused]] entt::storage<pinned_type> pool;
+}
+
+ENTT_DEBUG_TEST(StorageDeathTest, PinnedComponent) {
+    entt::storage<pinned_type> 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;