cleanup/refactoring

src/entt/entity/registry.hpp

@@ -70,20 +70,16 @@ class basic_registry {
 
         template<typename... Args>
         decltype(auto) assign(basic_registry &owner, const Entity entt, Args &&... args) {
-            if constexpr(ENTT_ENABLE_ETO(Component)) {
-                storage<Entity, Component>::construct(entt);
-                construction.publish(entt, owner, Component{});
-                return Component{std::forward<Args>(args)...};
-            } else {
-                auto &component = storage<Entity, Component>::construct(entt, std::forward<Args>(args)...);
-                construction.publish(entt, owner, component);
-                return component;
-            }
+            using component_type = std::conditional_t<ENTT_ENABLE_ETO(Component), Component, Component &>;
+            component_type component = storage<Entity, Component>::construct(entt, std::forward<Args>(args)...);
+            construction.publish(entt, owner, component);
+            return component;
         }
 
         template<typename It, typename... Args>
-        auto batch(basic_registry &owner, It first, It last, Args &&... args) {
-            auto it = storage<Entity, Component>::batch(first, last, std::forward<Args>(args)...);
+        std::enable_if_t<!std::is_same_v<It, Entity>, typename storage<Entity, Component>::reverse_iterator_type>
+        assign(basic_registry &owner, It first, It last, Args &&... args) {
+            auto it = storage<Entity, Component>::construct(first, last, std::forward<Args>(args)...);
 
             if(!construction.empty()) {
                 std::for_each(first, last, [this, &owner, it](const auto entt) mutable {
@@ -101,15 +97,9 @@ class basic_registry {
 
         template<typename... Args>
         decltype(auto) replace(basic_registry &owner, const Entity entt, Args &&... args) {
-            if constexpr(ENTT_ENABLE_ETO(Component)) {
-                ENTT_ASSERT((storage<Entity, Component>::has(entt)));
-                update.publish(entt, owner, Component{});
-                return Component{std::forward<Args>(args)...};
-            } else {
-                Component component{std::forward<Args>(args)...};
-                update.publish(entt, owner, component);
-                return (storage<Entity, Component>::get(entt) = std::move(component));
-            }
+            Component component{std::forward<Args>(args)...};
+            update.publish(entt, owner, component);
+            return (storage<Entity, Component>::get(entt) = std::move(component));
         }
 
     private:
@@ -592,8 +582,7 @@ public:
         std::generate(first, last, [this]() { return generate(); });
 
         if constexpr(sizeof...(Component) > 0) {
-            const auto distance = std::distance(first, last);
-            return std::make_tuple(std::make_reverse_iterator(assure<Component>()->batch(*this, first, last) + distance)...);
+            return std::make_tuple(assure<Component>()->assign(*this, first, last)...);
         }
     }
 
@@ -634,8 +623,8 @@ public:
      * existing types. The non-copyable ones will be ignored.
      *
      * @note
-     * Specifying the list of components is ways faster than an opaque copy and
-     * uses the batch creation under the hood.
+     * Specifying the list of components is faster than an opaque copy and uses
+     * the batch creation under the hood.
      *
      * @tparam Component Types of components to copy.
      * @tparam It Type of input iterator.
@@ -653,7 +642,7 @@ public:
             stomp<Component...>(first, last, src, other, exclude<Exclude...>);
         } else {
             static_assert(sizeof...(Exclude) == 0);
-            (assure<Component>()->batch(*this, first, last, other.get<Component>(src)), ...);
+            (assure<Component>()->assign(*this, first, last, other.get<Component>(src)), ...);
         }
     }
 
@@ -748,10 +737,10 @@ public:
      * @return An iterator to the list of components just created.
      */
     template<typename Component, typename It, typename... Args>
-    std::enable_if_t<!std::is_same_v<It, entity_type>, std::reverse_iterator<typename pool_type<Component>::iterator_type>>
+    std::enable_if_t<!std::is_same_v<It, entity_type>, typename pool_type<Component>::reverse_iterator_type>
     assign(It first, It last, Args &&... args) {
         ENTT_ASSERT(std::all_of(first, last, [this](const auto entity) { return valid(entity); }));
-        return std::make_reverse_iterator(assure<Component>()->batch(*this, first, last, std::forward<Args>(args)...) + std::distance(first, last));
+        return assure<Component>()->assign(*this, first, last, std::forward<Args>(args)...);
     }
 
     /**

src/entt/entity/sparse_set.hpp

@@ -418,7 +418,7 @@ public:
      * @param last An iterator past the last element of the range of entities.
      */
     template<typename It>
-    void batch(It first, It last) {
+    void construct(It first, It last) {
         std::for_each(first, last, [this, next = direct.size()](const auto entt) mutable {
             ENTT_ASSERT(!has(entt));
             auto [page, offset] = map(entt);

src/entt/entity/storage.hpp

@@ -165,6 +165,8 @@ public:
     using iterator_type = iterator<false>;
     /*! @brief Constant random access iterator type. */
     using const_iterator_type = iterator<true>;
+    /*! @brief Reverse iterator type. */
+    using reverse_iterator_type = std::reverse_iterator<iterator<false>>;
 
     /**
      * @brief Increases the capacity of a storage.
@@ -353,7 +355,8 @@ public:
      * same of the entities.
      */
     template<typename It, typename... Args>
-    iterator_type batch(It first, It last, [[maybe_unused]] Args &&... args) {
+    std::enable_if_t<!std::is_same_v<It, entity_type>, reverse_iterator_type>
+    construct(It first, It last, [[maybe_unused]] Args &&... args) {
         if constexpr(sizeof...(Args) == 0) {
             instances.resize(instances.size() + std::distance(first, last));
         } else {
@@ -361,8 +364,8 @@ public:
         }
 
         // entity goes after component in case constructor throws
-        underlying_type::batch(first, last);
-        return begin();
+        underlying_type::construct(first, last);
+        return std::make_reverse_iterator(begin() + std::distance(first, last));
     }
 
     /**
@@ -589,6 +592,8 @@ public:
     using size_type = std::size_t;
     /*! @brief Random access iterator type. */
     using iterator_type = iterator;
+    /*! @brief Reverse iterator type. */
+    using reverse_iterator_type = std::reverse_iterator<iterator>;
 
     /**
      * @brief Returns an iterator to the beginning.
@@ -656,10 +661,35 @@ public:
         return {};
     }
 
+    /**
+     * @brief Assigns an entity to a storage and constructs its object.
+     *
+     * The object type must be at least copy constructible.
+     *
+     * @warning
+     * Attempting to use an entity that already belongs to the storage results
+     * in undefined behavior.<br/>
+     * An assertion will abort the execution at runtime in debug mode if the
+     * storage already contains the given entity.
+     *
+     * @tparam Args Types of arguments to use to construct the object.
+     * @param entt A valid entity identifier.
+     * @param args Parameters to use to construct an object for the entity.
+     * @return The object associated with the entity.
+     */
+    template<typename... Args>
+    object_type construct(const entity_type entt, Args &&... args) {
+        object_type instance{std::forward<Args>(args)...};
+        // entity goes after component in case constructor throws
+        underlying_type::construct(entt);
+        return instance;
+    }
+
     /**
      * @brief Assigns one or more entities to a storage.
      *
-     * The object type must be at least default constructible.
+     * The object type must be at least default constructible if no arguments
+     * are provided.
      *
      * @warning
      * Attempting to assign an entity that already belongs to the storage
@@ -668,15 +698,20 @@ public:
      * storage already contains the given entity.
      *
      * @tparam It Type of forward iterator.
+     * @tparam Args Types of arguments to use to construct the object.
      * @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 construct an object for the entities.
      * @return An iterator to the list of instances just created and sorted the
      * same of the entities.
      */
-    template<typename It>
-    iterator_type batch(It first, It last, const object_type & = {}) {
-        underlying_type::batch(first, last);
-        return begin();
+    template<typename It, typename... Args>
+    std::enable_if_t<!std::is_same_v<It, entity_type>, reverse_iterator_type>
+    construct(It first, It last, Args &&... args) {
+        [[maybe_unused]] object_type instance{std::forward<Args>(args)...};
+        // entity goes after component in case constructor throws
+        underlying_type::construct(first, last);
+        return std::make_reverse_iterator(begin() + std::distance(first, last));
     }
 
     /*! @copydoc storage::sort */

test/entt/entity/sparse_set.cpp

@@ -124,7 +124,7 @@ TEST(SparseSet, BatchAdd) {
     entities[1] = entt::entity{42};
 
     set.construct(entt::entity{12});
-    set.batch(std::begin(entities), std::end(entities));
+    set.construct(std::begin(entities), std::end(entities));
     set.construct(entt::entity{24});
 
     ASSERT_TRUE(set.has(entities[0]));
@@ -414,7 +414,7 @@ TEST(SparseSet, SortRange) {
 TEST(SparseSet, ArrangOrdered) {
     entt::sparse_set<entt::entity> set;
     entt::entity entities[5]{entt::entity{42}, entt::entity{12}, entt::entity{9}, entt::entity{7}, entt::entity{3}};
-    set.batch(std::begin(entities), std::end(entities));
+    set.construct(std::begin(entities), std::end(entities));
 
     set.arrange(set.begin(), set.end(), [](auto...) { FAIL(); }, std::less{});
 
@@ -436,7 +436,7 @@ TEST(SparseSet, ArrangOrdered) {
 TEST(SparseSet, ArrangeReverse) {
     entt::sparse_set<entt::entity> set;
     entt::entity entities[5]{entt::entity{3}, entt::entity{7}, entt::entity{9}, entt::entity{12}, entt::entity{42}};
-    set.batch(std::begin(entities), std::end(entities));
+    set.construct(std::begin(entities), std::end(entities));
 
     set.arrange(set.begin(), set.end(), [&set, &entities](const auto lhs, const auto rhs) {
         std::swap(entities[set.index(lhs)], entities[set.index(rhs)]);
@@ -460,7 +460,7 @@ TEST(SparseSet, ArrangeReverse) {
 TEST(SparseSet, ArrangeUnordered) {
     entt::sparse_set<entt::entity> set;
     entt::entity entities[5]{entt::entity{9}, entt::entity{7}, entt::entity{3}, entt::entity{12}, entt::entity{42}};
-    set.batch(std::begin(entities), std::end(entities));
+    set.construct(std::begin(entities), std::end(entities));
 
     set.arrange(set.begin(), set.end(), [&set, &entities](const auto lhs, const auto rhs) {
         std::swap(entities[set.index(lhs)], entities[set.index(rhs)]);
@@ -484,7 +484,7 @@ TEST(SparseSet, ArrangeUnordered) {
 TEST(SparseSet, ArrangeRange) {
     entt::sparse_set<entt::entity> set;
     entt::entity entities[5]{entt::entity{9}, entt::entity{7}, entt::entity{3}, entt::entity{12}, entt::entity{42}};
-    set.batch(std::begin(entities), std::end(entities));
+    set.construct(std::begin(entities), std::end(entities));
 
     set.arrange(set.end(), set.end(), [](const auto, const auto) { FAIL(); }, std::less{});
 
@@ -530,7 +530,7 @@ TEST(SparseSet, ArrangeRange) {
 TEST(SparseSet, ArrangeCornerCase) {
     entt::sparse_set<entt::entity> set;
     entt::entity entities[5]{entt::entity{0}, entt::entity{1}, entt::entity{4}, entt::entity{3}, entt::entity{2}};
-    set.batch(std::begin(entities), std::end(entities));
+    set.construct(std::begin(entities), std::end(entities));
 
     set.arrange(++set.begin(), set.end(), [&set, &entities](const auto lhs, const auto rhs) {
         std::swap(entities[set.index(lhs)], entities[set.index(rhs)]);

test/entt/entity/storage.cpp

@@ -100,7 +100,7 @@ TEST(Storage, BatchAdd) {
 
     entities[0] = entt::entity{3};
     entities[1] = entt::entity{42};
-    auto it = pool.batch(std::begin(entities), std::end(entities));
+    auto it = pool.construct(std::begin(entities), std::end(entities));
 
     ASSERT_TRUE(pool.has(entities[0]));
     ASSERT_TRUE(pool.has(entities[1]));
@@ -113,8 +113,8 @@ TEST(Storage, BatchAdd) {
     it[0] = 1;
     it[1] = 2;
 
-    ASSERT_EQ(pool.get(entities[0]), 2);
-    ASSERT_EQ(pool.get(entities[1]), 1);
+    ASSERT_EQ(pool.get(entities[0]), 1);
+    ASSERT_EQ(pool.get(entities[1]), 2);
 }
 
 TEST(Storage, BatchAddByCopy) {
@@ -123,7 +123,7 @@ TEST(Storage, BatchAddByCopy) {
 
     entities[0] = entt::entity{3};
     entities[1] = entt::entity{42};
-    auto it = pool.batch(std::begin(entities), std::end(entities), 3);
+    auto it = pool.construct(std::begin(entities), std::end(entities), 3);
 
     ASSERT_TRUE(pool.has(entities[0]));
     ASSERT_TRUE(pool.has(entities[1]));
@@ -136,8 +136,8 @@ TEST(Storage, BatchAddByCopy) {
     it[0] = 1;
     it[1] = 2;
 
-    ASSERT_EQ(pool.get(entities[0]), 2);
-    ASSERT_EQ(pool.get(entities[1]), 1);
+    ASSERT_EQ(pool.get(entities[0]), 1);
+    ASSERT_EQ(pool.get(entities[1]), 2);
 }
 
 TEST(Storage, BatchAddEmptyType) {
@@ -147,7 +147,7 @@ TEST(Storage, BatchAddEmptyType) {
     entities[0] = entt::entity{3};
     entities[1] = entt::entity{42};
 
-    pool.batch(std::begin(entities), std::end(entities));
+    pool.construct(std::begin(entities), std::end(entities));
 
     ASSERT_TRUE(pool.has(entities[0]));
     ASSERT_TRUE(pool.has(entities[1]));