From e7403d85516d07ce651708433e8f57134a85667f Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Sat, 15 Feb 2020 15:54:47 +0100 Subject: [PATCH] registry: in-place ::replace, the signal no longer receives an extra argument (close #406) --- docs/md/entity.md | 51 +++++++++------------------------ src/entt/entity/registry.hpp | 53 +++++++++++++++++++++++++++-------- test/entt/entity/registry.cpp | 8 +++--- 3 files changed, 60 insertions(+), 52 deletions(-) diff --git a/docs/md/entity.md b/docs/md/entity.md index 960e907b2..ff8130be1 100644 --- a/docs/md/entity.md +++ b/docs/md/entity.md @@ -228,16 +228,10 @@ This function is overloaded and accepts also a couple of iterators in order to: ``` If an entity already has the given component, the `replace` member function -template can be used to replace it: +template can be used to edit it in-place: ```cpp -registry.replace(entity, 0., 0.); - -// ... - -auto &velocity = registry.replace(entity); -vel.dx = 0.; -vel.dy = 0.; +registry.replace(entity, [](auto &pos) { pos.x = pos.y = 0.; }); ``` When it's unknown whether an entity already owns an instance of a component, @@ -245,19 +239,13 @@ When it's unknown whether an entity already owns an instance of a component, ```cpp registry.assign_or_replace(entity, 0., 0.); - -// ... - -auto &velocity = registry.assign_or_replace(entity); -vel.dx = 0.; -vel.dy = 0.; ``` This is a slightly faster alternative for the following snippet: ```cpp if(registry.has(entity)) { - registry.replace(entity, 0., 0.); + registry.replace(entity, [](auto &vel) { vel.dx = vel.dy = 0.; }); } else { registry.assign(entity, 0., 0.); } @@ -352,36 +340,24 @@ 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 and destruction signals -should be equivalent to the following: +The function type of a listener should be equivalent to the following: ```cpp void(entt::registry &, entt::entity); ``` -In both cases, listeners are provided with the registry that triggered the -notification and the entity affected by the change.
-The function type of a listener that observes changes to components is slightly -different instead: - -```cpp -void(entt::registry &, entt::entity, Component &); -``` - -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. +In all cases, listeners are provided with the registry that triggered the +notification and the involved entity. Note also that: -* Listeners for the construction signal are invoked **after** components have +* Listeners for the construction signals are invoked **after** components have been assigned to entities. -* Listeners designed to observe changes are invoked **before** components have - been replaced and therefore before newly created instances have been assigned - to entities. +* Listeners designed to observe changes are invoked **after** components have + been updated. -* Listeners for the destruction signal are invoked **before** components have +* Listeners for the destruction signals are invoked **before** components have been removed from entities. There are also some limitations on what a listener can and cannot do: @@ -390,7 +366,7 @@ There are also some limitations on what a listener can and cannot do: listener should be avoided. It can lead to undefined behavior in some cases. * Removing the component from within the body of a listener that observes the - construction or replacement of instances of a given type isn't allowed. + construction or update of instances of a given type isn't allowed. * Assigning and removing components from within the body of a listener that observes the destruction of instances of a given type should be avoided. It @@ -646,8 +622,9 @@ A dependency can also be easily broken as follows: registry.on_construct().disconnect<&entt::registry::assign_or_replace>(); ``` -There are many other types of dependencies. In general, all functions that -accept an entity as the first argument are good candidates for this purpose. +There are many other types of dependencies. In general, most of the functions +that accept an entity as the first argument are good candidates for this +purpose. ### Tags diff --git a/src/entt/entity/registry.hpp b/src/entt/entity/registry.hpp index 1c1d7e041..b57d626d4 100644 --- a/src/entt/entity/registry.hpp +++ b/src/entt/entity/registry.hpp @@ -98,17 +98,16 @@ class basic_registry { } } - template - auto replace(basic_registry &owner, const Entity entt, Args &&... args) -> decltype(this->get(entt)) { - Component component{std::forward(args)...}; - update.publish(owner, entt, component); - return (this->get(entt) = std::move(component)); + template + void replace(basic_registry &owner, const Entity entt, Func &&... func) { + (std::forward(func)(this->get(entt)), ...); + update.publish(owner, entt); } private: sigh construction{}; sigh destruction{}; - sigh>().get({})))> update{}; + sigh update{}; }; struct pool_data { @@ -717,12 +716,39 @@ public: decltype(auto) assign_or_replace(const entity_type entity, Args &&... args) { ENTT_ASSERT(valid(entity)); auto &cpool = assure(); - return cpool.has(entity) ? cpool.replace(*this, entity, std::forward(args)...) : cpool.assign(*this, entity, std::forward(args)...); + + return cpool.has(entity) + ? (cpool.replace(*this, entity, [args = std::forward_as_tuple(std::forward(args)...)](auto &&component) { component = std::make_from_tuple(std::move(args)); }), cpool.get(entity)) + : cpool.assign(*this, entity, std::forward(args)...); } /** * @brief Replaces the given component for an entity. * + * The signature of the functions should be equivalent to the following: + * + * @code{.cpp} + * void(Component &); + * @endcode + * + * Temporary objects are returned for empty types though. Capture them by + * copy or by const reference if needed. + * + * @tparam Component Type of component to replace. + * @tparam Func Types of the function objects to invoke. + * @param entity A valid entity identifier. + * @param func Valid function objects. + */ + template + auto replace(const entity_type entity, Func &&... func) + -> decltype(std::enable_if_t(), (func(std::declval()), ...), void()) { + ENTT_ASSERT(valid(entity)); + assure().replace(*this, entity, std::forward(func)...); + } + + /** + * @copybrief replace + * * A new instance of the given component is created and initialized with the * arguments provided (the component must have a proper constructor or be of * aggregate type). Then the component is assigned to the given entity. @@ -738,12 +764,17 @@ public: * @tparam Args Types of arguments to use to construct the component. * @param entity A valid entity identifier. * @param args Parameters to use to initialize the component. - * @return A reference to the newly created component. + * @return A reference to the component being replaced. */ template - decltype(auto) replace(const entity_type entity, Args &&... args) { - ENTT_ASSERT(valid(entity)); - return assure().replace(*this, entity, std::forward(args)...); + [[deprecated("use in-place replace instead")]] + auto replace(const entity_type entity, Args &&... args) + -> decltype(Component{std::forward(args)...}, assure().get(entity)) { + replace(entity, [args = std::forward_as_tuple(std::forward(args)...)](auto &&component) { + component = std::make_from_tuple(std::move(args)); + }); + + return assure().get(entity); } /** diff --git a/test/entt/entity/registry.cpp b/test/entt/entity/registry.cpp index 97c9bca87..c9ebf95af 100644 --- a/test/entt/entity/registry.cpp +++ b/test/entt/entity/registry.cpp @@ -177,7 +177,7 @@ TEST(Registry, Functionalities) { ASSERT_NE(®istry.get(e0), ®istry.get(e2)); ASSERT_NE(®istry.get(e0), ®istry.get(e2)); - ASSERT_NO_THROW(registry.replace(e0, 0)); + ASSERT_NO_THROW(registry.replace(e0, [](auto &instance) { instance = 0; })); ASSERT_EQ(registry.get(e0), 0); ASSERT_NO_THROW(registry.assign_or_replace(e0, 1)); @@ -1431,8 +1431,8 @@ TEST(Registry, Stamp) { ASSERT_EQ(registry.get(entity), 3); ASSERT_EQ(registry.get(entity), 'c'); - registry.replace(prototype, 42); - registry.replace(prototype, 'a'); + registry.replace(prototype, [](auto &instance) { instance = 42; }); + registry.replace(prototype, [](auto &instance) { instance = 'a'; }); registry.stamp(entity, registry, prototype); ASSERT_EQ(registry.get(entity), 42); @@ -1454,7 +1454,7 @@ TEST(Registry, StampExclude) { ASSERT_FALSE(registry.has(entity)); ASSERT_EQ(registry.get(entity), 3); - registry.replace(prototype, 42); + registry.replace(prototype, [](auto &instance) { instance = 42; }); registry.stamp(entity, registry, prototype, entt::exclude); ASSERT_TRUE((registry.has(entity)));