emitter: full review

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<T>::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

src/entt/signal/emitter.hpp

@@ -1,10 +1,7 @@
 #ifndef ENTT_SIGNAL_EMITTER_HPP
 #define ENTT_SIGNAL_EMITTER_HPP
 
-#include <algorithm>
 #include <functional>
-#include <iterator>
-#include <list>
 #include <memory>
 #include <type_traits>
 #include <utility>
@@ -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<my_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.<br/>
- * 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.<br/>
+ * 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<typename Derived>
 class emitter {
-    struct basic_pool {
-        virtual ~basic_pool() = default;
-        virtual bool empty() const ENTT_NOEXCEPT = 0;
-        virtual void clear() ENTT_NOEXCEPT = 0;
-    };
+    template<typename Type>
+    using function_type = std::function<void(Type &, Derived &)>;
 
-    template<typename Event>
-    struct pool_handler final: basic_pool {
-        static_assert(std::is_same_v<Event, std::decay_t<Event>>, "Invalid event type");
+    template<typename Type>
+    [[nodiscard]] function_type<Type> &assure() {
+        static_assert(std::is_same_v<Type, std::decay_t<Type>>, "Non-decayed types not allowed");
+        auto &&ptr = handlers[type_hash<Type>::value()];
 
-        using listener_type = std::function<void(Event &, Derived &)>;
-        using element_type = std::pair<bool, listener_type>;
-        using container_type = std::list<element_type>;
-        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);
-        }
-
-        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; });
+        if(!ptr) {
+            ptr = std::make_shared<function_type<Type>>();
         }
 
-    private:
-        bool publishing{false};
-        container_type once_list{};
-        container_type on_list{};
-    };
-
-    template<typename Event>
-    [[nodiscard]] pool_handler<Event> *assure() {
-        if(auto &&ptr = pools[type_hash<Event>::value()]; !ptr) {
-            auto *cpool = new pool_handler<Event>{};
-            ptr.reset(cpool);
-            return cpool;
-        } else {
-            return static_cast<pool_handler<Event> *>(ptr.get());
-        }
+        return *static_cast<function_type<Type> *>(ptr.get());
     }
 
-    template<typename Event>
-    [[nodiscard]] const pool_handler<Event> *assure() const {
-        const auto it = pools.find(type_hash<Event>::value());
-        return (it == pools.cend()) ? nullptr : static_cast<const pool_handler<Event> *>(it->second.get());
+    template<typename Type>
+    [[nodiscard]] const function_type<Type> *assure() const {
+        const auto it = handlers.find(type_hash<Type>::value());
+        return (it == handlers.cend()) ? nullptr : static_cast<const function_type<Type> *>(it->second.get());
     }
 
 public:
-    /** @brief Type of listeners accepted for the given event. */
-    template<typename Event>
-    using listener = typename pool_handler<Event>::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.<br/>
-     * It can be used to break connections still in use.
-     *
-     * @tparam Event Type of event for which the connection is created.
-     */
-    template<typename Event>
-    struct connection: private pool_handler<Event>::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<Event>::connection_type conn)
-            : pool_handler<Event>::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<emitter<Derived>, Derived>, "Incorrect use of the class template");
+        static_assert(std::is_base_of_v<emitter<Derived>, Derived>, "Invalid emitter type");
     }
 
     /*! @brief Default move constructor. */
     emitter(emitter &&) = default;
 
-    /*! @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 Default move assignment operator.
+     * @return This emitter.
      */
-    template<typename Event, typename... Args>
-    void publish(Args &&...args) {
-        Event instance{std::forward<Args>(args)...};
-        assure<Event>()->publish(instance, *static_cast<Derived *>(this));
-    }
+    emitter &operator=(emitter &&) = default;
 
     /**
-     * @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.<br/>
-     * 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.
+     * @brief Publishes a given event.
+     * @tparam Type Type of event to trigger.
+     * @param value An instance of the given type of event.
      */
