Преглед изворни кода

storage: emplace/insert -> generate, for storage entity

Michele Caini пре 1 година
родитељ
комит
15c3ac27e9

+ 2 - 1
TODO

@@ -23,7 +23,7 @@ TODO:
   - get/reset placeholder to position after saving/loading (avoid long lookup)
   - allow skipping/reserving entity identifiers
   - documentation for reserved entities
-* storage entity: no emplace/insert, rename and add a fast range-push from above
+* storage entity: fast range-push from above
 * table: pop back to support swap and pop, single column access, empty type optimization
 * checkout tools workflow
 * entity based component_traits
@@ -38,3 +38,4 @@ TODO:
 * sparse_set shrink_to_fit argument for sparse array shrink policy (none, empty, deep, whatever)
 * any cdynamic to support const ownership construction
 * return meta context from meta objects
+* review sigh_mixin::insert, maybe bugged (ie with pointer stability enabled)

+ 45 - 38
src/entt/entity/mixin.hpp

@@ -294,48 +294,59 @@ public:
     }
 
     /**
-     * @brief Emplace elements into a storage.
-     *
-     * The behavior of this operation depends on the underlying storage type
-     * (for example, components vs entities).<br/>
-     * Refer to the specific documentation for more details.
-     *
-     * @return Whatever the underlying storage returns.
+     * @brief Creates a new identifier or recycles a destroyed one.
+     * @return A valid identifier.
      */
