Просмотр исходного кода

view/group: consistent ::get without template arguments (breaking changes)

Michele Caini 5 лет назад
Родитель
Сommit
9983faef70
6 измененных файлов с 88 добавлено и 41 удалено
  1. 1 0
      TODO
  2. 14 13
      docs/md/entity.md
  3. 10 10
      src/entt/entity/group.hpp
  4. 20 11
      src/entt/entity/view.hpp
  5. 6 2
      test/entt/entity/group.cpp
  6. 37 5
      test/entt/entity/view.cpp

+ 1 - 0
TODO

@@ -10,6 +10,7 @@
 
 WIP:
 * HP: write documentation for custom storages and views!!
+* use extended get to remove is_eto_eligible checks from views and groups.
 * factory invoke: add support for external member functions as static meta functions
 * pagination doesn't work nicely across boundaries probably, give it a look. RO operations are fine, adding components maybe not.
 * make it easier to hook into the type system and describe how to do that to eg auto-generate meta types on first use

+ 14 - 13
docs/md/entity.md

@@ -1343,16 +1343,19 @@ auto view = registry.view<position, velocity>(entt::exclude<renderable>);
 To iterate a view, either use it in a range-for loop:
 
 ```cpp
-auto view = registry.view<position, velocity>();
+auto view = registry.view<position, velocity, renderable>();
 
 for(auto entity: view) {
     // a component at a time ...
     auto &position = view.get<position>(entity);
     auto &velocity = view.get<velocity>(entity);
 
-    // ... or multiple components at once
+    // ... multiple components ...
     auto [pos, vel] = view.get<position, velocity>(entity);
 
+    // ... all components at once
+    auto [pos, vel, rend] = view.get(entity);
+
     // ...
 }
 ```
@@ -1386,13 +1389,15 @@ recommend referring to the official documentation for more details and I won't
 further investigate the topic here.
 
 As a side note, in the case of single component views, `get` accepts but doesn't
-strictly require a template parameter, since the type is implicitly defined:
+strictly require a template parameter, since the type is implicitly defined.
+However, when the type isn't specified, for consistency with the multi component
+view, the instance will be returned using a tuple:
 
 ```cpp
 auto view = registry.view<const renderable>();
 
 for(auto entity: view) {
-    const auto &renderable = view.get(entity);
+    auto [renderable] = view.get(entity);
     // ...
 }
 ```
