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

registry/view (close #660):
* const registry::view is no longer thread safe
* views are always valid when created from a registry, either const or not
* added a note to the doc about const registry and multithreading

Michele Caini 5 лет назад
Родитель
Сommit
c3f7f83c55
5 измененных файлов с 120 добавлено и 126 удалено
  1. 2 0
      TODO
  2. 23 27
      docs/md/entity.md
  3. 13 13
      src/entt/entity/registry.hpp
  4. 45 48
      src/entt/entity/view.hpp
  5. 37 38
      test/entt/entity/view.cpp

+ 2 - 0
TODO

@@ -9,6 +9,8 @@
 * custom pools example (multi instance, tables, enable/disable, and so on...)
 
 WIP:
+* HP: any_vector for context variables
+* HP: make const registry::view thread safe, switch to a view<T...>{registry} model (long term goal)
 * HP: remove storage category and get_as_tuple, make storage classes return tuple (+ view generator detect shared storage in future)?
 * HP: review registry::get, registry::try_get
 * HP: weak reference wrapper example with custom storage

+ 23 - 27
docs/md/entity.md

@@ -40,7 +40,6 @@
     * [Partial-owning groups](#partial-owning-groups)
     * [Non-owning groups](#non-owning-groups)
     * [Nested groups](#nested-groups)
-  * [Invalid views and groups](#invalid-views-and-groups)
   * [Types: const, non-const and all in between](#types-const-non-const-and-all-in-between)
   * [Give me everything](#give-me-everything)
   * [What is allowed and what is not](#what-is-allowed-and-what-is-not)
@@ -48,6 +47,7 @@
 * [Empty type optimization](#empty-type-optimization)
 * [Multithreading](#multithreading)
   * [Iterators](#iterators)
+  * [Const registry](#const-registry)
 * [Beyond this document](#beyond-this-document)
 <!--
 @endcond TURN_OFF_DOXYGEN
@@ -1666,32 +1666,6 @@ restrictive of them. To prevent users from having to remember which of their
 groups is the most restrictive, the registry class offers the `sortable` member
 function to know if a group can be sorted or not.
 
-## Invalid views and groups
-
-Views and groups as returned by a registry are generally valid. However, there
-are some exceptions where an invalid object might be returned.<br/>
-In these cases, they should be renewed as soon as possible. In fact, an invalid
-view or group contains a broken reference to one or more pools and this will
-never be fixed. The view or the group will continue to return no data, even if
-the pool for the pending reference is created in the registry in the meantime.
-
-There is only one case in which an invalid object can be returned, that is when
-the view or the group is created from a constant reference to a registry in
-which the required pools haven't yet been created.<br/>
-Pools are typically created whenever any method is used on a non-const registry.
-This also means that creating views and groups from a non-const registry can
-never result in an invalid object.
-
-It's also perfectly fine to use an invalid view or group, to invoke `each` on
-them or to iterate them like any other object. The only difference from a valid
-view or group is that the invalid ones will always appear as _empty_.<br/>
-In general, when views and groups are created on the fly and used at the same
-time, then discarded immediately afterwards, it doesn't matter whether or not
-they may be invalid. Therefore, this remains the recommended approach.
-
-To know if a view or a group is properly initialized, both can be converted to
-bool explicitly and used in a guard.
-
 ## Types: const, non-const and all in between
 
 The `registry` class offers two overloads when it comes to constructing views
@@ -1975,6 +1949,28 @@ both the entities and a list of references to their components by default sooner
 or later. Multi-pass guarantee won't break in any case and the performance
 should even benefit from it further.
 
+## Const registry
+
+Contrary to what the standard library containers offer, a const registry is
+generally but not completely thread safe.<br/>
+In particular, one (and only one) of its const member functions isn't fully
+thread safe. That is the `view` method.
+
+The reason for this is easy to explain. To avoid requiring types to be
+_announced_ in advance, the registry lazily initializes the storage objects for
+the different components.<br/>
+In most cases, this isn't even necessary. The absence of a storage is itself the
+required information. However, when building a view, all pools must necessarily
+exist. This makes the `view` member function not thread safe even in its const
+overload, unless all pools already exist.
+
+Fortunately, there is also a way to instantiate storage classes early when in
+doubt or when there are special requirements.<br/>
+Calling the `prepare` method is equivalent to _announcing_ the existence of a
+particular storage, to avoid running into problems. For those interested, there
+are also alternative approaches, such as a single threaded tick for the registry
+warm-up, but these are not always applicable.
+
 # Beyond this document
 
 There are many other features and functions not listed in this document.<br/>

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

@@ -105,7 +105,7 @@ class basic_registry {
     };
 
     template<typename Component>
-    [[nodiscard]] storage_type<Component> * assure() {
+    [[nodiscard]] storage_type<Component> * assure() const {
         const auto index = type_seq<Component>::value();
 
         if(!(index < pools.size())) {
@@ -121,7 +121,7 @@ class basic_registry {
     }
 
     template<typename Component>
-    [[nodiscard]] const storage_type<Component> * assure() const {
+    [[nodiscard]] const storage_type<Component> * pool_if_exists() const {
         const auto index = type_seq<Component>::value();
         return (!(index < pools.size()) || !pools[index].pool) ? nullptr : static_cast<const storage_type<Component> *>(pools[index].pool.get());
     }
@@ -225,7 +225,7 @@ public:
      */
     template<typename Component>
     [[nodiscard]] size_type size() const {
-        const auto *cpool = assure<Component>();
+        const auto *cpool = pool_if_exists<Component>();
         return cpool ? cpool->size() : size_type{};
     }
 
@@ -288,7 +288,7 @@ public:
      */
     template<typename Component>
     [[nodiscard]] size_type capacity() const {
-        const auto *cpool = assure<Component>();
+        const auto *cpool = pool_if_exists<Component>();
         return cpool ? cpool->capacity() : size_type{};
     }
 
@@ -327,7 +327,7 @@ public:
         if constexpr(sizeof...(Component) == 0) {
             return !alive();
         } else {
-            return [](auto *... cpool) { return ((!cpool || cpool->empty()) && ...); }(assure<Component>()...);
+            return [](auto *... cpool) { return ((!cpool || cpool->empty()) && ...); }(pool_if_exists<Component>()...);
         }
     }
 
@@ -766,7 +766,7 @@ public:
     template<typename... Component>
     [[nodiscard]] bool all_of(const entity_type entity) const {
         ENTT_ASSERT(valid(entity));
-        return [entity](auto *... cpool) { return ((cpool && cpool->contains(entity)) && ...); }(assure<Component>()...);
+        return [entity](auto *... cpool) { return ((cpool && cpool->contains(entity)) && ...); }(pool_if_exists<Component>()...);
     }
 
     /**
@@ -802,9 +802,9 @@ public:
         ENTT_ASSERT(valid(entity));
 
         if constexpr(sizeof...(Component) == 1) {
-            return (assure<Component>()->get(entity), ...);
+            return (pool_if_exists<Component>()->get(entity), ...);
         } else {
-            return std::forward_as_tuple(assure<Component>()->get(entity)...);
+            return std::forward_as_tuple(pool_if_exists<Component>()->get(entity)...);
         }
     }
 
@@ -867,7 +867,7 @@ public:
         ENTT_ASSERT(valid(entity));
 
         if constexpr(sizeof...(Component) == 1) {
-            auto *cpool = assure<Component...>();
+            auto *cpool = pool_if_exists<Component...>();
             return (cpool && cpool->contains(entity)) ? &cpool->get(entity) : nullptr;
         } else {
             return std::make_tuple(try_get<Component>(entity)...);
@@ -1083,8 +1083,8 @@ public:
     template<typename... Component, typename... Exclude>
     [[nodiscard]] basic_view<Entity, exclude_t<Exclude...>, Component...> view(exclude_t<Exclude...> = {}) const {
         static_assert(sizeof...(Component) > 0, "Exclusion-only views are not supported");
-        using view_type = basic_view<Entity, exclude_t<Exclude...>, Component...>;
-        return [](auto *... cpools) { return (cpools && ...) ? view_type{*cpools...} : view_type{}; }(assure<std::decay_t<Component>>()..., assure<Exclude>()...);
+        static_assert((std::is_const_v<Component> && ...), "Invalid non-const type");
+        return { *assure<std::decay_t<Component>>()..., *assure<Exclude>()... };
     }
 
     /*! @copydoc view */
@@ -1271,7 +1271,7 @@ public:
             return {};
         } else {
             using handler_type = group_handler<exclude_t<Exclude...>, get_t<std::decay_t<Get>...>, std::decay_t<Owned>...>;
-            return { static_cast<handler_type *>(it->group.get())->current, *assure<std::decay_t<Owned>>()... , *assure<std::decay_t<Get>>()... };
+            return { static_cast<handler_type *>(it->group.get())->current, *pool_if_exists<std::decay_t<Owned>>()... , *pool_if_exists<std::decay_t<Get>>()... };
         }
     }
 
@@ -1594,7 +1594,7 @@ public:
 
 private:
     std::vector<basic_any<0u>> vars{};
-    std::vector<pool_data> pools{};
+    mutable std::vector<pool_data> pools{};
     std::vector<group_data> groups{};
     std::vector<entity_type> entities{};
     entity_type available{null};

+ 45 - 48
src/entt/entity/view.hpp

@@ -245,35 +245,33 @@ class basic_view<Entity, exclude_t<Exclude...>, Component...> final {
 
     template<typename Comp, typename Func>
     void traverse(Func func) const {
-        if(*this) {
-            if constexpr(std::is_same_v<typename storage_type<Comp>::storage_category, empty_storage_tag>) {
-                for(const auto entt: static_cast<const basic_sparse_set<entity_type> &>(*std::get<storage_type<Comp> *>(pools))) {
-                    if(((std::is_same_v<Comp, Component> || std::get<storage_type<Component> *>(pools)->contains(entt)) && ...)
-                        && !(std::get<const storage_type<Exclude> *>(filter)->contains(entt) || ...))
-                    {
-                        if constexpr(is_applicable_v<Func, decltype(std::tuple_cat(std::tuple<entity_type>{}, std::declval<basic_view>().get({})))>) {
-                            std::apply(func, std::tuple_cat(std::make_tuple(entt), get(entt)));
-                        } else {
-                            std::apply(func, get(entt));
-                        }
+        if constexpr(std::is_same_v<typename storage_type<Comp>::storage_category, empty_storage_tag>) {
+            for(const auto entt: static_cast<const basic_sparse_set<entity_type> &>(*std::get<storage_type<Comp> *>(pools))) {
+                if(((std::is_same_v<Comp, Component> || std::get<storage_type<Component> *>(pools)->contains(entt)) && ...)
+                    && !(std::get<const storage_type<Exclude> *>(filter)->contains(entt) || ...))
+                {
+                    if constexpr(is_applicable_v<Func, decltype(std::tuple_cat(std::tuple<entity_type>{}, std::declval<basic_view>().get({})))>) {
+                        std::apply(func, std::tuple_cat(std::make_tuple(entt), get(entt)));
+                    } else {
+                        std::apply(func, get(entt));
                     }
                 }
-            } else {
-                auto it = std::get<storage_type<Comp> *>(pools)->begin();
-
-                for(const auto entt: static_cast<const basic_sparse_set<entity_type> &>(*std::get<storage_type<Comp> *>(pools))) {
-                    if(((std::is_same_v<Comp, Component> || std::get<storage_type<Component> *>(pools)->contains(entt)) && ...)
-                        && !(std::get<const storage_type<Exclude> *>(filter)->contains(entt) || ...))
-                    {
-                        if constexpr(is_applicable_v<Func, decltype(std::tuple_cat(std::tuple<entity_type>{}, std::declval<basic_view>().get({})))>) {
-                            std::apply(func, std::tuple_cat(std::make_tuple(entt), dispatch_get<Component>(it, entt)...));
-                        } else {
-                            std::apply(func, std::tuple_cat(dispatch_get<Component>(it, entt)...));
-                        }
+            }
+        } else {
+            auto it = std::get<storage_type<Comp> *>(pools)->begin();
+        
+            for(const auto entt: static_cast<const basic_sparse_set<entity_type> &>(*std::get<storage_type<Comp> *>(pools))) {
+                if(((std::is_same_v<Comp, Component> || std::get<storage_type<Component> *>(pools)->contains(entt)) && ...)
+                    && !(std::get<const storage_type<Exclude> *>(filter)->contains(entt) || ...))
+                {
+                    if constexpr(is_applicable_v<Func, decltype(std::tuple_cat(std::tuple<entity_type>{}, std::declval<basic_view>().get({})))>) {
+                        std::apply(func, std::tuple_cat(std::make_tuple(entt), dispatch_get<Component>(it, entt)...));
+                    } else {
+                        std::apply(func, std::tuple_cat(dispatch_get<Component>(it, entt)...));
                     }
-
-                    ++it;
                 }
+        
+                ++it;
             }
         }
     }
@@ -310,7 +308,7 @@ public:
      */
     template<typename Comp>
     void use() const ENTT_NOEXCEPT {
-        view = *this ? std::get<storage_type<Comp> *>(pools) : nullptr;
+        view = std::get<storage_type<Comp> *>(pools);
     }
 
     /**
@@ -318,7 +316,7 @@ public:
      * @return Estimated number of entities iterated by the view.
      */
     [[nodiscard]] size_type size_hint() const ENTT_NOEXCEPT {
-        return *this ? view->size() : size_type{};
+        return view->size();
     }
 
     /**
@@ -330,7 +328,7 @@ public:
      * @return An iterator to the first entity of the view.
      */
     [[nodiscard]] iterator begin() const {
-        return *this ? iterator{view->begin(), view->end(), view->begin(), unchecked(view), filter} : iterator{};
+        return iterator{view->begin(), view->end(), view->begin(), unchecked(view), filter};
     }
 
     /**
@@ -343,7 +341,7 @@ public:
      * @return An iterator to the entity following the last entity of the view.
      */
     [[nodiscard]] iterator end() const {
-        return *this ? iterator{view->begin(), view->end(), view->end(), unchecked(view), filter} : iterator{};
+        return iterator{view->begin(), view->end(), view->end(), unchecked(view), filter};
     }
 
     /**
@@ -355,7 +353,7 @@ public:
      * @return An iterator to the first entity of the reversed view.
      */
     [[nodiscard]] reverse_iterator rbegin() const {
-        return *this ? reverse_iterator{view->rbegin(), view->rend(), view->rbegin(), unchecked(view), filter} : reverse_iterator{};
+        return reverse_iterator{view->rbegin(), view->rend(), view->rbegin(), unchecked(view), filter};
     }
 
     /**
@@ -370,7 +368,7 @@ public:
      * reversed view.
      */
     [[nodiscard]] reverse_iterator rend() const {
-        return *this ? reverse_iterator{view->rbegin(), view->rend(), view->rend(), unchecked(view), filter} : reverse_iterator{};
+        return reverse_iterator{view->rbegin(), view->rend(), view->rend(), unchecked(view), filter};
     }
 
     /**
@@ -400,7 +398,7 @@ public:
      * iterator otherwise.
      */
     [[nodiscard]] iterator find(const entity_type entt) const {
-        const auto it = *this ? iterator{view->begin(), view->end(), view->find(entt), unchecked(view), filter} : end();
+        const auto it = iterator{view->begin(), view->end(), view->find(entt), unchecked(view), filter};
         return (it != end() && *it == entt) ? it : end();
     }
 
@@ -418,7 +416,7 @@ public:
      * @return True if the view contains the given entity, false otherwise.
      */
     [[nodiscard]] bool contains(const entity_type entt) const {
-        return *this && (std::get<storage_type<Component> *>(pools)->contains(entt) && ...) && !(std::get<const storage_type<Exclude> *>(filter)->contains(entt) || ...);
+        return (std::get<storage_type<Component> *>(pools)->contains(entt) && ...) && !(std::get<const storage_type<Exclude> *>(filter)->contains(entt) || ...);
     }
 
     /**
@@ -445,7 +443,8 @@ public:
         } else if constexpr(sizeof...(Comp) == 1) {
             return (std::get<storage_type<Comp> *>(pools)->get(entt), ...);
         } else {
-            return std::tuple_cat(get_as_tuple(*std::get<storage_type<Comp> *>(pools), entt)...);        }
+            return std::tuple_cat(get_as_tuple(*std::get<storage_type<Comp> *>(pools), entt)...);
+        }
     }
 
     /**
@@ -690,7 +689,7 @@ public:
      * @return Number of entities that have the given component.
      */
     [[nodiscard]] size_type size() const ENTT_NOEXCEPT {
-        return *this ? pool->size() : size_type{};
+        return pool->size();
     }
 
     /**
@@ -698,7 +697,7 @@ public:
      * @return True if the view is empty, false otherwise.
      */
     [[nodiscard]] bool empty() const ENTT_NOEXCEPT {
-        return !*this || pool->empty();
+        return pool->empty();
     }
 
     /**
@@ -710,7 +709,7 @@ public:
      * @return A pointer to the array of components.
      */
     [[nodiscard]] raw_type * raw() const ENTT_NOEXCEPT {
-        return *this ? pool->raw() : nullptr;
+        return pool->raw();
     }
 
     /**
@@ -722,7 +721,7 @@ public:
      * @return A pointer to the array of entities.
      */
     [[nodiscard]] const entity_type * data() const ENTT_NOEXCEPT {
-        return *this ? pool->data() : nullptr;
+        return pool->data();
     }
 
     /**
@@ -734,7 +733,7 @@ public:
      * @return An iterator to the first entity of the view.
      */
     [[nodiscard]] iterator begin() const ENTT_NOEXCEPT {
-        return *this ? pool->basic_sparse_set<entity_type>::begin() : iterator{};
+        return pool->basic_sparse_set<entity_type>::begin();
     }
 
     /**
@@ -747,7 +746,7 @@ public:
      * @return An iterator to the entity following the last entity of the view.
      */
     [[nodiscard]] iterator end() const ENTT_NOEXCEPT {
-        return *this ? pool->basic_sparse_set<entity_type>::end() : iterator{};
+        return pool->basic_sparse_set<entity_type>::end();
     }
 
     /**
@@ -759,7 +758,7 @@ public:
      * @return An iterator to the first entity of the reversed view.
      */
     [[nodiscard]] reverse_iterator rbegin() const ENTT_NOEXCEPT {
-        return *this ? pool->basic_sparse_set<entity_type>::rbegin() : reverse_iterator{};
+        return pool->basic_sparse_set<entity_type>::rbegin();
     }
 
     /**
@@ -774,7 +773,7 @@ public:
      * reversed view.
      */
     [[nodiscard]] reverse_iterator rend() const ENTT_NOEXCEPT {
-        return *this ? pool->basic_sparse_set<entity_type>::rend() : reverse_iterator{};
+        return pool->basic_sparse_set<entity_type>::rend();
     }
 
     /**
@@ -804,7 +803,7 @@ public:
      * iterator otherwise.
      */
     [[nodiscard]] iterator find(const entity_type entt) const {
-        const auto it = *this ? pool->find(entt) : end();
+        const auto it = pool->find(entt);
         return it != end() && *it == entt ? it : end();
     }
 
@@ -831,7 +830,7 @@ public:
      * @return True if the view contains the given entity, false otherwise.
      */
     [[nodiscard]] bool contains(const entity_type entt) const {
-        return *this && pool->contains(entt);
+        return pool->contains(entt);
     }
 
     /**
@@ -901,10 +900,8 @@ public:
                     std::apply(func, pack);
                 }
             } else {
-                if(*this) {
-                    for(auto &&component: *pool) {
-                        func(component);
-                    }
+                for(auto &&component: *pool) {
+                    func(component);
                 }
             }
         }

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

@@ -92,7 +92,7 @@ TEST(SingleComponentView, RawData) {
     ASSERT_EQ(cview.size(), 0u);
 }
 
-TEST(SingleComponentView, Invalid) {
+TEST(SingleComponentView, LazyTypeFromConstRegistry) {
     entt::registry registry{};
     auto eview = std::as_const(registry).view<const empty_type>();
     auto cview = std::as_const(registry).view<const int>();
@@ -101,34 +101,21 @@ TEST(SingleComponentView, Invalid) {
     registry.emplace<empty_type>(entity);
     registry.emplace<int>(entity);
 
-    ASSERT_FALSE(cview);
-    ASSERT_FALSE(eview);
-
-    ASSERT_TRUE(cview.empty());
-    ASSERT_EQ(eview.size(), 0u);
-
-    ASSERT_EQ(cview.raw(), nullptr);
-    ASSERT_EQ(eview.data(), nullptr);
-
-    ASSERT_EQ(cview.begin(), cview.end());
-    ASSERT_EQ(eview.rbegin(), eview.rend());
-
-    ASSERT_FALSE(cview.contains(entity));
-    ASSERT_EQ(eview.find(entity), eview.end());
-    ASSERT_EQ(cview.front(), entt::entity{entt::null});
-    ASSERT_EQ(eview.back(), entt::entity{entt::null});
-
-    cview.each([](const auto, const auto &) { FAIL(); });
-    cview.each([](const auto &) { FAIL(); });
+    ASSERT_TRUE(cview);
+    ASSERT_TRUE(eview);
 
-    eview.each([](const auto) { FAIL(); });
-    eview.each([]() { FAIL(); });
+    ASSERT_NE(cview.raw(), nullptr);
+    ASSERT_NE(eview.data(), nullptr);
 
-    for([[maybe_unused]] auto all: cview.each()) { FAIL(); }
-    for(auto first = cview.each().rbegin(), last = cview.each().rend(); first != last; ++first) { FAIL(); }
+    ASSERT_FALSE(cview.empty());
+    ASSERT_EQ(eview.size(), 1u);
+    ASSERT_TRUE(cview.contains(entity));
 
-    for([[maybe_unused]] auto entt: eview.each()) { FAIL(); }
-    for(auto first = eview.each().rbegin(), last = eview.each().rend(); first != last; ++first) { FAIL(); }
+    ASSERT_NE(cview.begin(), cview.end());
+    ASSERT_NE(eview.rbegin(), eview.rend());
+    ASSERT_NE(eview.find(entity), eview.end());
+    ASSERT_EQ(cview.front(), entity);
+    ASSERT_EQ(eview.back(), entity);
 }
 
 TEST(SingleComponentView, ElementAccess) {
@@ -431,7 +418,7 @@ TEST(MultiComponentView, Functionalities) {
     ASSERT_FALSE(invalid);
 }
 
-TEST(MultiComponentView, Invalid) {
+TEST(MultiComponentView, LazyTypesFromConstRegistry) {
     entt::registry registry{};
     auto view = std::as_const(registry).view<const empty_type, const int>();
 
@@ -439,22 +426,34 @@ TEST(MultiComponentView, Invalid) {
     registry.emplace<empty_type>(entity);
     registry.emplace<int>(entity);
 
-    ASSERT_FALSE(view);
+    ASSERT_TRUE(view);
 
-    ASSERT_EQ(view.size_hint(), 0u);
+    ASSERT_EQ(view.size_hint(), 1u);
+    ASSERT_TRUE(view.contains(entity));
 
-    ASSERT_EQ(view.begin(), view.end());
+    ASSERT_NE(view.begin(), view.end());
+    ASSERT_NE(view.find(entity), view.end());
+    ASSERT_EQ(view.front(), entity);
+    ASSERT_EQ(view.back(), entity);
+}
 
-    ASSERT_FALSE(view.contains(entity));
-    ASSERT_EQ(view.find(entity), view.end());
-    ASSERT_EQ(view.front(), entt::entity{entt::null});
-    ASSERT_EQ(view.back(), entt::entity{entt::null});
+TEST(MultiComponentView, LazyExcludedTypeFromConstRegistry) {
+    entt::registry registry;
+
+    auto entity = registry.create();
+    registry.emplace<int>(entity);
 
-    view.each([](const auto, const auto &) { FAIL(); });
-    view.each([](const auto &) { FAIL(); });
+    auto view = std::as_const(registry).view<const int>(entt::exclude<char>);
 
-    for([[maybe_unused]] auto all: view.each()) { FAIL(); }
-    for(auto first = view.each().rbegin(), last = view.each().rend(); first != last; ++first) { FAIL(); }
+    ASSERT_TRUE(view);
+
+    ASSERT_EQ(view.size_hint(), 1u);
+    ASSERT_TRUE(view.contains(entity));
+
+    ASSERT_NE(view.begin(), view.end());
+    ASSERT_NE(view.find(entity), view.end());
+    ASSERT_EQ(view.front(), entity);
+    ASSERT_EQ(view.back(), entity);
 }
 
 TEST(MultiComponentView, Iterator) {