-    template<typename Event>
-    connection<Event> on(listener<Event> instance) {
-        return assure<Event>()->on(std::move(instance));
+    template<typename Type>
+    void publish(Type &&value) {
+        if(auto &handler = assure<std::remove_const_t<std::remove_reference_t<Type>>>(); handler) {
+            handler(value, *static_cast<Derived *>(this));
+        }
     }
 
     /**
-     * @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.<br/>
-     * 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.
+     * @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<typename Event>
-    connection<Event> once(listener<Event> instance) {
-        return assure<Event>()->once(std::move(instance));
+    template<typename Type>
+    void on(std::function<void(Type &, Derived &)> func) {
+        assure<Type>() = std::move(func);
     }
 
     /**
      * @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.
+     * @tparam Type Type of event of the listener.
      */
-    template<typename Event>
-    void erase(connection<Event> conn) {
-        assure<Event>()->erase(std::move(conn));
+    template<typename Type>
+    void erase() {
+        handlers.erase(type_hash<std::remove_const_t<std::remove_reference_t<Type>>>::value());
     }
 
-    /**
-     * @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<typename Event>
-    void clear() {
-        assure<Event>()->clear();
-    }
-
-    /**
-     * @brief Disconnects all the listeners.
-     *
-     * All the connections previously returned are invalidated. Using them
-     * results in undefined behavior.
-     */
+    /*! @brief Disconnects all the listeners. */
     void clear() ENTT_NOEXCEPT {
-        for(auto &&cpool: pools) {
-            cpool.second->clear();
-        }
+        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<typename Event>
-    [[nodiscard]] bool empty() const {
-        const auto *cpool = assure<Event>();
-        return !cpool || cpool->empty();
+    template<typename Type>
+    [[nodiscard]] bool contains() const {
+        return handlers.contains(type_hash<std::remove_const_t<std::remove_reference_t<Type>>>::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<id_type, std::unique_ptr<basic_pool>, identity> pools{};
+    dense_map<id_type, std::shared_ptr<void>, identity> handlers{};
 };
 
 } // namespace entt

test/entt/signal/emitter.cpp

@@ -1,3 +1,5 @@
+#include <functional>
+#include <utility>
 #include <gtest/gtest.h>
 #include <entt/signal/emitter.hpp>
 
@@ -11,123 +13,119 @@ struct foo_event {
 struct bar_event {};
 struct quux_event {};
 
+TEST(Emitter, Move) {
+    test_emitter emitter;
+    emitter.on<foo_event>([](auto &, const auto &) {});
+
+    ASSERT_FALSE(emitter.empty());
+    ASSERT_TRUE(emitter.contains<foo_event>());
+
+    test_emitter other{std::move(emitter)};
+
+    ASSERT_FALSE(other.empty());
+    ASSERT_TRUE(other.contains<foo_event>());
+    ASSERT_TRUE(emitter.empty());
+
+    emitter = std::move(other);
+
+    ASSERT_FALSE(emitter.empty());
+    ASSERT_TRUE(emitter.contains<foo_event>());
+    ASSERT_TRUE(other.empty());
+}
+
 TEST(Emitter, Clear) {
     test_emitter emitter;
 
     ASSERT_TRUE(emitter.empty());
 
     emitter.on<foo_event>([](auto &, const auto &) {});
-    emitter.once<quux_event>([](const auto &, const auto &) {});
+    emitter.on<quux_event>([](const auto &, const auto &) {});
 
     ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<foo_event>());
-    ASSERT_FALSE(emitter.empty<quux_event>());
-    ASSERT_TRUE(emitter.empty<bar_event>());
+    ASSERT_TRUE(emitter.contains<foo_event>());
+    ASSERT_TRUE(emitter.contains<quux_event>());
+    ASSERT_FALSE(emitter.contains<bar_event>());
 
-    emitter.clear<bar_event>();
+    emitter.erase<bar_event>();
 
     ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<foo_event>());
-    ASSERT_FALSE(emitter.empty<quux_event>());
-    ASSERT_TRUE(emitter.empty<bar_event>());
+    ASSERT_TRUE(emitter.contains<foo_event>());
+    ASSERT_TRUE(emitter.contains<quux_event>());
+    ASSERT_FALSE(emitter.contains<bar_event>());
 
-    emitter.clear<foo_event>();
+    emitter.erase<foo_event>();
 
     ASSERT_FALSE(emitter.empty());
-    ASSERT_TRUE(emitter.empty<foo_event>());
-    ASSERT_FALSE(emitter.empty<quux_event>());
-    ASSERT_TRUE(emitter.empty<bar_event>());
+    ASSERT_FALSE(emitter.contains<foo_event>());
+    ASSERT_TRUE(emitter.contains<quux_event>());
+    ASSERT_FALSE(emitter.contains<bar_event>());
 
     emitter.on<foo_event>([](auto &, const auto &) {});
     emitter.on<bar_event>([](const auto &, const auto &) {});
 
     ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<foo_event>());
-    ASSERT_FALSE(emitter.empty<quux_event>());
-    ASSERT_FALSE(emitter.empty<bar_event>());
+    ASSERT_TRUE(emitter.contains<foo_event>());
+    ASSERT_TRUE(emitter.contains<quux_event>());
+    ASSERT_TRUE(emitter.contains<bar_event>());
 
     emitter.clear();
 
     ASSERT_TRUE(emitter.empty());
-    ASSERT_TRUE(emitter.empty<foo_event>());
-    ASSERT_TRUE(emitter.empty<bar_event>());
+    ASSERT_FALSE(emitter.contains<foo_event>());
+    ASSERT_FALSE(emitter.contains<bar_event>());
 }
 
-TEST(Emitter, ClearPublishing) {
+TEST(Emitter, ClearFromCallback) {
     test_emitter emitter;
 
     ASSERT_TRUE(emitter.empty());
 
-    emitter.once<foo_event>([](auto &, auto &em) {
-        em.template once<foo_event>([](auto &, auto &) {});
-        em.template clear<foo_event>();
+    emitter.on<foo_event>([](auto &, auto &owner) {
+        owner.template on<foo_event>([](auto &, auto &) {});
+        owner.template erase<foo_event>();
     });
 
-    emitter.on<bar_event>([](const auto &, auto &em) {
-        em.template once<bar_event>([](const auto &, auto &) {});
-        em.template clear<bar_event>();
+    emitter.on<bar_event>([](const auto &, auto &owner) {
+        owner.template on<bar_event>([](const auto &, auto &) {});
+        owner.template erase<bar_event>();
     });
 
     ASSERT_FALSE(emitter.empty());
 
-    emitter.publish<foo_event>();
-    emitter.publish<bar_event>();
+    emitter.publish(foo_event{});
+    emitter.publish(bar_event{});
 
     ASSERT_TRUE(emitter.empty());
 }
 
 TEST(Emitter, On) {
     test_emitter emitter;
+    int value{};
 
-    emitter.on<foo_event>([](auto &, const auto &) {});
-
-    ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<foo_event>());
-
-    emitter.publish<foo_event>(0, 'c');
-
-    ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<foo_event>());
-}
-
-TEST(Emitter, Once) {
-    test_emitter emitter;
-
-    emitter.once<bar_event>([](const auto &, const auto &) {});
-
-    ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<bar_event>());
-
-    emitter.publish<bar_event>();
-
-    ASSERT_TRUE(emitter.empty());
-    ASSERT_TRUE(emitter.empty<bar_event>());
-}
-
-TEST(Emitter, OnceAndErase) {
-    test_emitter emitter;
-
-    auto conn = emitter.once<foo_event>([](auto &, const auto &) {});
+    emitter.on<foo_event>([&value](auto &event, const auto &) {
+        value = event.i;
+    });
 
     ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<foo_event>());
+    ASSERT_TRUE(emitter.contains<foo_event>());
+    ASSERT_EQ(value, 0);
 
-    emitter.erase(conn);
+    emitter.publish(foo_event{42, 'c'});
 
-    ASSERT_TRUE(emitter.empty());
-    ASSERT_TRUE(emitter.empty<foo_event>());
+    ASSERT_EQ(value, 42);
 }
 
 TEST(Emitter, OnAndErase) {
     test_emitter emitter;
+    std::function<void(bar_event &, test_emitter &)> func{};
 
-    auto conn = emitter.on<bar_event>([](const auto &, const auto &) {});
+    emitter.on(func);
 
     ASSERT_FALSE(emitter.empty());
-    ASSERT_FALSE(emitter.empty<bar_event>());
+    ASSERT_TRUE(emitter.contains<bar_event>());
 
-    emitter.erase(conn);
+    emitter.erase<bar_event>();
 
     ASSERT_TRUE(emitter.empty());
-    ASSERT_TRUE(emitter.empty<bar_event>());
+    ASSERT_FALSE(emitter.contains<bar_event>());
 }

test/lib/emitter/lib.cpp

@@ -3,7 +3,7 @@
 #include "types.h"
 
 ENTT_API void emit(test_emitter &emitter) {
-    emitter.publish<event>();
-    emitter.publish<message>(42);
-    emitter.publish<message>(3);
+    emitter.publish(event{});
+    emitter.publish(message{42});
+    emitter.publish(message{3});
 }

test/lib/emitter/main.cpp

@@ -11,7 +11,11 @@ TEST(Lib, Emitter) {
 
     ASSERT_EQ(value, 0);
 
-    emitter.once<message>([&](message msg, test_emitter &) { value = msg.payload; });
+    emitter.on<message>([&](message msg, test_emitter &emitter) {
+        value = msg.payload;
+        emitter.erase<message>();
+    });
+
     emit(emitter);
 
     ASSERT_EQ(value, 42);

test/lib/emitter_plugin/main.cpp

@@ -11,7 +11,10 @@ TEST(Lib, Emitter) {
 
     ASSERT_EQ(value, 0);
 
-    emitter.once<message>([&](message msg, test_emitter &) { value = msg.payload; });
+    emitter.on<message>([&](message msg, test_emitter &emitter) {
+        value = msg.payload;
+        emitter.erase<message>();
+    });
 
     cr_plugin ctx;
     cr_plugin_load(ctx, PLUGIN);

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<test_emitter *>(ctx->userdata)->publish<event>();
-        static_cast<test_emitter *>(ctx->userdata)->publish<message>(42);
-        static_cast<test_emitter *>(ctx->userdata)->publish<message>(3);
+        static_cast<test_emitter *>(ctx->userdata)->publish(event{});
+        static_cast<test_emitter *>(ctx->userdata)->publish(message{42});
+        static_cast<test_emitter *>(ctx->userdata)->publish(message{3});
         break;
     case CR_CLOSE:
     case CR_LOAD: