From 9983faef70cddd722d9cf69c924ce04b09abb466 Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Fri, 16 Oct 2020 14:18:43 +0200 Subject: [PATCH] view/group: consistent ::get without template arguments (breaking changes) --- TODO | 1 + docs/md/entity.md | 27 ++++++++++++------------ src/entt/entity/group.hpp | 20 +++++++++--------- src/entt/entity/view.hpp | 31 ++++++++++++++++++---------- test/entt/entity/group.cpp | 8 ++++++-- test/entt/entity/view.cpp | 42 +++++++++++++++++++++++++++++++++----- 6 files changed, 88 insertions(+), 41 deletions(-) diff --git a/TODO b/TODO index b0388d69f..2e0cceb4d 100644 --- a/TODO +++ b/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 diff --git a/docs/md/entity.md b/docs/md/entity.md index e1cd1fb82..8b783455f 100644 --- a/docs/md/entity.md +++ b/docs/md/entity.md @@ -1343,16 +1343,19 @@ auto view = registry.view(entt::exclude); To iterate a view, either use it in a range-for loop: ```cpp -auto view = registry.view(); +auto view = registry.view(); for(auto entity: view) { // a component at a time ... auto &position = view.get(entity); auto &velocity = view.get(entity); - // ... or multiple components at once + // ... multiple components ... auto [pos, vel] = view.get(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(); 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::value(), entt::type_hash(entity); - auto &velocity = registry.get(entity); - - // ... or multiple components at once - auto [pos, vel] = registry.get(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(entt::get); +auto group = registry.group(entt::get); for(auto entity: group) { // a component at a time ... auto &position = group.get(entity); auto &velocity = group.get(entity); - // ... or multiple components at once + // ... multiple components ... auto [pos, vel] = group.get(entity); + // ... all components at once + auto [pos, vel, rend] = group.get(entity); + // ... } ``` diff --git a/src/entt/entity/group.hpp b/src/entt/entity/group.hpp index e4469d875..06824a43a 100644 --- a/src/entt/entity/group.hpp +++ b/src/entt/entity/group.hpp @@ -135,7 +135,7 @@ class basic_group, get_t> final { [[nodiscard]] iterator begin() const ENTT_NOEXCEPT { return iterable_group_iterator{handler->begin(), std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::value_type>) { - return std::make_tuple(); + return std::tuple{}; } else { return std::make_tuple(cpool); } @@ -145,7 +145,7 @@ class basic_group, get_t> final { [[nodiscard]] iterator end() const ENTT_NOEXCEPT { return iterable_group_iterator{handler->end(), std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::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::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 *>(pools)->get(entt), ...); } else { - return std::tuple({}))...>{get(entt)...}; + return std::forward_as_tuple(get(entt)...); } } @@ -692,14 +692,14 @@ class basic_group, get_t, Owned...> final std::get<0>(pools)->basic_sparse_set::end() - *length, std::tuple_cat([length = *length](auto *cpool) { if constexpr(is_eto_eligible_v::value_type>) { - return std::make_tuple(); + return std::tuple{}; } else { return std::make_tuple(cpool->end() - length); } }(std::get *>(pools))...), std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::value_type>) { - return std::make_tuple(); + return std::tuple{}; } else { return std::make_tuple(cpool); } @@ -712,14 +712,14 @@ class basic_group, get_t, Owned...> final std::get<0>(pools)->basic_sparse_set::end(), std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::value_type>) { - return std::make_tuple(); + return std::tuple{}; } else { return std::make_tuple(cpool->end()); } }(std::get *>(pools))...), std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::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::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 *>(pools)->get(entt), ...); } else { - return std::tuple({}))...>{get(entt)...}; + return std::forward_as_tuple(get(entt)...); } } diff --git a/src/entt/entity/view.hpp b/src/entt/entity/view.hpp index 76b7d1ba1..7deb64eb4 100644 --- a/src/entt/entity/view.hpp +++ b/src/entt/entity/view.hpp @@ -215,7 +215,7 @@ class basic_view, Component...> final { [[nodiscard]] iterator begin() const ENTT_NOEXCEPT { return iterable_view_iterator{first, std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::value_type>) { - return std::make_tuple(); + return std::tuple{}; } else { return std::make_tuple(cpool); } @@ -225,7 +225,7 @@ class basic_view, Component...> final { [[nodiscard]] iterator end() const ENTT_NOEXCEPT { return iterable_view_iterator{last, std::tuple_cat([](auto *cpool) { if constexpr(is_eto_eligible_v::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::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 *>(pools)->get(entt), ...); } else { - return std::tuple({}))...>{get(entt)...}; + return std::forward_as_tuple(get(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.
+ * 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.
* 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 + template [[nodiscard]] decltype(auto) get(const entity_type entt) const { - static_assert(std::is_same_v, "Invalid component type"); - ENTT_ASSERT(contains(entt)); - return pool->get(entt); + if constexpr(sizeof...(Comp) == 0) { + if constexpr(is_eto_eligible_v) { + return std::tuple{}; + } else { + return std::forward_as_tuple(pool->get(entt)); + } + } else { + static_assert(std::is_same_v, "Invalid component type"); + return pool->get(entt); + } } /** @@ -969,7 +978,7 @@ public: } } } else { - if constexpr(std::is_invocable_v) { + if constexpr(std::is_invocable_v>) { for(auto &&component: *pool) { func(component); } diff --git a/test/entt/entity/group.cpp b/test/entt/entity/group.cpp index a972c9e9c..ff589f402 100644 --- a/test/entt/entity/group.cpp +++ b/test/entt/entity/group.cpp @@ -330,12 +330,13 @@ TEST(NonOwningGroup, IndexRebuiltOnDestroy) { TEST(NonOwningGroup, ConstNonConstAndAllInBetween) { entt::registry registry; - auto group = registry.group(entt::get); + auto group = registry.group(entt::get); ASSERT_EQ(group.size(), decltype(group.size()){0}); const auto entity = registry.create(); registry.emplace(entity, 0); + registry.emplace(entity); registry.emplace(entity, 'c'); ASSERT_EQ(group.size(), decltype(group.size()){1}); @@ -343,6 +344,7 @@ TEST(NonOwningGroup, ConstNonConstAndAllInBetween) { static_assert(std::is_same_v({})), int &>); static_assert(std::is_same_v({})), const char &>); static_assert(std::is_same_v({})), std::tuple>); + static_assert(std::is_same_v>); static_assert(std::is_same_v()), const char *>); static_assert(std::is_same_v()), int *>); @@ -984,13 +986,14 @@ TEST(OwningGroup, IndexRebuiltOnDestroy) { TEST(OwningGroup, ConstNonConstAndAllInBetween) { entt::registry registry; - auto group = registry.group(entt::get); + auto group = registry.group(entt::get); ASSERT_EQ(group.size(), decltype(group.size()){0}); const auto entity = registry.create(); registry.emplace(entity, 0); registry.emplace(entity, 'c'); + registry.emplace(entity); registry.emplace(entity, 0.); registry.emplace(entity, 0.f); @@ -1001,6 +1004,7 @@ TEST(OwningGroup, ConstNonConstAndAllInBetween) { static_assert(std::is_same_v({})), double &>); static_assert(std::is_same_v({})), const float &>); static_assert(std::is_same_v({})), std::tuple>); + static_assert(std::is_same_v>); static_assert(std::is_same_v()), const float *>); static_assert(std::is_same_v()), double *>); static_assert(std::is_same_v()), const char *>); diff --git a/test/entt/entity/view.cpp b/test/entt/entity/view.cpp index 4da9107ca..b43b41bb9 100644 --- a/test/entt/entity/view.cpp +++ b/test/entt/entity/view.cpp @@ -37,10 +37,10 @@ TEST(SingleComponentView, Functionalities) { ASSERT_EQ(view.size(), 2u); view.get(e0) = '1'; - view.get(e1) = '2'; + std::get<0>(view.get(e1)) = '2'; for(auto entity: view) { - ASSERT_TRUE(cview.get(entity) == '1' || cview.get(entity) == '2'); + ASSERT_TRUE(cview.get(entity) == '1' || std::get(cview.get(entity)) == '2'); } ASSERT_EQ(*(view.data() + 0), e1); @@ -157,9 +157,11 @@ TEST(SingleComponentView, ConstNonConstAndAllInBetween) { static_assert(std::is_same_v); static_assert(std::is_same_v); - static_assert(std::is_same_v); + static_assert(std::is_same_v({})), int &>); + static_assert(std::is_same_v>); static_assert(std::is_same_v); - static_assert(std::is_same_v); + static_assert(std::is_same_v({})), const int &>); + static_assert(std::is_same_v>); static_assert(std::is_same_v); view.each([](auto &&i) { @@ -181,6 +183,34 @@ TEST(SingleComponentView, ConstNonConstAndAllInBetween) { } } +TEST(SingleComponentView, ConstNonConstAndAllInBetweenWithEmptyType) { + entt::registry registry; + auto view = registry.view(); + auto cview = std::as_const(registry).view(); + + ASSERT_EQ(view.size(), decltype(view.size()){0}); + ASSERT_EQ(cview.size(), decltype(cview.size()){0}); + + registry.emplace(registry.create()); + + ASSERT_EQ(view.size(), decltype(view.size()){1}); + ASSERT_EQ(cview.size(), decltype(cview.size()){1}); + + static_assert(std::is_same_v); + static_assert(std::is_same_v); + + static_assert(std::is_same_v>); + static_assert(std::is_same_v>); + + for(auto &&[entt]: view.each()) { + static_assert(std::is_same_v); + } + + for(auto &&[entt]: cview.each()) { + static_assert(std::is_same_v); + } +} + TEST(SingleComponentView, Find) { entt::registry registry; auto view = registry.view(); @@ -528,12 +558,13 @@ TEST(MultiComponentView, EachWithHoles) { TEST(MultiComponentView, ConstNonConstAndAllInBetween) { entt::registry registry; - auto view = registry.view(); + auto view = registry.view(); ASSERT_EQ(view.size_hint(), decltype(view.size_hint()){0}); const auto entity = registry.create(); registry.emplace(entity, 0); + registry.emplace(entity); registry.emplace(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({})), int &>); static_assert(std::is_same_v({})), const char &>); static_assert(std::is_same_v({})), std::tuple>); + static_assert(std::is_same_v>); view.each([](auto &&i, auto &&c) { static_assert(std::is_same_v);