@@ -1424,13 +1429,6 @@ entt::id_type types[] = { entt::type_hash<position>::value(), entt::type_hash<ve
 auto view = registry.runtime_view(std::cbegin(types), std::cend(types));
 
 for(auto entity: view) {
-    // a component at a time ...
-    auto &position = registry.get<position>(entity);
-    auto &velocity = registry.get<velocity>(entity);
-
-    // ... or multiple components at once
-    auto [pos, vel] = registry.get<position, velocity>(entity);
-
     // ...
 }
 ```
@@ -1504,16 +1502,19 @@ iterators whenever `begin` or `end` are invoked.
 To iterate groups, either use them in a range-for loop:
 
 ```cpp
-auto group = registry.group<position>(entt::get<velocity>);
+auto group = registry.group<position>(entt::get<velocity, renderable>);
 
 for(auto entity: group) {
     // a component at a time ...
     auto &position = group.get<position>(entity);
     auto &velocity = group.get<velocity>(entity);
 
-    // ... or multiple components at once
+    // ... multiple components ...
     auto [pos, vel] = group.get<position, velocity>(entity);
 
+    // ... all components at once
+    auto [pos, vel, rend] = group.get(entity);
+
     // ...
 }
 ```

+ 10 - 10
src/entt/entity/group.hpp

@@ -135,7 +135,7 @@ class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>> final {
         [[nodiscard]] iterator begin() const ENTT_NOEXCEPT {
             return iterable_group_iterator{handler->begin(), std::tuple_cat([](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::make_tuple(cpool);
                 }
@@ -145,7 +145,7 @@ class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>> final {
         [[nodiscard]] iterator end() const ENTT_NOEXCEPT {
             return iterable_group_iterator{handler->end(), std::tuple_cat([](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::make_tuple(cpool);
                 }
@@ -425,7 +425,7 @@ public:
         if constexpr(sizeof...(Component) == 0) {
             return std::tuple_cat([entt](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::forward_as_tuple(cpool->get(entt));
                 }
@@ -433,7 +433,7 @@ public:
         } else if constexpr(sizeof...(Component) == 1) {
             return (std::get<pool_type<Component> *>(pools)->get(entt), ...);
         } else {
-            return std::tuple<decltype(get<Component>({}))...>{get<Component>(entt)...};
+            return std::forward_as_tuple(get<Component>(entt)...);
         }
     }
 
@@ -692,14 +692,14 @@ class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>, Owned...> final
                 std::get<0>(pools)->basic_sparse_set<Entity>::end() - *length,
                 std::tuple_cat([length = *length](auto *cpool) {
                     if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                        return std::make_tuple();
+                        return std::tuple{};
                     } else {
                         return std::make_tuple(cpool->end() - length);
                     }
                 }(std::get<pool_type<Owned> *>(pools))...),
                 std::tuple_cat([](auto *cpool) {
                     if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                        return std::make_tuple();
+                        return std::tuple{};
                     } else {
                         return std::make_tuple(cpool);
                     }
@@ -712,14 +712,14 @@ class basic_group<Entity, exclude_t<Exclude...>, get_t<Get...>, Owned...> final
                 std::get<0>(pools)->basic_sparse_set<Entity>::end(),
                 std::tuple_cat([](auto *cpool) {
                     if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                        return std::make_tuple();
+                        return std::tuple{};
                     } else {
                         return std::make_tuple(cpool->end());
                     }
                 }(std::get<pool_type<Owned> *>(pools))...),
                 std::tuple_cat([](auto *cpool) {
                     if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                        return std::make_tuple();
+                        return std::tuple{};
                     } else {
                         return std::make_tuple(cpool);
                     }
@@ -1001,7 +1001,7 @@ public:
         if constexpr(sizeof...(Component) == 0) {
             auto filter = [entt](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::forward_as_tuple(cpool->get(entt));
                 }
@@ -1011,7 +1011,7 @@ public:
         } else if constexpr(sizeof...(Component) == 1) {
             return (std::get<pool_type<Component> *>(pools)->get(entt), ...);
         } else {
-            return std::tuple<decltype(get<Component>({}))...>{get<Component>(entt)...};
+            return std::forward_as_tuple(get<Component>(entt)...);
         }
     }
 

+ 20 - 11
src/entt/entity/view.hpp

@@ -215,7 +215,7 @@ class basic_view<Entity, exclude_t<Exclude...>, Component...> final {
         [[nodiscard]] iterator begin() const ENTT_NOEXCEPT {
             return iterable_view_iterator{first, std::tuple_cat([](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::make_tuple(cpool);
                 }
@@ -225,7 +225,7 @@ class basic_view<Entity, exclude_t<Exclude...>, Component...> final {
         [[nodiscard]] iterator end() const ENTT_NOEXCEPT {
             return iterable_view_iterator{last, std::tuple_cat([](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::make_tuple(cpool);
                 }
@@ -476,7 +476,7 @@ public:
         if constexpr(sizeof...(Comp) == 0) {
             return std::tuple_cat([entt](auto *cpool) {
                 if constexpr(is_eto_eligible_v<typename std::remove_reference_t<decltype(*cpool)>::value_type>) {
-                    return std::make_tuple();
+                    return std::tuple{};
                 } else {
                     return std::forward_as_tuple(cpool->get(entt));
                 }
@@ -484,7 +484,7 @@ public:
         } else if constexpr(sizeof...(Comp) == 1) {
             return (std::get<pool_type<Comp> *>(pools)->get(entt), ...);
         } else {
-            return std::tuple<decltype(get<Comp>({}))...>{get<Comp>(entt)...};
+            return std::forward_as_tuple(get<Comp>(entt)...);
         }
     }
 
@@ -919,19 +919,28 @@ public:
      * far better performance than its counterpart.
      *
      * @warning
-     * Attempting to use an entity that doesn't belong to the view results in
-     * undefined behavior.<br/>
+     * Attempting to use an invalid component type results in a compilation
+     * error. Attempting to use an entity that doesn't belong to the view
+     * results in undefined behavior.<br/>
      * An assertion will abort the execution at runtime in debug mode if the
      * view doesn't contain the given entity.
      *
+     * @tparam Comp Types of components to get.
      * @param entt A valid entity identifier.
      * @return The component assigned to the entity.
      */
-    template<typename Comp = Component>
+    template<typename... Comp>
     [[nodiscard]] decltype(auto) get(const entity_type entt) const {
-        static_assert(std::is_same_v<Comp, Component>, "Invalid component type");
-        ENTT_ASSERT(contains(entt));
-        return pool->get(entt);
+        if constexpr(sizeof...(Comp) == 0) {
+            if constexpr(is_eto_eligible_v<Component>) {
+                return std::tuple{};
+            } else {
+                return std::forward_as_tuple(pool->get(entt));
+            }
+        } else {
+            static_assert(std::is_same_v<Comp..., Component>, "Invalid component type");
+            return pool->get(entt);
+        }
     }
 
     /**
@@ -969,7 +978,7 @@ public:
                 }
             }
         } else {
-            if constexpr(std::is_invocable_v<Func, decltype(get({}))>) {
+            if constexpr(std::is_invocable_v<Func, std::add_lvalue_reference_t<Component>>) {
                 for(auto &&component: *pool) {
                     func(component);
                 }

+ 6 - 2
test/entt/entity/group.cpp

@@ -330,12 +330,13 @@ TEST(NonOwningGroup, IndexRebuiltOnDestroy) {
 
 TEST(NonOwningGroup, ConstNonConstAndAllInBetween) {
     entt::registry registry;
-    auto group = registry.group(entt::get<int, const char>);
+    auto group = registry.group(entt::get<int, empty_type, const char>);
 
     ASSERT_EQ(group.size(), decltype(group.size()){0});
 
     const auto entity = registry.create();
     registry.emplace<int>(entity, 0);
+    registry.emplace<empty_type>(entity);
     registry.emplace<char>(entity, 'c');
 
     ASSERT_EQ(group.size(), decltype(group.size()){1});
@@ -343,6 +344,7 @@ TEST(NonOwningGroup, ConstNonConstAndAllInBetween) {
     static_assert(std::is_same_v<decltype(group.get<int>({})), int &>);
     static_assert(std::is_same_v<decltype(group.get<const char>({})), const char &>);
     static_assert(std::is_same_v<decltype(group.get<int, const char>({})), std::tuple<int &, const char &>>);
+    static_assert(std::is_same_v<decltype(group.get({})), std::tuple<int &, const char &>>);
     static_assert(std::is_same_v<decltype(group.raw<const char>()), const char *>);
     static_assert(std::is_same_v<decltype(group.raw<int>()), int *>);
 
@@ -984,13 +986,14 @@ TEST(OwningGroup, IndexRebuiltOnDestroy) {
 
 TEST(OwningGroup, ConstNonConstAndAllInBetween) {
     entt::registry registry;
-    auto group = registry.group<int, const char>(entt::get<double, const float>);
+    auto group = registry.group<int, const char>(entt::get<empty_type, double, const float>);
 
     ASSERT_EQ(group.size(), decltype(group.size()){0});
 
     const auto entity = registry.create();
     registry.emplace<int>(entity, 0);
     registry.emplace<char>(entity, 'c');
+    registry.emplace<empty_type>(entity);
     registry.emplace<double>(entity, 0.);
     registry.emplace<float>(entity, 0.f);
 
@@ -1001,6 +1004,7 @@ TEST(OwningGroup, ConstNonConstAndAllInBetween) {
     static_assert(std::is_same_v<decltype(group.get<double>({})), double &>);
     static_assert(std::is_same_v<decltype(group.get<const float>({})), const float &>);
     static_assert(std::is_same_v<decltype(group.get<int, const char, double, const float>({})), std::tuple<int &, const char &, double &, const float &>>);
+    static_assert(std::is_same_v<decltype(group.get({})), std::tuple<int &, const char &, double &, const float &>>);
     static_assert(std::is_same_v<decltype(group.raw<const float>()), const float *>);
     static_assert(std::is_same_v<decltype(group.raw<double>()), double *>);
     static_assert(std::is_same_v<decltype(group.raw<const char>()), const char *>);

+ 37 - 5
test/entt/entity/view.cpp

@@ -37,10 +37,10 @@ TEST(SingleComponentView, Functionalities) {
     ASSERT_EQ(view.size(), 2u);
 
     view.get<char>(e0) = '1';
-    view.get(e1) = '2';
+    std::get<0>(view.get(e1)) = '2';
 
     for(auto entity: view) {
-        ASSERT_TRUE(cview.get<const char>(entity) == '1' || cview.get(entity) == '2');
+        ASSERT_TRUE(cview.get<const char>(entity) == '1' || std::get<const char &>(cview.get(entity)) == '2');
     }
 
     ASSERT_EQ(*(view.data() + 0), e1);
@@ -157,9 +157,11 @@ TEST(SingleComponentView, ConstNonConstAndAllInBetween) {
     static_assert(std::is_same_v<typename decltype(view)::raw_type, int>);
     static_assert(std::is_same_v<typename decltype(cview)::raw_type, const int>);
 
-    static_assert(std::is_same_v<decltype(view.get({})), int &>);
+    static_assert(std::is_same_v<decltype(view.get<int>({})), int &>);
+    static_assert(std::is_same_v<decltype(view.get({})), std::tuple<int &>>);
     static_assert(std::is_same_v<decltype(view.raw()), int *>);
-    static_assert(std::is_same_v<decltype(cview.get({})), const int &>);
+    static_assert(std::is_same_v<decltype(cview.get<const int>({})), const int &>);
+    static_assert(std::is_same_v<decltype(cview.get({})), std::tuple<const int &>>);
     static_assert(std::is_same_v<decltype(cview.raw()), const int *>);
 
     view.each([](auto &&i) {
@@ -181,6 +183,34 @@ TEST(SingleComponentView, ConstNonConstAndAllInBetween) {
     }
 }
 
+TEST(SingleComponentView, ConstNonConstAndAllInBetweenWithEmptyType) {
+    entt::registry registry;
+    auto view = registry.view<empty_type>();
+    auto cview = std::as_const(registry).view<const empty_type>();
+
+    ASSERT_EQ(view.size(), decltype(view.size()){0});
+    ASSERT_EQ(cview.size(), decltype(cview.size()){0});
+
+    registry.emplace<empty_type>(registry.create());
+
+    ASSERT_EQ(view.size(), decltype(view.size()){1});
+    ASSERT_EQ(cview.size(), decltype(cview.size()){1});
+
+    static_assert(std::is_same_v<typename decltype(view)::raw_type, empty_type>);
+    static_assert(std::is_same_v<typename decltype(cview)::raw_type, const empty_type>);
+
+    static_assert(std::is_same_v<decltype(view.get({})), std::tuple<>>);
+    static_assert(std::is_same_v<decltype(cview.get({})), std::tuple<>>);
+
+    for(auto &&[entt]: view.each()) {
+        static_assert(std::is_same_v<decltype(entt), entt::entity>);
+    }
+
+    for(auto &&[entt]: cview.each()) {
+        static_assert(std::is_same_v<decltype(entt), entt::entity>);
+    }
+}
+
 TEST(SingleComponentView, Find) {
     entt::registry registry;
     auto view = registry.view<int>();
@@ -528,12 +558,13 @@ TEST(MultiComponentView, EachWithHoles) {
 
 TEST(MultiComponentView, ConstNonConstAndAllInBetween) {
     entt::registry registry;
-    auto view = registry.view<int, const char>();
+    auto view = registry.view<int, empty_type, const char>();
 
     ASSERT_EQ(view.size_hint(), decltype(view.size_hint()){0});
 
     const auto entity = registry.create();
     registry.emplace<int>(entity, 0);
+    registry.emplace<empty_type>(entity);
     registry.emplace<char>(entity, 'c');
 
     ASSERT_EQ(view.size_hint(), decltype(view.size_hint()){1});
@@ -541,6 +572,7 @@ TEST(MultiComponentView, ConstNonConstAndAllInBetween) {
     static_assert(std::is_same_v<decltype(view.get<int>({})), int &>);
     static_assert(std::is_same_v<decltype(view.get<const char>({})), const char &>);
     static_assert(std::is_same_v<decltype(view.get<int, const char>({})), std::tuple<int &, const char &>>);
+    static_assert(std::is_same_v<decltype(view.get({})), std::tuple<int &, const char &>>);
 
     view.each([](auto &&i, auto &&c) {
         static_assert(std::is_same_v<decltype(i), int &>);