Improve signal obj forwarding with move semantics (and performance)

3 files changed, 35 insertions, 30 deletions

diff --git a/engine/include/core/signal.hpp b/engine/include/core/signal.hpp
index f06a193..6026ce5 100644
--- a/engine/include/core/signal.hpp
+++ b/engine/include/core/signal.hpp
@@ -31,8 +31,8 @@ namespace flat::core
         class signal : public prioritized
         {
         public:
-            /// copyable
-            signal(const signal& other) = default;
+            /// non copyable
+            signal(const signal& other) = delete;
             /// movable
             signal(signal&& other) = default;
 
@@ -42,7 +42,7 @@ namespace flat::core
             /// not default constructible
             signal() = delete;
 
-            /// normal constructor
+            /// normal constructor, allowed only by core::signal
             signal(priority_t p = priority_t::none) : prioritized(p) {}
         };
     }
@@ -63,18 +63,18 @@ namespace flat::core
         //       is not the signature of the listener (so not yet)
         signal() = delete;
 
-        /// copyable
-        signal(const signal& other) = default;
+        /// non copyable, copying signal<...>::args is inefficient
+        signal(const signal& other) = delete;
 
         /// movable
         signal(signal&& other) = default;
 
-        /// normal constructor that copies arguments
+        /// normal (inefficient) constructor that copies arguments
         constexpr signal(const Args&... _args)
             : helper::signal(priority_t::none), args(_args...) {}
 
-        constexpr signal(priority_t p, const Args&... _args)
-            : helper::signal(p), args(_args...) {}
+        // constexpr signal(priority_t p, const Args&... _args)
+        //     : helper::signal(p), args(_args...) {}
 
         /// normal constructor that forwards arguments
         /// this optimizes rvalue initializations
@@ -83,10 +83,10 @@ namespace flat::core
               args(std::forward<Args>(_args)...)
         {}
 
-        constexpr signal(priority_t p, Args&&... _args)
-            : helper::signal(p),
-              args(std::forward<Args>(_args)...)
-        {}
+        // constexpr signal(priority_t p, Args&&... _args)
+        //     : helper::signal(p),
+        //       args(std::forward<Args>(_args)...)
+        // {}
     };
 
 
@@ -155,7 +155,7 @@ namespace flat::core
             }
 
             npdebug("invoked listener ", this, " with signal ", p);
-            std::apply(m_callback, p->args);
+            std::apply(m_callback, (p->args));
 
             return true;
         }
@@ -187,8 +187,9 @@ namespace flat::core
         template<typename R, typename ...Args>
         std::shared_ptr<listener<Args...>> _connect(std::function<R(Args...)>&& f)
         {
+            // construct a listener by forwarding f to listener's constructor
             auto lis_ptr = std::make_shared<listener<Args...>>(
-                listener<Args...>(f)
+                listener<Args...>(std::forward<decltype(f)>(f))
             );
 
             // insert pointer
@@ -219,10 +220,12 @@ namespace flat::core
 
         /// add a signal to the queue/stack of signals (m_signals)
         template<class ...Args> 
-        void emit(const signal<Args...>& sig)
+        void emit(signal<Args...>&& sig)
         {   
             // create a shared_ptr
-            auto p = std::make_shared<signal<Args...>>(sig)
+            auto p = std::make_shared<signal<Args...>>(
+                std::forward<signal<Args...>>(sig)
+            );
 
             npdebug("emitted signal ", p);
 
@@ -248,8 +251,9 @@ namespace flat::core
         std::shared_ptr<listener<Args...>> connect(R (*f)(Args...))
         {
             return _connect(static_cast<std::function<R(Args...)>>(
-                [f](Args ...args) constexpr -> R {
-                    return f((args)...);
+                // closure that forwards ...args to f
+                [f](Args&& ...args) constexpr -> R {
+                    return f(std::forward<Args>(args)...);
                 })
             );
         }
@@ -259,8 +263,9 @@ namespace flat::core
         std::shared_ptr<listener<Args...>> connect(R (T::*mf)(Args ...args), T* obj)
         {
             return _connect(static_cast<std::function<R(Args...)>>(
-                [mf, obj](Args ...args) constexpr -> R {
-                    return (obj->*mf)((args)...);
+                // closure that forwards ...args to mf called on obj
+                [mf, obj](Args&& ...args) constexpr -> R {
+                    return (obj->*mf)(std::forward<Args>(args)...);
                 })
             );
         }
diff --git a/engine/signal.cpp b/engine/signal.cpp
index 244e1b7..309407f 100644
--- a/engine/signal.cpp
+++ b/engine/signal.cpp
@@ -19,10 +19,10 @@ void channel::broadcast()
     npdebug("broadcasting signals from channel ", this);
     std::list<std::weak_ptr<helper::listener>> to_erase;
 
-    for (const auto& sig : m_signals) {
+    for (auto&& sig : m_signals) {
         bool invoked = false;
 
-        for (const auto& l : m_listeners) {
+        for (auto&& l : m_listeners) {
             if (std::shared_ptr<helper::listener> pt = l.lock()) {
                 if (pt->invoke(sig))
                     invoked = true;
@@ -38,7 +38,7 @@ void channel::broadcast()
     }
 
     // TODO: check if this works
-    for (const auto& l : to_erase) {
+    for (auto&& l : to_erase) {
         // because weak_ptr do not have operator==
         m_listeners.remove_if([&](const auto& v) {
             return std::owner_less<std::weak_ptr<helper::listener>>()(l, v);
diff --git a/test/signal_test.cpp b/test/signal_test.cpp
index a34ed21..aa6e177 100644
--- a/test/signal_test.cpp
+++ b/test/signal_test.cpp
@@ -42,19 +42,19 @@ private:
 public:
     test_emitter(channel& ch) : m_chan(ch) {}
 
-    void send_str(const std::string& msg) {
+    void send_str(std::string&& msg) {
         std::cout << "emitting signal with msg=" << msg << std::endl;
-        m_chan.emit(signal(msg));
+        m_chan.emit(signal(std::move(msg)));
     }
 
-    void send_num(int n) {
+    void send_num(int&& n) {
         std::cout << "emitting signal with n=" << n << std::endl;
-        m_chan.emit(signal(n));
+        m_chan.emit(signal(std::move(n)));
     }
 
-    void send_custom(const custom_type& c) {
+    void send_custom(custom_type&& c) {
         std::cout << "emitting custom_type" << std::endl;
-        m_chan.emit(signal(c));
+        m_chan.emit(signal(std::move(c)));
     }
 };
 
@@ -100,7 +100,7 @@ int main() {
     // test with a function
     auto fun_lis = chan.connect(got_signal);
     std::cout << "emitting signal with x=100, y=293.0" << std::endl;
-    chan.emit(signal(100, 293.0));
+    chan.emit(std::move(signal(100, 293.0)));
 
     // test with a closure
     // TODO: fix