-    auto emplace() {
-        const auto entt = underlying_type::emplace();
+    auto generate() {
+        const auto entt = underlying_type::generate();
         construction.publish(owner_or_assert(), entt);
         return entt;
     }
 
     /**
-     * @brief Emplace elements into a storage.
-     *
-     * The behavior of this operation depends on the underlying storage type
-     * (for example, components vs entities).<br/>
-     * Refer to the specific documentation for more details.
-     *
+     * @brief Creates a new identifier or recycles a destroyed one.
+     * @param hint Required identifier.
+     * @return A valid identifier.
+     */
+    entity_type generate(const entity_type hint) {
+        const auto entt = underlying_type::generate(hint);
+        construction.publish(owner_or_assert(), entt);
+        return entt;
+    }
+
+    /**
+     * @brief Assigns each element in a range an identifier.
+     * @tparam It Type of mutable forward iterator.
+     * @param first An iterator to the first element of the range to generate.
+     * @param last An iterator past the last element of the range to generate.
+     */
+    template<typename It>
+    void generate(It first, It last) {
+        underlying_type::generate(first, last);
+
+        if(auto &reg = owner_or_assert(); !construction.empty()) {
+            for(; first != last; ++first) {
+                construction.publish(reg, *first);
+            }
+        }
+    }
+
+    /**
+     * @brief Assigns an entity to a storage and constructs its object.
      * @tparam Args Types of arguments to forward to the underlying storage.
-     * @param hint A valid identifier.
+     * @param entt A valid identifier.
      * @param args Parameters to forward to the underlying storage.
-     * @return Whatever the underlying storage returns.
+     * @return A reference to the newly created object.
      */
     template<typename... Args>
-    std::conditional_t<std::is_same_v<typename underlying_type::element_type, entity_type>, entity_type, decltype(std::declval<underlying_type>().get({}))>
-    emplace(const entity_type hint, Args &&...args) {
-        if constexpr(std::is_same_v<typename underlying_type::element_type, entity_type>) {
-            const auto entt = underlying_type::emplace(hint, std::forward<Args>(args)...);
-            construction.publish(owner_or_assert(), entt);
-            return entt;
-        } else {
-            underlying_type::emplace(hint, std::forward<Args>(args)...);
-            construction.publish(owner_or_assert(), hint);
-            return this->get(hint);
-        }
+    decltype(auto) emplace(const entity_type entt, Args &&...args) {
+        underlying_type::emplace(entt, std::forward<Args>(args)...);
+        construction.publish(owner_or_assert(), entt);
+        return this->get(entt);
     }
 
     /**
-     * @brief Patches the given instance for an entity.
+     * @brief Updates the instance assigned to a given entity in-place.
      * @tparam Func Types of the function objects to invoke.
      * @param entt A valid identifier.
      * @param func Valid function objects.
@@ -349,16 +360,12 @@ public:
     }
 
     /**
-     * @brief Emplace elements into a storage.
-     *
-     * The behavior of this operation depends on the underlying storage type
-     * (for example, components vs entities).<br/>
-     * Refer to the specific documentation for more details.
-     *
-     * @tparam It Iterator type (as required by the underlying storage type).
+     * @brief Assigns one or more entities to a storage and constructs their
+     * objects from a given instance.
+     * @tparam It Type of input iterator.
      * @tparam Args Types of arguments to forward to the underlying storage.
-     * @param first An iterator to the first element of the range.
-     * @param last An iterator past the last element of the range.
+     * @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 args Parameters to use to forward to the underlying storage.
      */
     template<typename It, typename... Args>

+ 3 - 3
src/entt/entity/registry.hpp

@@ -497,7 +497,7 @@ public:
      * @return A valid identifier.
      */
     [[nodiscard]] entity_type create() {
-        return entities.emplace();
+        return entities.generate();
     }
 
     /**
@@ -510,7 +510,7 @@ public:
      * @return A valid identifier.
      */
     [[nodiscard]] entity_type create(const entity_type hint) {
-        return entities.emplace(hint);
+        return entities.generate(hint);
     }
 
     /**
@@ -524,7 +524,7 @@ public:
      */
     template<typename It>
     void create(It first, It last) {
-        entities.insert(std::move(first), std::move(last));
+        entities.generate(std::move(first), std::move(last));
     }
 
     /**

+ 2 - 2
src/entt/entity/snapshot.hpp

@@ -237,7 +237,7 @@ public:
 
             for(entity_type entity = null; length; --length) {
                 archive(entity);
-                storage.emplace(entity);
+                storage.generate(entity);
             }
 
             storage.free_list(count);
@@ -247,7 +247,7 @@ public:
 
             while(length--) {
                 if(archive(entt); entt != null) {
-                    const auto entity = other.contains(entt) ? entt : other.emplace(entt);
+                    const auto entity = other.contains(entt) ? entt : other.generate(entt);
                     ENTT_ASSERT(entity == entt, "Entity not available for use");
 
                     if constexpr(std::tuple_size_v<decltype(storage.get_as_tuple({}))> == 0u) {

+ 44 - 12
src/entt/entity/storage.hpp

@@ -1007,7 +1007,7 @@ protected:
      * @return Iterator pointing to the emplaced element.
      */
     underlying_iterator try_emplace(const Entity hint, const bool, const void *) override {
-        return base_type::find(emplace(hint));
+        return base_type::find(generate(hint));
     }
 
 public:
@@ -1113,7 +1113,7 @@ public:
      * @brief Creates a new identifier or recycles a destroyed one.
      * @return A valid identifier.
      */
-    entity_type emplace() {
+    entity_type generate() {
         const auto len = base_type::free_list();
         const auto entt = (len == base_type::size()) ? next() : base_type::data()[len];
         return *base_type::try_emplace(entt, true);
@@ -1128,14 +1128,52 @@ public:
      * @param hint Required identifier.
      * @return A valid identifier.
      */
-    entity_type emplace(const entity_type hint) {
+    entity_type generate(const entity_type hint) {
         if(hint != null && hint != tombstone) {
             if(const auto curr = traits_type::construct(traits_type::to_entity(hint), base_type::current(hint)); curr == tombstone || !(base_type::index(curr) < base_type::free_list())) {
                 return *base_type::try_emplace(hint, true);
             }
         }
 
-        return emplace();
+        return generate();
+    }
+
+    /**
+     * @brief Assigns each element in a range an identifier.
+     * @tparam It Type of mutable forward iterator.
+     * @param first An iterator to the first element of the range to generate.
+     * @param last An iterator past the last element of the range to generate.
+     */
+    template<typename It>
+    void generate(It first, It last) {
+        for(const auto sz = base_type::size(); first != last && base_type::free_list() != sz; ++first) {
+            *first = *base_type::try_emplace(base_type::data()[base_type::free_list()], true);
+        }
+
+        for(; first != last; ++first) {
+            *first = *base_type::try_emplace(next(), true);
+        }
+    }
+
+    /**
+     * @brief Creates a new identifier or recycles a destroyed one.
+     * @return A valid identifier.
+     */
+    [[deprecated("use ::generate() instead")]] entity_type emplace() {
+        return generate();
+    }
+
+    /**
+     * @brief Creates a new identifier or recycles a destroyed one.
+     *
+     * If the requested identifier isn't in use, the suggested one is used.
+     * Otherwise, a new identifier is returned.
+     *
+     * @param hint Required identifier.
+     * @return A valid identifier.
+     */
+    [[deprecated("use ::generate(hint) instead")]] entity_type emplace(const entity_type hint) {
+        return generate(hint);
     }
 
     /**
@@ -1157,14 +1195,8 @@ public:
      * @param last An iterator past the last element of the range to generate.
      */
     template<typename It>
-    void insert(It first, It last) {
-        for(const auto sz = base_type::size(); first != last && base_type::free_list() != sz; ++first) {
-            *first = *base_type::try_emplace(base_type::data()[base_type::free_list()], true);
-        }
-
-        for(; first != last; ++first) {
-            *first = *base_type::try_emplace(next(), true);
-        }
+    [[deprecated("use ::generate(first, last) instead")]] void insert(It first, It last) {
+        generate(std::move(first), std::move(last));
     }
 
     /**

+ 3 - 3
test/entt/entity/sigh_mixin.cpp

@@ -305,11 +305,11 @@ TEST(SighMixin, StorageEntity) {
     ASSERT_EQ(on_construct, 3u);
     ASSERT_EQ(on_destroy, 3u);
 
-    pool.emplace();
-    pool.emplace(entt::entity{0});
+    pool.generate();
+    pool.generate(entt::entity{0});
 
     std::array<entt::entity, 1u> entity{};
-    pool.insert(entity.begin(), entity.end());
+    pool.generate(entity.begin(), entity.end());
 
     ASSERT_EQ(on_construct, 6u);
     ASSERT_EQ(on_destroy, 3u);

+ 40 - 40
test/entt/entity/storage_entity.cpp

@@ -32,7 +32,7 @@ TEST(StorageEntity, Move) {
     entt::storage<entt::entity> pool;
     const std::array entity{entt::entity{3}, entt::entity{2}};
 
-    pool.emplace(entity[0u]);
+    pool.generate(entity[0u]);
 
     static_assert(std::is_move_constructible_v<decltype(pool)>, "Move constructible type required");
     static_assert(std::is_move_assignable_v<decltype(pool)>, "Move assignable type required");
@@ -68,7 +68,7 @@ TEST(StorageEntity, Move) {
     ASSERT_EQ(pool.index(entity[0u]), 0u);
 
     other = entt::storage<entt::entity>{};
-    other.emplace(entity[1u]);
+    other.generate(entity[1u]);
     other = std::move(pool);
     test::is_initialized(pool);
 
@@ -86,10 +86,10 @@ TEST(StorageEntity, Swap) {
     ASSERT_EQ(pool.type(), entt::type_id<void>());
     ASSERT_EQ(other.type(), entt::type_id<void>());
 
-    pool.emplace(entt::entity{4});
+    pool.generate(entt::entity{4});
 
-    other.emplace(entt::entity{2});
-    other.emplace(entt::entity{1});
+    other.generate(entt::entity{2});
+    other.generate(entt::entity{1});
     other.erase(entt::entity{2});
 
     ASSERT_EQ(pool.size(), 1u);
@@ -111,7 +111,7 @@ TEST(StorageEntity, Getters) {
     entt::storage<entt::entity> pool;
     const entt::entity entity{4};
 
-    pool.emplace(entity);
+    pool.generate(entity);
 
     testing::StaticAssertTypeEq<decltype(pool.get({})), void>();
     testing::StaticAssertTypeEq<decltype(std::as_const(pool).get({})), void>();
@@ -137,18 +137,18 @@ ENTT_DEBUG_TEST(StorageEntityDeathTest, Getters) {
     ASSERT_DEATH([[maybe_unused]] const auto value = std::as_const(pool).get_as_tuple(entity), "");
 }
 
-TEST(StorageEntity, Emplace) {
+TEST(StorageEntity, Generate) {
     using traits_type = entt::entt_traits<entt::entity>;
 
     entt::storage<entt::entity> pool;
     std::array<entt::entity, 2u> entity{};
 
     ASSERT_EQ(pool.emplace(), entt::entity{0});
-    ASSERT_EQ(pool.emplace(entt::null), entt::entity{1});
-    ASSERT_EQ(pool.emplace(entt::tombstone), entt::entity{2});
+    ASSERT_EQ(pool.generate(entt::null), entt::entity{1});
+    ASSERT_EQ(pool.generate(entt::tombstone), entt::entity{2});
     ASSERT_EQ(pool.emplace(entt::entity{0}), entt::entity{3});
-    ASSERT_EQ(pool.emplace(traits_type::construct(1, 1)), entt::entity{4});
-    ASSERT_EQ(pool.emplace(traits_type::construct(6, 3)), traits_type::construct(6, 3));
+    ASSERT_EQ(pool.generate(traits_type::construct(1, 1)), entt::entity{4});
+    ASSERT_EQ(pool.generate(traits_type::construct(6, 3)), traits_type::construct(6, 3));
 
     ASSERT_LT(pool.index(entt::entity{0}), pool.free_list());
     ASSERT_LT(pool.index(entt::entity{1}), pool.free_list());
@@ -158,15 +158,15 @@ TEST(StorageEntity, Emplace) {
     ASSERT_EQ(pool.current(entt::entity{5}), traits_type::to_version(entt::tombstone));
     ASSERT_LT(pool.index(traits_type::construct(6, 3)), pool.free_list());
 
-    ASSERT_EQ(pool.emplace(traits_type::construct(5, 2)), traits_type::construct(5, 2));
-    ASSERT_EQ(pool.emplace(traits_type::construct(5, 3)), entt::entity{7});
+    ASSERT_EQ(pool.generate(traits_type::construct(5, 2)), traits_type::construct(5, 2));
+    ASSERT_EQ(pool.generate(traits_type::construct(5, 3)), entt::entity{7});
 
     pool.erase(entt::entity{2});
 
-    ASSERT_EQ(pool.emplace(), traits_type::construct(2, 1));
+    ASSERT_EQ(pool.generate(), traits_type::construct(2, 1));
 
     pool.erase(traits_type::construct(2, 1));
-    pool.insert(entity.begin(), entity.end());
+    pool.generate(entity.begin(), entity.end());
 
     ASSERT_EQ(entity[0u], traits_type::construct(2, 2));
     ASSERT_EQ(entity[1u], entt::entity{8});
@@ -177,15 +177,15 @@ TEST(StorageEntity, EmplaceInUse) {
     std::array<entt::entity, 2u> entity{};
     const entt::entity other{1};
 
-    ASSERT_EQ(pool.emplace(other), other);
-    ASSERT_EQ(pool.emplace(), entt::entity{0});
-    ASSERT_EQ(pool.emplace(), entt::entity{2});
+    ASSERT_EQ(pool.generate(other), other);
+    ASSERT_EQ(pool.generate(), entt::entity{0});
+    ASSERT_EQ(pool.generate(), entt::entity{2});
 
     pool.clear();
 
-    ASSERT_EQ(pool.emplace(other), other);
+    ASSERT_EQ(pool.generate(other), other);
 
-    pool.insert(entity.begin(), entity.end());
+    pool.generate(entity.begin(), entity.end());
 
     ASSERT_EQ(entity[0u], entt::entity{0});
     ASSERT_EQ(entity[1u], entt::entity{2});
@@ -255,7 +255,7 @@ TEST(StorageEntity, TryEmplaceInUse) {
 
 TEST(StorageEntity, Patch) {
     entt::storage<entt::entity> pool;
-    const auto entity = pool.emplace();
+    const auto entity = pool.generate();
 
     int counter = 0;
     auto callback = [&counter]() { ++counter; };
@@ -294,7 +294,7 @@ TEST(StorageEntity, Insert) {
     ASSERT_EQ(pool.size(), 2u);
     ASSERT_EQ(pool.free_list(), 0u);
 
-    pool.insert(entity.begin(), entity.begin() + 1u);
+    pool.generate(entity.begin(), entity.begin() + 1u);
 
     ASSERT_TRUE(pool.contains(entity[0u]));
     ASSERT_FALSE(pool.contains(entity[1u]));
@@ -333,7 +333,7 @@ TEST(StorageEntity, Pack) {
 TEST(StorageEntity, FreeList) {
     entt::storage<entt::entity> pool;
 
-    pool.emplace(entt::entity{0});
+    pool.generate(entt::entity{0});
 
     ASSERT_EQ(pool.size(), 1u);
     ASSERT_EQ(pool.free_list(), 1u);
@@ -352,7 +352,7 @@ TEST(StorageEntity, FreeList) {
 ENTT_DEBUG_TEST(StorageEntityDeathTest, FreeList) {
     entt::storage<entt::entity> pool;
 
-    pool.emplace(entt::entity{0});
+    pool.generate(entt::entity{0});
 
     ASSERT_DEATH(pool.free_list(2u), "");
 }
@@ -366,9 +366,9 @@ TEST(StorageEntity, Iterable) {
 
     entt::storage<entt::entity> pool;
 
-    pool.emplace(entt::entity{1});
-    pool.emplace(entt::entity{3});
-    pool.emplace(entt::entity{4});
+    pool.generate(entt::entity{1});
+    pool.generate(entt::entity{3});
+    pool.generate(entt::entity{4});
 
     pool.erase(entt::entity{3});
 
@@ -410,9 +410,9 @@ TEST(StorageEntity, ConstIterable) {
 
     entt::storage<entt::entity> pool;
 
-    pool.emplace(entt::entity{1});
-    pool.emplace(entt::entity{3});
-    pool.emplace(entt::entity{4});
+    pool.generate(entt::entity{1});
+    pool.generate(entt::entity{3});
+    pool.generate(entt::entity{4});
 
     pool.erase(entt::entity{3});
 
@@ -447,7 +447,7 @@ TEST(StorageEntity, ConstIterable) {
 
 TEST(StorageEntity, IterableIteratorConversion) {
     entt::storage<entt::entity> pool;
-    pool.emplace(entt::entity{3});
+    pool.generate(entt::entity{3});
 
     const typename entt::storage<entt::entity>::iterable::iterator it = pool.each().begin();
     typename entt::storage<entt::entity>::const_iterable::iterator cit = it;
@@ -461,7 +461,7 @@ TEST(StorageEntity, IterableIteratorConversion) {
 
 TEST(StorageEntity, IterableAlgorithmCompatibility) {
     entt::storage<entt::entity> pool;
-    pool.emplace(entt::entity{3});
+    pool.generate(entt::entity{3});
 
     const auto iterable = pool.each();
     const auto it = std::find_if(iterable.begin(), iterable.end(), [](auto args) { return std::get<0>(args) == entt::entity{3}; });
@@ -478,9 +478,9 @@ TEST(StorageEntity, ReverseIterable) {
 
     entt::storage<entt::entity> pool;
 
-    pool.emplace(entt::entity{1});
-    pool.emplace(entt::entity{3});
-    pool.emplace(entt::entity{4});
+    pool.generate(entt::entity{1});
+    pool.generate(entt::entity{3});
+    pool.generate(entt::entity{4});
 
     pool.erase(entt::entity{3});
 
@@ -522,9 +522,9 @@ TEST(StorageEntity, ReverseConstIterable) {
 
     entt::storage<entt::entity> pool;
 
-    pool.emplace(entt::entity{1});
-    pool.emplace(entt::entity{3});
-    pool.emplace(entt::entity{4});
+    pool.generate(entt::entity{1});
+    pool.generate(entt::entity{3});
+    pool.generate(entt::entity{4});
 
     pool.erase(entt::entity{3});
 
@@ -559,7 +559,7 @@ TEST(StorageEntity, ReverseConstIterable) {
 
 TEST(StorageEntity, ReverseIterableIteratorConversion) {
     entt::storage<entt::entity> pool;
-    pool.emplace(entt::entity{3});
+    pool.generate(entt::entity{3});
 
     const typename entt::storage<entt::entity>::reverse_iterable::iterator it = pool.reach().begin();
     typename entt::storage<entt::entity>::const_reverse_iterable::iterator cit = it;
@@ -573,7 +573,7 @@ TEST(StorageEntity, ReverseIterableIteratorConversion) {
 
 TEST(StorageEntity, ReverseIterableAlgorithmCompatibility) {
     entt::storage<entt::entity> pool;
-    pool.emplace(entt::entity{3});
+    pool.generate(entt::entity{3});
 
     const auto iterable = pool.reach();
     const auto it = std::find_if(iterable.begin(), iterable.end(), [](auto args) { return std::get<0>(args) == entt::entity{3}; });

+ 4 - 4
test/entt/entity/view.cpp

@@ -543,7 +543,7 @@ TEST(SingleStorageView, SwapStorage) {
 TEST(SingleStorageView, StorageEntity) {
     entt::storage<entt::entity> storage{};
     entt::basic_view<entt::get_t<entt::storage<entt::entity>>, entt::exclude_t<>> view{};
-    const std::array entity{storage.emplace(), storage.emplace()};
+    const std::array entity{storage.generate(), storage.generate()};
 
     ASSERT_EQ(view.front(), static_cast<entt::entity>(entt::null));
     ASSERT_EQ(view.back(), static_cast<entt::entity>(entt::null));
@@ -1426,7 +1426,7 @@ TEST(MultiStorageView, SwapStorage) {
 TEST(MultiStorageView, StorageEntity) {
     std::tuple<entt::storage<entt::entity>, entt::storage<char>> storage{};
     entt::basic_view view{std::get<0>(storage), std::get<1>(storage)};
-    const std::array entity{std::get<0>(storage).emplace(), std::get<0>(storage).emplace()};
+    const std::array entity{std::get<0>(storage).generate(), std::get<0>(storage).generate()};
 
     std::get<1>(storage).emplace(entity[0u]);
     std::get<1>(storage).emplace(entity[1u]);
@@ -1459,7 +1459,7 @@ TEST(MultiStorageView, StorageEntity) {
 TEST(MultiStorageView, StorageEntityWithExclude) {
     std::tuple<entt::storage<entt::entity>, entt::storage<int>, entt::storage<char>> storage{};
     entt::basic_view view{std::forward_as_tuple(std::get<0>(storage), std::get<1>(storage)), std::forward_as_tuple(std::get<2>(storage))};
-    const std::array entity{std::get<0>(storage).emplace(), std::get<0>(storage).emplace(), std::get<0>(storage).emplace()};
+    const std::array entity{std::get<0>(storage).generate(), std::get<0>(storage).generate(), std::get<0>(storage).generate()};
 
     std::get<1>(storage).emplace(entity[0u]);
     std::get<1>(storage).emplace(entity[1u]);
@@ -1496,7 +1496,7 @@ TEST(MultiStorageView, StorageEntityWithExclude) {
 TEST(MultiStorageView, StorageEntityExcludeOnly) {
     std::tuple<entt::storage<entt::entity>, entt::storage<int>> storage{};
     entt::basic_view view{std::forward_as_tuple(std::get<0>(storage)), std::forward_as_tuple(std::get<1>(storage))};
-    const std::array entity{std::get<0>(storage).emplace(), std::get<0>(storage).emplace(), std::get<0>(storage).emplace()};
+    const std::array entity{std::get<0>(storage).generate(), std::get<0>(storage).generate(), std::get<0>(storage).generate()};
 
     std::get<1>(storage).emplace(entity[2u]);