review: sort/respect

TODO

@@ -27,5 +27,7 @@
 * built-in support for dual (or N-) buffering
 
 TODO
-* registry::sort and registry::respect also for types that are part of a group (untracked items only)
-* review group::sort, rely on storage::sort and storage::respect
+* registry::sort also for types that are part of a group (untracked items only)
+* add sort by type to the non-owning group
+* add test for range sort to storage
+* custom (decoupled) pools ==> double buffering, shared components, multi-model

src/entt/entity/group.hpp

@@ -94,22 +94,21 @@ class basic_group;
  *
  * @tparam Entity A valid entity type (see entt_traits for more details).
  * @tparam Exclude Types of components used to filter the group.
- * @tparam Get Types of components observed by the group.
+ * @tparam Get Type of component observed by the group.
+ * @tparam Other Other types of components observed by the group.
  */
-template<typename Entity, typename... Exclude, typename... Get>
-class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>> {
-    static_assert(sizeof...(Get) > 0);
-
+template<typename Entity, typename... Exclude, typename Get, typename... Other>
+class basic_group<Entity, exclude_t<Exclude...>, get_t<Get, Other...>> {
     /*! @brief A registry is allowed to create groups. */
     friend class basic_registry<Entity>;
 
     template<typename Component>
     using pool_type = std::conditional_t<std::is_const_v<Component>, const storage<Entity, std::remove_const_t<Component>>, storage<Entity, Component>>;
 
-    // we could use pool_type<Get> *..., but vs complains about it and refuses to compile for unknown reasons (likely a bug)
-    basic_group(sparse_set<Entity> *ref, storage<Entity, std::remove_const_t<Get>> *... get) ENTT_NOEXCEPT
+    // we could use pool_type<Type> *..., but vs complains about it and refuses to compile for unknown reasons (most likely a bug)
+    basic_group(sparse_set<Entity> *ref, storage<Entity, std::remove_const_t<Get>> *get, storage<Entity, std::remove_const_t<Other>> *... other) ENTT_NOEXCEPT
         : handler{ref},
-          pools{get...}
+          pools{get, other...}
     {}
 
     template<typename Func, typename... Weak>
@@ -341,8 +340,8 @@ public:
      * forms:
      *
      * @code{.cpp}
-     * void(const entity_type, Get &...);
-     * void(Get &...);
+     * void(const entity_type, Get &, Other &...);
+     * void(Get &, Other &...);
      * @endcode
      *
      * @note
@@ -355,7 +354,7 @@ public:
      */
     template<typename Func>
     void each(Func func) const {
-        traverse(std::move(func), type_list<Get...>{});
+        traverse(std::move(func), type_list<Get, Other...>{});
     }
 
     /**
@@ -380,8 +379,9 @@ public:
      */
     template<typename Func>
     void less(Func func) const {
-        using non_empty_get = type_list_cat_t<std::conditional_t<std::is_empty_v<Get>, type_list<>, type_list<Get>>...>;
-        traverse(std::move(func), non_empty_get{});
+        using get_type_list = std::conditional_t<std::is_empty_v<Get>, type_list<>, type_list<Get>>;
+        using other_type_list = type_list_cat_t<std::conditional_t<std::is_empty_v<Other>, type_list<>, type_list<Other>>...>;
+        traverse(std::move(func), type_list_cat_t<get_type_list, other_type_list>{});
     }
 
     /**
@@ -407,7 +407,7 @@ public:
 
 private:
     sparse_set<entity_type> *handler;
-    const std::tuple<pool_type<Get> *...> pools;
+    const std::tuple<pool_type<Get> *, pool_type<Other> *...> pools;
 };
 
 
@@ -456,12 +456,11 @@ private:
  * @tparam Entity A valid entity type (see entt_traits for more details).
  * @tparam Exclude Types of components used to filter the group.
  * @tparam Get Types of components observed by the group.
- * @tparam Owned Types of components owned by the group.
+ * @tparam Owned Type of component owned by the group.
+ * @tparam Other Other types of components owned by the group.
  */
-template<typename Entity, typename... Exclude, typename... Get, typename... Owned>
-class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>, Owned...> {
-    static_assert(sizeof...(Get) + sizeof...(Owned) > 0);
-
+template<typename Entity, typename... Exclude, typename... Get, typename Owned, typename... Other>
+class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>, Owned, Other...> {
     /*! @brief A registry is allowed to create groups. */
     friend class basic_registry<Entity>;
 
@@ -471,16 +470,16 @@ class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>, Owned...> {
     template<typename Component>
     using component_iterator_type = decltype(std::declval<pool_type<Component>>().begin());
 
-    // we could use pool_type<Type> *..., but vs complains about it and refuses to compile for unknown reasons (likely a bug)
-    basic_group(const typename basic_registry<Entity>::size_type *sz, storage<Entity, std::remove_const_t<Owned>> *... owned, storage<Entity, std::remove_const_t<Get>> *... get) ENTT_NOEXCEPT
+    // we could use pool_type<Type> *..., but vs complains about it and refuses to compile for unknown reasons (most likely a bug)
+    basic_group(const typename basic_registry<Entity>::size_type *sz, storage<Entity, std::remove_const_t<Owned>> *owned, storage<Entity, std::remove_const_t<Other>> *... other, storage<Entity, std::remove_const_t<Get>> *... get) ENTT_NOEXCEPT
         : length{sz},
-          pools{owned..., get...}
+          pools{owned, other..., get...}
     {}
 
     template<typename Func, typename... Strong, typename... Weak>
     void traverse(Func func, type_list<Strong...>, type_list<Weak...>) const {
         [[maybe_unused]] auto raw = std::make_tuple((std::get<pool_type<Strong> *>(pools)->end() - *length)...);
-        [[maybe_unused]] auto data = std::get<0>(pools)->sparse_set<entity_type>::end() - *length;
+        [[maybe_unused]] auto data = std::get<pool_type<Owned> *>(pools)->sparse_set<entity_type>::end() - *length;
 
         for(auto next = *length; next; --next) {
             if constexpr(std::is_invocable_v<Func, decltype(get<Strong>({}))..., decltype(get<Weak>({}))...>) {
@@ -599,7 +598,7 @@ public:
      * @return A pointer to the array of entities.
      */
     const entity_type * data() const ENTT_NOEXCEPT {
-        return std::get<0>(pools)->data();
+        return std::get<pool_type<Owned> *>(pools)->data();
     }
 
     /**
@@ -617,7 +616,7 @@ public:
      * @return An iterator to the first entity that has the given components.
      */
     iterator_type begin() const ENTT_NOEXCEPT {
-        return std::get<0>(pools)->sparse_set<entity_type>::end() - *length;
+        return std::get<pool_type<Owned> *>(pools)->sparse_set<entity_type>::end() - *length;
     }
 
     /**
@@ -636,7 +635,7 @@ public:
      * given components.
      */
     iterator_type end() const ENTT_NOEXCEPT {
-        return std::get<0>(pools)->sparse_set<entity_type>::end();
+        return std::get<pool_type<Owned> *>(pools)->sparse_set<entity_type>::end();
     }
 
     /**
@@ -646,7 +645,7 @@ public:
      * iterator otherwise.
      */
     iterator_type find(const entity_type entt) const ENTT_NOEXCEPT {
-        const auto it = std::get<0>(pools)->find(entt);
+        const auto it = std::get<pool_type<Owned> *>(pools)->find(entt);
         return it != end() && it >= begin() && *it == entt ? it : end();
     }
 
@@ -707,8 +706,8 @@ public:
      * forms:
      *
      * @code{.cpp}
-     * void(const entity_type, Owned &..., Get &...);
-     * void(Owned &..., Get &...);
+     * void(const entity_type, Owned &, Other &..., Get &...);
+     * void(Owned &, Other &..., Get &...);
      * @endcode
      *
      * @note
@@ -721,7 +720,7 @@ public:
      */
     template<typename Func>
     void each(Func func) const {
-        traverse(std::move(func), type_list<Owned...>{}, type_list<Get...>{});
+        traverse(std::move(func), type_list<Owned, Other...>{}, type_list<Get...>{});
     }
 
     /**
@@ -746,9 +745,10 @@ public:
      */
     template<typename Func>
     void less(Func func) const {
-        using non_empty_owned = type_list_cat_t<std::conditional_t<std::is_empty_v<Owned>, type_list<>, type_list<Owned>>...>;
-        using non_empty_get = type_list_cat_t<std::conditional_t<std::is_empty_v<Get>, type_list<>, type_list<Get>>...>;
-        traverse(std::move(func), non_empty_owned{}, non_empty_get{});
+        using owned_type_list = std::conditional_t<std::is_empty_v<Owned>, type_list<>, type_list<Owned>>;
+        using other_type_list = type_list_cat_t<std::conditional_t<std::is_empty_v<Other>, type_list<>, type_list<Other>>...>;
+        using get_type_list = type_list_cat_t<std::conditional_t<std::is_empty_v<Get>, type_list<>, type_list<Get>>...>;
+        traverse(std::move(func), type_list_cat_t<owned_type_list, other_type_list>{}, get_type_list{});
     }
 
     /**
@@ -798,36 +798,32 @@ public:
      */
     template<typename... Component, typename Compare, typename Sort = std_sort, typename... Args>
     void sort(Compare compare, Sort algo = Sort{}, Args &&... args) {
-        std::vector<size_type> copy(*length);
-        std::iota(copy.begin(), copy.end(), 0);
+        auto *cpool = std::get<pool_type<Owned> *>(pools);
 
         if constexpr(sizeof...(Component) == 0) {
-            algo(copy.rbegin(), copy.rend(), [compare = std::move(compare), data = data()](const auto lhs, const auto rhs) {
-                return compare(data[lhs], data[rhs]);
-            }, std::forward<Args>(args)...);
+            static_assert(std::is_invocable_v<Compare, const entity_type, const entity_type>);
+            cpool->sort(cpool->end()-*length, cpool->end(), std::move(compare), std::move(algo), std::forward<Args>(args)...);
         } else {
-            algo(copy.rbegin(), copy.rend(), [compare = std::move(compare), data = data(), this](const auto lhs, const auto rhs) {
+            cpool->sort(cpool->end()-*length, cpool->end(), [compare = std::move(compare), this](const entity_type lhs, const entity_type rhs) {
                 // useless this-> used to suppress a warning with clang
-                return compare(this->get<Component>(data[lhs])..., this->get<Component>(data[rhs])...);
-            }, std::forward<Args>(args)...);
+                return compare(this->get<Component>(lhs)..., this->get<Component>(rhs)...);
+            }, std::move(algo), std::forward<Args>(args)...);
         }
 
-        for(size_type pos{}, last = copy.size(); pos < last; ++pos) {
-            auto curr = pos;
-            auto next = copy[curr];
+        for(auto next = *length; next; --next) {
+            ([next = next-1, curr = cpool->data()[next-1]](auto *cpool) {
+                const auto pos = cpool->sparse_set<entity_type>::get(curr);
 
-            while(curr != next) {
-                (std::get<pool_type<Owned> *>(pools)->swap(copy[curr], copy[next]), ...);
-                copy[curr] = curr;
-                curr = next;
-                next = copy[curr];
-            }
+                if(pos != next) {
+                    cpool->swap(next, cpool->sparse_set<entity_type>::get(curr));
+                }
+            }(std::get<pool_type<Other> *>(pools)), ...);
         }
     }
 
 private:
     const typename basic_registry<Entity>::size_type *length;
-    const std::tuple<pool_type<Owned> *..., pool_type<Get> *...> pools;
+    const std::tuple<pool_type<Owned> *, pool_type<Other> *..., pool_type<Get> *...> pools;
 };
 
 

src/entt/entity/registry.hpp

@@ -1034,7 +1034,8 @@ public:
     template<typename Component, typename Compare, typename Sort = std_sort, typename... Args>
     void sort(Compare compare, Sort algo = Sort{}, Args &&... args) {
         ENTT_ASSERT(!owned<Component>());
-        assure<Component>()->sort(std::move(compare), std::move(algo), std::forward<Args>(args)...);
+        auto *cpool = assure<Component>();
+        cpool->sort(cpool->begin(), cpool->end(), std::move(compare), std::move(algo), std::forward<Args>(args)...);
     }
 
     /**

src/entt/entity/sparse_set.hpp

@@ -461,7 +461,7 @@ public:
      * @param lhs A valid position within the sparse set.
      * @param rhs A valid position within the sparse set.
      */
-    void swap(const size_type lhs, const size_type rhs) ENTT_NOEXCEPT {
+    virtual void swap(const size_type lhs, const size_type rhs) ENTT_NOEXCEPT {
         ENTT_ASSERT(lhs < direct.size());
         ENTT_ASSERT(rhs < direct.size());
         auto [src_page, src_offset] = index(direct[lhs]);
@@ -489,7 +489,7 @@ public:
      *
      * @param other The sparse sets that imposes the order of the entities.
      */
-    virtual void respect(const sparse_set &other) ENTT_NOEXCEPT {
+    void respect(const sparse_set &other) ENTT_NOEXCEPT {
         const auto to = other.end();
         auto from = other.begin();
 

src/entt/entity/storage.hpp

@@ -388,7 +388,7 @@ public:
      * @param lhs A valid position within the sparse set.
      * @param rhs A valid position within the sparse set.
      */
-    void swap(const size_type lhs, const size_type rhs) ENTT_NOEXCEPT {
+    void swap(const size_type lhs, const size_type rhs) ENTT_NOEXCEPT override {
         ENTT_ASSERT(lhs < instances.size());
         ENTT_ASSERT(rhs < instances.size());
         std::swap(instances[lhs], instances[rhs]);
@@ -396,11 +396,11 @@ public:
     }
 
     /**
-     * @brief Sort instances according to the given comparison function.
+     * @brief Sort elements according to the given comparison function.
      *
-     * Sort the elements so that iterating the storage with a couple of
-     * iterators returns them in the expected order. See `begin` and `end` for
-     * more details.
+     * Sort the elements so that iterating the range with a couple of iterators
+     * returns them in the expected order. See `begin` and `end` for more
+     * details.
      *
      * The comparison function object must return `true` if the first element
      * is _less_ than the second one, `false` otherwise. The signature of the
@@ -433,14 +433,18 @@ public:
      * @tparam Compare Type of comparison function object.
      * @tparam Sort Type of sort function object.
      * @tparam Args Types of arguments to forward to the sort function object.
+     * @param first An iterator to the first element of the range to sort.
+     * @param last An iterator past the last element of the range to sort.
      * @param compare A valid comparison function object.
      * @param algo A valid sort function object.
      * @param args Arguments to forward to the sort function object, if any.
      */
     template<typename Compare, typename Sort = std_sort, typename... Args>
-    void sort(Compare compare, Sort algo = Sort{}, Args &&... args) {
-        std::vector<size_type> copy(instances.size());
-        std::iota(copy.begin(), copy.end(), 0);
+    void sort(iterator_type first, iterator_type last, Compare compare, Sort algo = Sort{}, Args &&... args) {
+        ENTT_ASSERT(!(first > last));
+
+        std::vector<size_type> copy(last - first);
+        std::iota(copy.begin(), copy.end(), std::distance(last, end()));
 
         if constexpr(std::is_invocable_v<Compare, const object_type &, const object_type &>) {
             static_assert(!std::is_empty_v<object_type>);
@@ -450,11 +454,11 @@ public:
             }, std::forward<Args>(args)...);
         } else {
             algo(copy.rbegin(), copy.rend(), [compare = std::move(compare), data = underlying_type::data()](const auto lhs, const auto rhs) {
-                return compare(data[lhs], data[rhs]);
+                return compare(std::as_const(data[lhs]), std::as_const(data[rhs]));
             }, std::forward<Args>(args)...);
         }
 
-        for(size_type pos{}, last = copy.size(); pos < last; ++pos) {
+        for(size_type pos{}, length = copy.size(); pos < length; ++pos) {
             auto curr = pos;
             auto next = copy[curr];
 
@@ -467,50 +471,6 @@ public:
         }
     }
 
-    /**
-     * @brief Sort instances according to the order of the entities in another
-     * sparse set.
-     *
-     * Entities that are part of both the storage are ordered internally
-     * according to the order they have in `other`. All the other entities goes
-     * to the end of the list and there are no guarantees on their order.
-     * Instances are sorted according to the entities to which they belong.<br/>
-     * In other terms, this function can be used to impose the same order on two
-     * sets by using one of them as a master and the other one as a slave.
-     *
-     * Iterating the storage with a couple of iterators returns elements in the
-     * expected order after a call to `respect`. See `begin` and `end` for more
-     * details.
-     *
-     * @note
-     * Attempting to iterate elements using a raw pointer returned by a call to
-     * either `data` or `raw` gives no guarantees on the order, even though
-     * `respect` has been invoked.
-     *
-     * @param other The sparse sets that imposes the order of the entities.
-     */
-    void respect(const sparse_set<Entity> &other) ENTT_NOEXCEPT override {
-        const auto to = other.end();
-        auto from = other.begin();
-
-        size_type pos = underlying_type::size() - 1;
-        const auto *local = underlying_type::data();
-
-        while(pos && from != to) {
-            const auto curr = *from;
-
-            if(underlying_type::has(curr)) {
-                if(curr != *(local + pos)) {
-                    swap(pos, underlying_type::get(curr));
-                }
-
-                --pos;
-            }
-
-            ++from;
-        }
-    }
-
     /*! @brief Resets a storage. */
     void reset() override {
         underlying_type::reset();

test/entt/entity/group.cpp

@@ -594,7 +594,7 @@ TEST(OwningGroup, SortOrdered) {
     registry.assign<boxed_int>(entities[3], 1);
     registry.assign<boxed_int>(entities[4], 2);
 
-    group.sort([&group](const auto lhs, const auto rhs) {
+    group.sort([&group](const entt::entity lhs, const entt::entity rhs) {
         return group.get<boxed_int>(lhs).value < group.get<boxed_int>(rhs).value;
     });
 

test/entt/entity/storage.cpp

@@ -328,7 +328,7 @@ TEST(Storage, SortOrdered) {
     ASSERT_EQ(set.get(entt::entity{3}).value, 3);
     ASSERT_EQ(set.get(entt::entity{9}).value, 1);
 
-    set.sort([](auto lhs, auto rhs) {
+    set.sort(set.begin(), set.end(), [](auto lhs, auto rhs) {
         return lhs.value < rhs.value;
     });
 
@@ -364,7 +364,7 @@ TEST(Storage, SortReverse) {
     ASSERT_EQ(set.get(entt::entity{3}).value, 9);
     ASSERT_EQ(set.get(entt::entity{9}).value, 12);
 
-    set.sort([&set](entt::entity lhs, entt::entity rhs) {
+    set.sort(set.begin(), set.end(), [&set](entt::entity lhs, entt::entity rhs) {
         return set.get(lhs).value < set.get(rhs).value;
     });
 
@@ -400,7 +400,7 @@ TEST(Storage, SortUnordered) {
     ASSERT_EQ(set.get(entt::entity{3}).value, 9);
     ASSERT_EQ(set.get(entt::entity{9}).value, 12);
 
-    set.sort([](auto lhs, auto rhs) {
+    set.sort(set.begin(), set.end(), [](auto lhs, auto rhs) {
         return lhs.value < rhs.value;
     });