From 36af39e2b4044be785a82b975e5672fb3983fc91 Mon Sep 17 00:00:00 2001 From: Michele Caini Date: Tue, 12 Apr 2022 11:15:22 +0200 Subject: [PATCH] emitter: full review --- TODO | 2 +- src/entt/signal/emitter.hpp | 306 ++++++----------------------- test/entt/signal/emitter.cpp | 124 ++++++------ test/lib/emitter/lib.cpp | 6 +- test/lib/emitter/main.cpp | 6 +- test/lib/emitter_plugin/main.cpp | 5 +- test/lib/emitter_plugin/plugin.cpp | 6 +- 7 files changed, 140 insertions(+), 315 deletions(-) diff --git a/TODO b/TODO index 45aa6c558..691e7e4a8 100644 --- a/TODO +++ b/TODO @@ -7,9 +7,9 @@ EXAMPLES * support to polymorphic types (see #859) WIP: +* emitter: runtime handlers, allocator support (ready for both already) * view/group: no storage_traits dependency -> use storage instead of components for the definition * resource::operator/>= -* simplify emitter (see uvw), runtime events * basic_storage::bind for cross-registry setups * uses-allocator construction: any (with allocator support), poly, ... * process scheduler: reviews, use free lists internally diff --git a/src/entt/signal/emitter.hpp b/src/entt/signal/emitter.hpp index fee36da62..70ba86b49 100644 --- a/src/entt/signal/emitter.hpp +++ b/src/entt/signal/emitter.hpp @@ -1,10 +1,7 @@ #ifndef ENTT_SIGNAL_EMITTER_HPP #define ENTT_SIGNAL_EMITTER_HPP -#include #include -#include -#include #include #include #include @@ -20,8 +17,7 @@ namespace entt { /** * @brief General purpose event emitter. * - * The emitter class template follows the CRTP idiom. To create a custom emitter - * type, derived classes must inherit directly from the base class as: + * To create an emitter type, derived classes must inherit from the base as: * * @code{.cpp} * struct my_emitter: emitter { @@ -29,273 +25,99 @@ namespace entt { * } * @endcode * - * Pools for the type of events are created internally on the fly. It's not - * required to specify in advance the full list of accepted types.
- * Moreover, whenever an event is published, an emitter provides the listeners - * with a reference to itself along with a reference to the event. Therefore - * listeners have an handy way to work with it without incurring in the need of - * capturing a reference to the emitter. + * Handlers for the different events are created internally on the fly. It's not + * required to specify in advance the full list of accepted events.
+ * Moreover, whenever an event is published, an emitter also passes a reference + * to itself to its listeners. * - * @tparam Derived Actual type of emitter that extends the class template. + * @tparam Derived Emitter type. */ template class emitter { - struct basic_pool { - virtual ~basic_pool() = default; - virtual bool empty() const ENTT_NOEXCEPT = 0; - virtual void clear() ENTT_NOEXCEPT = 0; - }; + template + using function_type = std::function; - template - struct pool_handler final: basic_pool { - static_assert(std::is_same_v>, "Invalid event type"); + template + [[nodiscard]] function_type &assure() { + static_assert(std::is_same_v>, "Non-decayed types not allowed"); + auto &&ptr = handlers[type_hash::value()]; - using listener_type = std::function; - using element_type = std::pair; - using container_type = std::list; - using connection_type = typename container_type::iterator; - - [[nodiscard]] bool empty() const ENTT_NOEXCEPT override { - auto pred = [](auto &&element) { return element.first; }; - - return std::all_of(once_list.cbegin(), once_list.cend(), pred) - && std::all_of(on_list.cbegin(), on_list.cend(), pred); + if(!ptr) { + ptr = std::make_shared>(); } - void clear() ENTT_NOEXCEPT override { - if(publishing) { - for(auto &&element: once_list) { - element.first = true; - } - - for(auto &&element: on_list) { - element.first = true; - } - } else { - once_list.clear(); - on_list.clear(); - } - } - - connection_type once(listener_type listener) { - return once_list.emplace(once_list.cend(), false, std::move(listener)); - } - - connection_type on(listener_type listener) { - return on_list.emplace(on_list.cend(), false, std::move(listener)); - } - - void erase(connection_type conn) { - conn->first = true; - - if(!publishing) { - auto pred = [](auto &&element) { return element.first; }; - once_list.remove_if(pred); - on_list.remove_if(pred); - } - } - - void publish(Event &event, Derived &ref) { - container_type swap_list; - once_list.swap(swap_list); - - publishing = true; - - for(auto &&element: on_list) { - element.first ? void() : element.second(event, ref); - } - - for(auto &&element: swap_list) { - element.first ? void() : element.second(event, ref); - } - - publishing = false; - - on_list.remove_if([](auto &&element) { return element.first; }); - } - - private: - bool publishing{false}; - container_type once_list{}; - container_type on_list{}; - }; - - template - [[nodiscard]] pool_handler *assure() { - if(auto &&ptr = pools[type_hash::value()]; !ptr) { - auto *cpool = new pool_handler{}; - ptr.reset(cpool); - return cpool; - } else { - return static_cast *>(ptr.get()); - } + return *static_cast *>(ptr.get()); } - template - [[nodiscard]] const pool_handler *assure() const { - const auto it = pools.find(type_hash::value()); - return (it == pools.cend()) ? nullptr : static_cast *>(it->second.get()); + template + [[nodiscard]] const function_type *assure() const { + const auto it = handlers.find(type_hash::value()); + return (it == handlers.cend()) ? nullptr : static_cast *>(it->second.get()); } public: - /** @brief Type of listeners accepted for the given event. */ - template - using listener = typename pool_handler::listener_type; - - /** - * @brief Generic connection type for events. - * - * Type of the connection object returned by the event emitter whenever a - * listener for the given type is registered.
- * It can be used to break connections still in use. - * - * @tparam Event Type of event for which the connection is created. - */ - template - struct connection: private pool_handler::connection_type { - /** @brief Event emitters are friend classes of connections. */ - friend class emitter; - - /*! @brief Default constructor. */ - connection() ENTT_NOEXCEPT = default; - - /** - * @brief Creates a connection that wraps its underlying instance. - * @param conn A connection object to wrap. - */ - connection(typename pool_handler::connection_type conn) - : pool_handler::connection_type{std::move(conn)} {} - }; - /*! @brief Default constructor. */ - emitter() = default; + emitter() + : handlers{} {} /*! @brief Default destructor. */ virtual ~emitter() ENTT_NOEXCEPT { - static_assert(std::is_base_of_v, Derived>, "Incorrect use of the class template"); + static_assert(std::is_base_of_v, Derived>, "Invalid emitter type"); } /*! @brief Default move constructor. */ emitter(emitter &&) = default; - /*! @brief Default move assignment operator. @return This emitter. */ + /** + * @brief Default move assignment operator. + * @return This emitter. + */ emitter &operator=(emitter &&) = default; /** - * @brief Emits the given event. - * - * All the listeners registered for the specific event type are invoked with - * the given event. The event type must either have a proper constructor for - * the arguments provided or be an aggregate type. - * - * @tparam Event Type of event to publish. - * @tparam Args Types of arguments to use to construct the event. - * @param args Parameters to use to initialize the event. + * @brief Publishes a given event. + * @tparam Type Type of event to trigger. + * @param value An instance of the given type of event. */ - template - void publish(Args &&...args) { - Event instance{std::forward(args)...}; - assure()->publish(instance, *static_cast(this)); - } - - /** - * @brief Registers a long-lived listener with the event emitter. - * - * This method can be used to register a listener designed to be invoked - * more than once for the given event type.
- * The connection returned by the method can be freely discarded. It's meant - * to be used later to disconnect the listener if required. - * - * The listener is as a callable object that can be moved and the type of - * which is _compatible_ with `void(Event &, Derived &)`. - * - * @note - * Whenever an event is emitted, the emitter provides the listener with a - * reference to the derived class. Listeners don't have to capture those - * instances for later uses. - * - * @tparam Event Type of event to which to connect the listener. - * @param instance The listener to register. - * @return Connection object that can be used to disconnect the listener. - */ - template - connection on(listener instance) { - return assure()->on(std::move(instance)); - } - - /** - * @brief Registers a short-lived listener with the event emitter. - * - * This method can be used to register a listener designed to be invoked - * only once for the given event type.
- * The connection returned by the method can be freely discarded. It's meant - * to be used later to disconnect the listener if required. - * - * The listener is as a callable object that can be moved and the type of - * which is _compatible_ with `void(Event &, Derived &)`. - * - * @note - * Whenever an event is emitted, the emitter provides the listener with a - * reference to the derived class. Listeners don't have to capture those - * instances for later uses. - * - * @tparam Event Type of event to which to connect the listener. - * @param instance The listener to register. - * @return Connection object that can be used to disconnect the listener. - */ - template - connection once(listener instance) { - return assure()->once(std::move(instance)); - } - - /** - * @brief Disconnects a listener from the event emitter. - * - * Do not use twice the same connection to disconnect a listener, it results - * in undefined behavior. Once used, discard the connection object. - * - * @tparam Event Type of event of the connection. - * @param conn A valid connection. - */ - template - void erase(connection conn) { - assure()->erase(std::move(conn)); - } - - /** - * @brief Disconnects all the listeners for the given event type. - * - * All the connections previously returned for the given event are - * invalidated. Using them results in undefined behavior. - * - * @tparam Event Type of event to reset. - */ - template - void clear() { - assure()->clear(); - } - - /** - * @brief Disconnects all the listeners. - * - * All the connections previously returned are invalidated. Using them - * results in undefined behavior. - */ - void clear() ENTT_NOEXCEPT { - for(auto &&cpool: pools) { - cpool.second->clear(); + template + void publish(Type &&value) { + if(auto &handler = assure>>(); handler) { + handler(value, *static_cast(this)); } } + /** + * @brief Registers a listener with the event emitter. + * @tparam Type Type of event to which to connect the listener. + * @param func The listener to register. + */ + template + void on(std::function func) { + assure() = std::move(func); + } + + /** + * @brief Disconnects a listener from the event emitter. + * @tparam Type Type of event of the listener. + */ + template + void erase() { + handlers.erase(type_hash>>::value()); + } + + /*! @brief Disconnects all the listeners. */ + void clear() ENTT_NOEXCEPT { + handlers.clear(); + } + /** * @brief Checks if there are listeners registered for the specific event. - * @tparam Event Type of event to test. + * @tparam Type Type of event to test. * @return True if there are no listeners registered, false otherwise. */ - template - [[nodiscard]] bool empty() const { - const auto *cpool = assure(); - return !cpool || cpool->empty(); + template + [[nodiscard]] bool contains() const { + return handlers.contains(type_hash>>::value()); } /** @@ -303,13 +125,11 @@ public: * @return True if there are no listeners registered, false otherwise. */ [[nodiscard]] bool empty() const ENTT_NOEXCEPT { - return std::all_of(pools.cbegin(), pools.cend(), [](auto &&cpool) { - return cpool.second->empty(); - }); + return handlers.empty(); } private: - dense_map, identity> pools{}; + dense_map, identity> handlers{}; }; } // namespace entt diff --git a/test/entt/signal/emitter.cpp b/test/entt/signal/emitter.cpp index a944273ee..0568865f8 100644 --- a/test/entt/signal/emitter.cpp +++ b/test/entt/signal/emitter.cpp @@ -1,3 +1,5 @@ +#include +#include #include #include @@ -11,123 +13,119 @@ struct foo_event { struct bar_event {}; struct quux_event {}; +TEST(Emitter, Move) { + test_emitter emitter; + emitter.on([](auto &, const auto &) {}); + + ASSERT_FALSE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); + + test_emitter other{std::move(emitter)}; + + ASSERT_FALSE(other.empty()); + ASSERT_TRUE(other.contains()); + ASSERT_TRUE(emitter.empty()); + + emitter = std::move(other); + + ASSERT_FALSE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); + ASSERT_TRUE(other.empty()); +} + TEST(Emitter, Clear) { test_emitter emitter; ASSERT_TRUE(emitter.empty()); emitter.on([](auto &, const auto &) {}); - emitter.once([](const auto &, const auto &) {}); + emitter.on([](const auto &, const auto &) {}); ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); + ASSERT_TRUE(emitter.contains()); + ASSERT_FALSE(emitter.contains()); - emitter.clear(); + emitter.erase(); ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); + ASSERT_TRUE(emitter.contains()); + ASSERT_FALSE(emitter.contains()); - emitter.clear(); + emitter.erase(); ASSERT_FALSE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); + ASSERT_FALSE(emitter.contains()); + ASSERT_TRUE(emitter.contains()); + ASSERT_FALSE(emitter.contains()); emitter.on([](auto &, const auto &) {}); emitter.on([](const auto &, const auto &) {}); ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); + ASSERT_TRUE(emitter.contains()); + ASSERT_TRUE(emitter.contains()); emitter.clear(); ASSERT_TRUE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); + ASSERT_FALSE(emitter.contains()); + ASSERT_FALSE(emitter.contains()); } -TEST(Emitter, ClearPublishing) { +TEST(Emitter, ClearFromCallback) { test_emitter emitter; ASSERT_TRUE(emitter.empty()); - emitter.once([](auto &, auto &em) { - em.template once([](auto &, auto &) {}); - em.template clear(); + emitter.on([](auto &, auto &owner) { + owner.template on([](auto &, auto &) {}); + owner.template erase(); }); - emitter.on([](const auto &, auto &em) { - em.template once([](const auto &, auto &) {}); - em.template clear(); + emitter.on([](const auto &, auto &owner) { + owner.template on([](const auto &, auto &) {}); + owner.template erase(); }); ASSERT_FALSE(emitter.empty()); - emitter.publish(); - emitter.publish(); + emitter.publish(foo_event{}); + emitter.publish(bar_event{}); ASSERT_TRUE(emitter.empty()); } TEST(Emitter, On) { test_emitter emitter; + int value{}; - emitter.on([](auto &, const auto &) {}); + emitter.on([&value](auto &event, const auto &) { + value = event.i; + }); ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); + ASSERT_EQ(value, 0); - emitter.publish(0, 'c'); + emitter.publish(foo_event{42, 'c'}); - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); -} - -TEST(Emitter, Once) { - test_emitter emitter; - - emitter.once([](const auto &, const auto &) {}); - - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - - emitter.publish(); - - ASSERT_TRUE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); -} - -TEST(Emitter, OnceAndErase) { - test_emitter emitter; - - auto conn = emitter.once([](auto &, const auto &) {}); - - ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); - - emitter.erase(conn); - - ASSERT_TRUE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); + ASSERT_EQ(value, 42); } TEST(Emitter, OnAndErase) { test_emitter emitter; + std::function func{}; - auto conn = emitter.on([](const auto &, const auto &) {}); + emitter.on(func); ASSERT_FALSE(emitter.empty()); - ASSERT_FALSE(emitter.empty()); + ASSERT_TRUE(emitter.contains()); - emitter.erase(conn); + emitter.erase(); ASSERT_TRUE(emitter.empty()); - ASSERT_TRUE(emitter.empty()); + ASSERT_FALSE(emitter.contains()); } diff --git a/test/lib/emitter/lib.cpp b/test/lib/emitter/lib.cpp index e9c4594dc..bda75e1de 100644 --- a/test/lib/emitter/lib.cpp +++ b/test/lib/emitter/lib.cpp @@ -3,7 +3,7 @@ #include "types.h" ENTT_API void emit(test_emitter &emitter) { - emitter.publish(); - emitter.publish(42); - emitter.publish(3); + emitter.publish(event{}); + emitter.publish(message{42}); + emitter.publish(message{3}); } diff --git a/test/lib/emitter/main.cpp b/test/lib/emitter/main.cpp index 9c879a0dd..b1b8b6800 100644 --- a/test/lib/emitter/main.cpp +++ b/test/lib/emitter/main.cpp @@ -11,7 +11,11 @@ TEST(Lib, Emitter) { ASSERT_EQ(value, 0); - emitter.once([&](message msg, test_emitter &) { value = msg.payload; }); + emitter.on([&](message msg, test_emitter &emitter) { + value = msg.payload; + emitter.erase(); + }); + emit(emitter); ASSERT_EQ(value, 42); diff --git a/test/lib/emitter_plugin/main.cpp b/test/lib/emitter_plugin/main.cpp index e6e915468..1797b7553 100644 --- a/test/lib/emitter_plugin/main.cpp +++ b/test/lib/emitter_plugin/main.cpp @@ -11,7 +11,10 @@ TEST(Lib, Emitter) { ASSERT_EQ(value, 0); - emitter.once([&](message msg, test_emitter &) { value = msg.payload; }); + emitter.on([&](message msg, test_emitter &emitter) { + value = msg.payload; + emitter.erase(); + }); cr_plugin ctx; cr_plugin_load(ctx, PLUGIN); diff --git a/test/lib/emitter_plugin/plugin.cpp b/test/lib/emitter_plugin/plugin.cpp index b5a46d65b..e9b9aa444 100644 --- a/test/lib/emitter_plugin/plugin.cpp +++ b/test/lib/emitter_plugin/plugin.cpp @@ -5,9 +5,9 @@ CR_EXPORT int cr_main(cr_plugin *ctx, cr_op operation) { switch(operation) { case CR_STEP: - static_cast(ctx->userdata)->publish(); - static_cast(ctx->userdata)->publish(42); - static_cast(ctx->userdata)->publish(3); + static_cast(ctx->userdata)->publish(event{}); + static_cast(ctx->userdata)->publish(message{42}); + static_cast(ctx->userdata)->publish(message{3}); break; case CR_CLOSE: case CR_LOAD: