registry (and a few others): callbacks no longer receive components, see #386

TODO

@@ -19,6 +19,6 @@
   - get -> all, exclude -> none
 
 * WIP: snapshot rework/deprecation
- - range-assign cannot return iterators (eg group or sort-like listeners, see #386 - probably we cannot pass the component to the callback...)
+ - range-assign cannot return iterators (see #386)
  - range-assign should accept also an iterator from which to "copy" instances
  - remove snapshot/loader from registry, make them external (faster) tools

docs/md/entity.md

@@ -329,27 +329,25 @@ To be notified when components are destroyed, use the `on_destroy` member
 function instead. Finally, the `on_replace` member function will return a sink
 to which to connect listeners to observe changes.
 
-The function type of a listener for the construction signal should be equivalent
-to the following:
+The function type of a listener for the construction and destruction signals
+should be equivalent to the following:
 
 ```cpp
-void(entt::entity, entt::registry &, Component &);
+void(entt::entity, entt::registry &);
 ```
 
-Where `Component` is intuitively the type of component of interest. The listener
-is provided with the registry that triggered the notification, the entity
-affected by the change and the newly created instance.<br/>
-The sink returned by the `on_replace` member function accepts listeners the
-signature of which is the same of that of the construction signal. The one of
-the destruction signal is also similar, except for the `Component` parameter:
+In both cases, listeners are provided with the registry that triggered the
+notification and the entity affected by the change.<br/>
+The function type of a listener that observes changes to components is slightly
+different instead:
 
 ```cpp
-void(entt::entity, entt::registry &);
+void(entt::entity, entt::registry &, Component &);
 ```
 
-This is mainly due to performance reasons. While the component is made available
-after the construction, it is not when destroyed. Because of that, there are no
-reasons to get it from the underlying storage unless the user requires so.
+In this case, `Component` is intuitively the type of component of interest. The
+extra argument is required because the registry cannot store and therefore
+return both the instances of the given type for an entity.
 
 Note also that:
 

src/entt/entity/observer.hpp

@@ -179,8 +179,11 @@ class basic_observer {
         template<std::size_t Index>
         static void maybe_valid_if(basic_observer &obs, const Entity entt, const basic_registry<Entity> &reg) {
             if(reg.template has<Require...>(entt) && !(reg.template has<Reject>(entt) || ...)) {
-                auto *comp = obs.view.try_get(entt);
-                (comp ? *comp : obs.view.construct(entt)) |= (1 << Index);
+                if(auto *comp = obs.view.try_get(entt); !comp) {
+                    obs.view.construct(entt);
+                }
+
+                obs.view.get(entt) |= (1 << Index);
             }
         }
 
@@ -212,8 +215,11 @@ class basic_observer {
         template<std::size_t Index>
         static void maybe_valid_if(basic_observer &obs, const Entity entt, const basic_registry<Entity> &reg) {
             if(reg.template has<AllOf..., Require...>(entt) && !(reg.template has<NoneOf>(entt) || ...) && !(reg.template has<Reject>(entt) || ...)) {
-                auto *comp = obs.view.try_get(entt);
-                (comp ? *comp : obs.view.construct(entt)) |= (1 << Index);
+                if(auto *comp = obs.view.try_get(entt); !comp) {
+                    obs.view.construct(entt);
+                }
+
+                obs.view.get(entt) |= (1 << Index);
             }
         }
 

src/entt/entity/registry.hpp

@@ -68,7 +68,8 @@ class basic_registry {
 
         template<typename... Args>
         decltype(auto) assign(basic_registry &owner, const Entity entt, Args &&... args) {
-            construction.publish(entt, owner, this->construct(entt, std::forward<Args>(args)...));
+            this->construct(entt, std::forward<Args>(args)...);
+            construction.publish(entt, owner);
             return this->get(entt);
         }
 
@@ -76,8 +77,7 @@ class basic_registry {
         std::enable_if_t<std::is_same_v<typename std::iterator_traits<It>::value_type, Entity>, typename storage<Entity, Component>::reverse_iterator_type>
         assign(basic_registry &owner, It first, It last, Args &&... args) {
             auto it = this->construct(first, last, std::forward<Args>(args)...);
-            const auto end = it + (!construction.empty() * std::distance(first, last));
-            std::for_each(it, end, [this, &owner, &first](decltype(*it) component) { construction.publish(*(first++), owner, component); });
+            std::for_each(first, last, [this, &owner](const auto entt) { construction.publish(entt, owner); });
             return it;
         }
 
@@ -108,9 +108,9 @@ class basic_registry {
         }
 
     private:
-        sigh<void(const Entity, basic_registry &, decltype(std::declval<storage<Entity, Component>>().get({})))> construction{};
+        sigh<void(const Entity, basic_registry &)> construction{};
         sigh<void(const Entity, basic_registry &)> destruction{};
-        decltype(construction) update{};
+        sigh<void(const Entity, basic_registry &, decltype(std::declval<storage<Entity, Component>>().get({})))> update{};
     };
 
     struct pool_data {
@@ -1007,11 +1007,11 @@ public:
      * The function type for a listener is equivalent to:
      *
      * @code{.cpp}
-     * void(Entity, registry<Entity> &, Component &);
+     * void(Entity, registry<Entity> &);
      * @endcode
      *
      * Listeners are invoked **after** the component has been assigned to the
-     * entity. The order of invocation of the listeners isn't guaranteed.
+     * entity.
      *
      * @note
      * Empty types aren't explicitly instantiated. Therefore, temporary objects
@@ -1041,8 +1041,7 @@ public:
      * void(Entity, registry<Entity> &, Component &);
      * @endcode
      *
-     * Listeners are invoked **before** the component has been replaced. The
-     * order of invocation of the listeners isn't guaranteed.
+     * Listeners are invoked **before** the component has been replaced.
      *
      * @note
      * Empty types aren't explicitly instantiated. Therefore, temporary objects
@@ -1074,7 +1073,7 @@ public:
      * @endcode
      *
      * Listeners are invoked **before** the component has been removed from the
-     * entity. The order of invocation of the listeners isn't guaranteed.
+     * entity.
      *
      * @note
      * Empty types aren't explicitly instantiated. Therefore, temporary objects

src/entt/entity/storage.hpp

@@ -317,10 +317,9 @@ public:
      * @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) {
+    void construct(const entity_type entt, Args &&... args) {
         if constexpr(std::is_aggregate_v<object_type>) {
             instances.emplace_back(Type{std::forward<Args>(args)...});
         } else {
@@ -329,7 +328,6 @@ public:
 
         // entity goes after component in case constructor throws
         underlying_type::construct(entt);
-        return instances.back();
     }
 
     /**
@@ -674,14 +672,12 @@ public:
      * @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)...};
+    void construct(const entity_type entt, Args &&... args) {
+        [[maybe_unused]] object_type instance{std::forward<Args>(args)...};
         // entity goes after component in case constructor throws
         underlying_type::construct(entt);
-        return instance;
     }
 
     /**

test/entt/entity/storage.cpp

@@ -169,7 +169,8 @@ TEST(Storage, AggregatesMustWork) {
 TEST(Storage, TypesFromStandardTemplateLibraryMustWork) {
     // see #37 - this test shouldn't crash, that's all
     entt::storage<entt::entity, std::unordered_set<int>> pool;
-    pool.construct(entt::entity{0}).insert(42);
+    pool.construct(entt::entity{0});
+    pool.get(entt::entity{0}).insert(42);
     pool.destroy(entt::entity{0});
 }