From c396baf32490f294934321b46451209c285f3d21 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Thu, 21 Mar 2024 15:31:59 +0100 Subject: [PATCH 1/8] Add sigslot library --- include/scratchcpp/signal.h | 1723 +++++++++++++++++++++++++++++++++++ 1 file changed, 1723 insertions(+) create mode 100644 include/scratchcpp/signal.h diff --git a/include/scratchcpp/signal.h b/include/scratchcpp/signal.h new file mode 100644 index 00000000..08f0fa6f --- /dev/null +++ b/include/scratchcpp/signal.h @@ -0,0 +1,1723 @@ +#pragma once +#include +#include +#include +#include +#include +#include +#include +#include + +#if defined(__GXX_RTTI) || defined(__cpp_rtti) || defined(_CPPRTTI) +#define SIGSLOT_RTTI_ENABLED 1 +#include +#endif + +namespace libscratchcpp::sigslot +{ + +template +class signal_base; + +namespace detail +{ + +// Used to detect an object of observer type +struct observer_type +{ }; + +} // namespace detail + +namespace trait +{ + +/// represent a list of types +template +struct typelist +{ +}; + +/** + * Pointers that can be converted to a weak pointer concept for tracking + * purpose must implement the to_weak() function in order to make use of + * ADL to convert that type and make it usable + */ + +template +std::weak_ptr to_weak(std::weak_ptr w) +{ + return w; +} + +template +std::weak_ptr to_weak(std::shared_ptr s) +{ + return s; +} + +// tools +namespace detail +{ + +template +struct voider +{ + using type = void; +}; + +// void_t from c++17 +template +using void_t = typename detail::voider::type; + +template +struct has_call_operator : std::false_type +{ +}; + +template +struct has_call_operator::type::operator())>> : std::true_type +{ +}; + +template +struct is_callable : std::false_type +{ +}; + +template +struct is_callable, void_t()).*std::declval())(std::declval()...))>> : std::true_type +{ +}; + +template +struct is_callable, void_t()(std::declval()...))>> : std::true_type +{ +}; + +template +struct is_weak_ptr : std::false_type +{ +}; + +template +struct is_weak_ptr().expired()), decltype(std::declval().lock()), decltype(std::declval().reset())>> : std::true_type +{ +}; + +template +struct is_weak_ptr_compatible : std::false_type +{ +}; + +template +struct is_weak_ptr_compatible()))>> : is_weak_ptr()))> +{ +}; + +template +struct is_signal : std::false_type +{ +}; + +template +struct is_signal> : std::true_type +{ +}; + +} // namespace detail + +static constexpr bool with_rtti = +#ifdef SIGSLOT_RTTI_ENABLED + true; +#else + false; +#endif + +/// determine if a pointer is convertible into a "weak" pointer +template +constexpr bool is_weak_ptr_compatible_v = detail::is_weak_ptr_compatible>::value; + +/// determine if a type T (Callable or Pmf) is callable with supplied arguments +template +constexpr bool is_callable_v = detail::is_callable::value; + +template +constexpr bool is_weak_ptr_v = detail::is_weak_ptr::value; + +template +constexpr bool has_call_operator_v = detail::has_call_operator::value; + +template +constexpr bool is_pointer_v = std::is_pointer::value; + +template +constexpr bool is_func_v = std::is_function::value; + +template +constexpr bool is_pmf_v = std::is_member_function_pointer::value; + +template +constexpr bool is_observer_v = std::is_base_of>>::value; + +template +constexpr bool is_signal_v = detail::is_signal::value; + +} // namespace trait + +/** + * A group_id is used to identify a group of slots + */ +using group_id = std::int32_t; + +namespace detail +{ + +/** + * The following function_traits and object_pointer series of templates are + * used to circumvent the type-erasing that takes place in the slot_base + * implementations. They are used to compare the stored functions and objects + * with another one for disconnection purpose. + */ + +/* + * Function pointers and member function pointers size differ from compiler to + * compiler, and for virtual members compared to non virtual members. On some + * compilers, multiple inheritance has an impact too. Hence, we form an union + * big enough to store any kind of function pointer. + */ +namespace mock +{ + +struct a +{ + virtual ~a() = default; + void f(); + virtual void g(); + static void h(); +}; +struct b +{ + virtual ~b() = default; + void f(); + virtual void g(); +}; +struct c + : a + , b +{ + void f(); + void g() override; +}; +struct d : virtual a +{ + void g() override; +}; + +union fun_types +{ + decltype(&d::g) dm; + decltype(&c::g) mm; + decltype(&c::g) mvm; + decltype(&a::f) m; + decltype(&a::g) vm; + decltype(&a::h) s; + void (*f)(); + void *o; +}; + +} // namespace mock + +/* + * This struct is used to store function pointers. + * This is needed for slot disconnection by function pointer. + * It assumes the underlying implementation to be trivially copiable. + */ +struct func_ptr +{ + func_ptr() : + sz{ 0 } + { + std::uninitialized_fill(std::begin(data), std::end(data), '\0'); + } + + template + void store(const T &t) + { + const auto *b = reinterpret_cast(&t); + sz = sizeof(T); + std::memcpy(data, b, sz); + } + + template + const T *as() const + { + if (sizeof(T) != sz) { + return nullptr; + } + return reinterpret_cast(data); + } + + private: + alignas(sizeof(mock::fun_types)) char data[sizeof(mock::fun_types)]; + size_t sz; +}; + +template +struct function_traits +{ + static void ptr(const T & /*t*/, func_ptr & /*d*/) { } + + static bool eq(const T & /*t*/, const func_ptr & /*d*/) { return false; } + + static constexpr bool is_disconnectable = false; + static constexpr bool must_check_object = true; +}; + +template +struct function_traits>> +{ + static void ptr(T &t, func_ptr &d) { d.store(&t); } + + static bool eq(T &t, const func_ptr &d) + { + const auto *r = d.as(); + return r && *r == &t; + } + + static constexpr bool is_disconnectable = true; + static constexpr bool must_check_object = false; +}; + +template +struct function_traits>> +{ + static void ptr(T *t, func_ptr &d) { function_traits::ptr(*t, d); } + + static bool eq(T *t, const func_ptr &d) { return function_traits::eq(*t, d); } + + static constexpr bool is_disconnectable = true; + static constexpr bool must_check_object = false; +}; + +template +struct function_traits>> +{ + static void ptr(T t, func_ptr &d) { d.store(t); } + + static bool eq(T t, const func_ptr &d) + { + const auto *r = d.as(); + return r && *r == t; + } + + static constexpr bool is_disconnectable = trait::with_rtti; + static constexpr bool must_check_object = true; +}; + +// for function objects, the assumption is that we are looking for the call operator +template +struct function_traits>> +{ + using call_type = decltype(&std::remove_reference::type::operator()); + + static void ptr(const T & /*t*/, func_ptr &d) { function_traits::ptr(&T::operator(), d); } + + static bool eq(const T & /*t*/, const func_ptr &d) { return function_traits::eq(&T::operator(), d); } + + static constexpr bool is_disconnectable = function_traits::is_disconnectable; + static constexpr bool must_check_object = function_traits::must_check_object; +}; + +template +func_ptr get_function_ptr(const T &t) +{ + func_ptr d; + function_traits>::ptr(t, d); + return d; +} + +template +bool eq_function_ptr(const T &t, const func_ptr &d) +{ + return function_traits>::eq(t, d); +} + +/* + * obj_ptr is used to store a pointer to an object. + * The object_pointer traits are needed to handle trackable objects correctly, + * as they are likely to not be pointers. + */ +using obj_ptr = const void *; + +template +obj_ptr get_object_ptr(const T &t); + +template +struct object_pointer +{ + static obj_ptr get(const T &) { return nullptr; } +}; + +template +struct object_pointer>> +{ + static obj_ptr get(const T *t) { return reinterpret_cast(t); } +}; + +template +struct object_pointer>> +{ + static obj_ptr get(const T &t) + { + auto p = t.lock(); + return get_object_ptr(p); + } +}; + +template +struct object_pointer && !trait::is_weak_ptr_v && trait::is_weak_ptr_compatible_v>> +{ + static obj_ptr get(const T &t) { return t ? reinterpret_cast(t.get()) : nullptr; } +}; + +template +obj_ptr get_object_ptr(const T &t) +{ + return object_pointer::get(t); +} + +// noop mutex for thread-unsafe use +struct null_mutex +{ + null_mutex() noexcept = default; + ~null_mutex() noexcept = default; + null_mutex(const null_mutex &) = delete; + null_mutex &operator=(const null_mutex &) = delete; + null_mutex(null_mutex &&) = delete; + null_mutex &operator=(null_mutex &&) = delete; + + inline bool try_lock() noexcept { return true; } + inline void lock() noexcept { } + inline void unlock() noexcept { } +}; + +/** + * A spin mutex that yields, mostly for use in benchmarks and scenarii that invoke + * slots at a very high pace. + * One should almost always prefer a standard mutex over this. + */ +struct spin_mutex +{ + spin_mutex() noexcept = default; + ~spin_mutex() noexcept = default; + spin_mutex(spin_mutex const &) = delete; + spin_mutex &operator=(const spin_mutex &) = delete; + spin_mutex(spin_mutex &&) = delete; + spin_mutex &operator=(spin_mutex &&) = delete; + + void lock() noexcept + { + while (true) { + while (!state.load(std::memory_order_relaxed)) { + std::this_thread::yield(); + } + + if (try_lock()) { + break; + } + } + } + + bool try_lock() noexcept { return state.exchange(false, std::memory_order_acquire); } + + void unlock() noexcept { state.store(true, std::memory_order_release); } + + private: + std::atomic state{ true }; +}; + +/** + * A simple copy on write container that will be used to improve slot lists + * access efficiency in a multithreaded context. + */ +template +class copy_on_write +{ + struct payload + { + payload() = default; + + template + explicit payload(Args &&...args) : + value(std::forward(args)...) + { + } + + std::atomic count{ 1 }; + T value; + }; + + public: + using element_type = T; + + copy_on_write() : + m_data(new payload) + { + } + + template + explicit copy_on_write(U &&x, std::enable_if_t, copy_on_write>::value> * = nullptr) : + m_data(new payload(std::forward(x))) + { + } + + copy_on_write(const copy_on_write &x) noexcept : + m_data(x.m_data) + { + ++m_data->count; + } + + copy_on_write(copy_on_write &&x) noexcept : + m_data(x.m_data) + { + x.m_data = nullptr; + } + + ~copy_on_write() + { + if (m_data && (--m_data->count == 0)) { + delete m_data; + } + } + + copy_on_write &operator=(const copy_on_write &x) noexcept + { + if (&x != this) { + *this = copy_on_write(x); + } + return *this; + } + + copy_on_write &operator=(copy_on_write &&x) noexcept + { + auto tmp = std::move(x); + swap(*this, tmp); + return *this; + } + + element_type &write() + { + if (!unique()) { + *this = copy_on_write(read()); + } + return m_data->value; + } + + const element_type &read() const noexcept { return m_data->value; } + + friend inline void swap(copy_on_write &x, copy_on_write &y) noexcept + { + using std::swap; + swap(x.m_data, y.m_data); + } + + private: + bool unique() const noexcept { return m_data->count == 1; } + + private: + payload *m_data; +}; + +/** + * Specializations for thread-safe code path + */ +template +const T &cow_read(const T &v) +{ + return v; +} + +template +const T &cow_read(copy_on_write &v) +{ + return v.read(); +} + +template +T &cow_write(T &v) +{ + return v; +} + +template +T &cow_write(copy_on_write &v) +{ + return v.write(); +} + +/** + * std::make_shared instantiates a lot a templates, and makes both compilation time + * and executable size far bigger than they need to be. We offer a make_shared + * equivalent that will avoid most instantiations with the following tradeoffs: + * - Not exception safe, + * - Allocates a separate control block, and will thus make the code slower. + */ +#ifdef SIGSLOT_REDUCE_COMPILE_TIME +template +inline std::shared_ptr make_shared(Arg &&...arg) +{ + return std::shared_ptr(static_cast(new D(std::forward(arg)...))); +} +#else +template +inline std::shared_ptr make_shared(Arg &&...arg) +{ + return std::static_pointer_cast(std::make_shared(std::forward(arg)...)); +} +#endif + +// Adapt a signal into a cheap function object, for easy signal chaining +template +struct signal_wrapper +{ + template + void operator()(U &&...u) + { + (*m_sig)(std::forward(u)...); + } + + SigT *m_sig{}; +}; + +/* slot_state holds slot type independent state, to be used to interact with + * slots indirectly through connection and scoped_connection objects. + */ +class slot_state +{ + public: + constexpr slot_state(group_id gid) noexcept : + m_index(0), + m_group(gid), + m_connected(true), + m_blocked(false) + { + } + + virtual ~slot_state() = default; + + virtual bool connected() const noexcept { return m_connected; } + + bool disconnect() noexcept + { + bool ret = m_connected.exchange(false); + if (ret) { + do_disconnect(); + } + return ret; + } + + bool blocked() const noexcept { return m_blocked.load(); } + void block() noexcept { m_blocked.store(true); } + void unblock() noexcept { m_blocked.store(false); } + + protected: + virtual void do_disconnect() { } + + auto index() const { return m_index; } + + auto &index() { return m_index; } + + group_id group() const { return m_group; } + + private: + template + friend class sigslot::signal_base; + + std::size_t m_index; // index into the array of slot pointers inside the signal + const group_id m_group; // slot group this slot belongs to + std::atomic m_connected; + std::atomic m_blocked; +}; + +} // namespace detail + +/** + * connection_blocker is a RAII object that blocks a connection until destruction + */ +class connection_blocker +{ + public: + connection_blocker() = default; + ~connection_blocker() noexcept { release(); } + + connection_blocker(const connection_blocker &) = delete; + connection_blocker &operator=(const connection_blocker &) = delete; + + connection_blocker(connection_blocker &&o) noexcept : + m_state{ std::move(o.m_state) } + { + } + + connection_blocker &operator=(connection_blocker &&o) noexcept + { + release(); + m_state.swap(o.m_state); + return *this; + } + + private: + friend class connection; + explicit connection_blocker(std::weak_ptr s) noexcept : + m_state{ std::move(s) } + { + if (auto d = m_state.lock()) { + d->block(); + } + } + + void release() noexcept + { + if (auto d = m_state.lock()) { + d->unblock(); + } + } + + private: + std::weak_ptr m_state; +}; + +/** + * A connection object allows interaction with an ongoing slot connection + * + * It allows common actions such as connection blocking and disconnection. + * Note that connection is not a RAII object, one does not need to hold one + * such object to keep the signal-slot connection alive. + */ +class connection +{ + public: + connection() = default; + virtual ~connection() = default; + + connection(const connection &) noexcept = default; + connection &operator=(const connection &) noexcept = default; + connection(connection &&) noexcept = default; + connection &operator=(connection &&) noexcept = default; + + bool valid() const noexcept { return !m_state.expired(); } + + bool connected() const noexcept + { + const auto d = m_state.lock(); + return d && d->connected(); + } + + bool disconnect() noexcept + { + auto d = m_state.lock(); + return d && d->disconnect(); + } + + bool blocked() const noexcept + { + const auto d = m_state.lock(); + return d && d->blocked(); + } + + void block() noexcept + { + if (auto d = m_state.lock()) { + d->block(); + } + } + + void unblock() noexcept + { + if (auto d = m_state.lock()) { + d->unblock(); + } + } + + connection_blocker blocker() const noexcept { return connection_blocker{ m_state }; } + + protected: + template + friend class signal_base; + explicit connection(std::weak_ptr s) noexcept : + m_state{ std::move(s) } + { + } + + protected: + std::weak_ptr m_state; +}; + +/** + * scoped_connection is a RAII version of connection + * It disconnects the slot from the signal upon destruction. + */ +class scoped_connection final : public connection +{ + public: + scoped_connection() = default; + ~scoped_connection() override { disconnect(); } + + /*implicit*/ scoped_connection(const connection &c) noexcept : + connection(c) + { + } + /*implicit*/ scoped_connection(connection &&c) noexcept : + connection(std::move(c)) + { + } + + scoped_connection(const scoped_connection &) noexcept = delete; + scoped_connection &operator=(const scoped_connection &) noexcept = delete; + + scoped_connection(scoped_connection &&o) noexcept : + connection{ std::move(o.m_state) } + { + } + + scoped_connection &operator=(scoped_connection &&o) noexcept + { + disconnect(); + m_state.swap(o.m_state); + return *this; + } + + private: + template + friend class signal_base; + explicit scoped_connection(std::weak_ptr s) noexcept : + connection{ std::move(s) } + { + } +}; + +/** + * Observer is a base class for intrusive lifetime tracking of objects. + * + * This is an alternative to trackable pointers, such as std::shared_ptr, + * and manual connection management by keeping connection objects in scope. + * Deriving from this class allows automatic disconnection of all the slots + * connected to any signal when an instance is destroyed. + */ +template +struct observer_base : private detail::observer_type +{ + virtual ~observer_base() = default; + + protected: + /** + * Disconnect all signals connected to this object. + * + * To avoid invocation of slots on a semi-destructed instance, which may happen + * in multi-threaded contexts, derived classes should call this method in their + * destructor. This will ensure proper disconnection prior to the destruction. + */ + void disconnect_all() + { + std::unique_lock _{ m_mutex }; + m_connections.clear(); + } + + private: + template + friend class signal_base; + + void add_connection(connection conn) + { + std::unique_lock _{ m_mutex }; + m_connections.emplace_back(std::move(conn)); + } + + Lockable m_mutex; + std::vector m_connections; +}; + +/** + * Specialization of observer_base to be used in single threaded contexts. + */ +using observer_st = observer_base; + +/** + * Specialization of observer_base to be used in multi-threaded contexts. + */ +using observer = observer_base; + +namespace detail +{ + +// interface for cleanable objects, used to cleanup disconnected slots +struct cleanable +{ + virtual ~cleanable() = default; + virtual void clean(slot_state *) = 0; +}; + +template +class slot_base; + +template +using slot_ptr = std::shared_ptr>; + +/* A base class for slot objects. This base type only depends on slot argument + * types, it will be used as an element in an intrusive singly-linked list of + * slots, hence the public next member. + */ +template +class slot_base : public slot_state +{ + public: + using base_types = trait::typelist; + + explicit slot_base(cleanable &c, group_id gid) : + slot_state(gid), + cleaner(c) + { + } + ~slot_base() override = default; + + // method effectively responsible for calling the "slot" function with + // supplied arguments whenever emission happens. + virtual void call_slot(Args...) = 0; + + template + void operator()(U &&...u) + { + if (slot_state::connected() && !slot_state::blocked()) { + call_slot(std::forward(u)...); + } + } + + // check if we are storing callable c + template + bool has_callable(const C &c) const + { + auto p = get_callable(); + return eq_function_ptr(c, p); + } + + template + std::enable_if_t::must_check_object, bool> has_full_callable(const C &c) const + { + return has_callable(c) && check_class_type>(); + } + + template + std::enable_if_t::must_check_object, bool> has_full_callable(const C &c) const + { + return has_callable(c); + } + + // check if we are storing object o + template + bool has_object(const O &o) const + { + return get_object() == get_object_ptr(o); + } + + protected: + void do_disconnect() final { cleaner.clean(this); } + + // retieve a pointer to the object embedded in the slot + virtual obj_ptr get_object() const noexcept { return nullptr; } + + // retieve a pointer to the callable embedded in the slot + virtual func_ptr get_callable() const noexcept { return get_function_ptr(nullptr); } + +#ifdef SIGSLOT_RTTI_ENABLED + // retieve a pointer to the callable embedded in the slot + virtual const std::type_info &get_callable_type() const noexcept + { + return typeid(nullptr); + } + + private: + template + bool check_class_type() const + { + return typeid(U) == get_callable_type(); + } + +#else + template + bool check_class_type() const + { + return false; + } +#endif + + private: + cleanable &cleaner; +}; + +/* + * A slot object holds state information, and a callable to to be called + * whenever the function call operator of its slot_base base class is called. + */ +template +class slot final : public slot_base +{ + public: + template + constexpr slot(cleanable &c, F &&f, Gid gid) : + slot_base(c, gid), + func{ std::forward(f) } + { + } + + protected: + void call_slot(Args... args) override { func(args...); } + + func_ptr get_callable() const noexcept override { return get_function_ptr(func); } + +#ifdef SIGSLOT_RTTI_ENABLED + const std::type_info &get_callable_type() const noexcept override + { + return typeid(func); + } +#endif + + private: + std::decay_t func; +}; + +/* + * Variation of slot that prepends a connection object to the callable + */ +template +class slot_extended final : public slot_base +{ + public: + template + constexpr slot_extended(cleanable &c, F &&f, group_id gid) : + slot_base(c, gid), + func{ std::forward(f) } + { + } + + connection conn; + + protected: + void call_slot(Args... args) override { func(conn, args...); } + + func_ptr get_callable() const noexcept override { return get_function_ptr(func); } + +#ifdef SIGSLOT_RTTI_ENABLED + const std::type_info &get_callable_type() const noexcept override + { + return typeid(func); + } +#endif + + private: + std::decay_t func; +}; + +/* + * A slot object holds state information, an object and a pointer over member + * function to be called whenever the function call operator of its slot_base + * base class is called. + */ +template +class slot_pmf final : public slot_base +{ + public: + template + constexpr slot_pmf(cleanable &c, F &&f, P &&p, group_id gid) : + slot_base(c, gid), + pmf{ std::forward(f) }, + ptr{ std::forward

(p) } + { + } + + protected: + void call_slot(Args... args) override { ((*ptr).*pmf)(args...); } + + func_ptr get_callable() const noexcept override { return get_function_ptr(pmf); } + + obj_ptr get_object() const noexcept override { return get_object_ptr(ptr); } + +#ifdef SIGSLOT_RTTI_ENABLED + const std::type_info &get_callable_type() const noexcept override + { + return typeid(pmf); + } +#endif + + private: + std::decay_t pmf; + std::decay_t ptr; +}; + +/* + * Variation of slot that prepends a connection object to the callable + */ +template +class slot_pmf_extended final : public slot_base +{ + public: + template + constexpr slot_pmf_extended(cleanable &c, F &&f, P &&p, group_id gid) : + slot_base(c, gid), + pmf{ std::forward(f) }, + ptr{ std::forward

(p) } + { + } + + connection conn; + + protected: + void call_slot(Args... args) override { ((*ptr).*pmf)(conn, args...); } + + func_ptr get_callable() const noexcept override { return get_function_ptr(pmf); } + obj_ptr get_object() const noexcept override { return get_object_ptr(ptr); } + +#ifdef SIGSLOT_RTTI_ENABLED + const std::type_info &get_callable_type() const noexcept override + { + return typeid(pmf); + } +#endif + + private: + std::decay_t pmf; + std::decay_t ptr; +}; + +/* + * An implementation of a slot that tracks the life of a supplied object + * through a weak pointer in order to automatically disconnect the slot + * on said object destruction. + */ +template +class slot_tracked final : public slot_base +{ + public: + template + constexpr slot_tracked(cleanable &c, F &&f, P &&p, group_id gid) : + slot_base(c, gid), + func{ std::forward(f) }, + ptr{ std::forward

(p) } + { + } + + bool connected() const noexcept override { return !ptr.expired() && slot_state::connected(); } + + protected: + void call_slot(Args... args) override + { + auto sp = ptr.lock(); + if (!sp) { + slot_state::disconnect(); + return; + } + if (slot_state::connected()) { + func(args...); + } + } + + func_ptr get_callable() const noexcept override { return get_function_ptr(func); } + + obj_ptr get_object() const noexcept override { return get_object_ptr(ptr); } + +#ifdef SIGSLOT_RTTI_ENABLED + const std::type_info &get_callable_type() const noexcept override + { + return typeid(func); + } +#endif + + private: + std::decay_t func; + std::decay_t ptr; +}; + +/* + * An implementation of a slot as a pointer over member function, that tracks + * the life of a supplied object through a weak pointer in order to automatically + * disconnect the slot on said object destruction. + */ +template +class slot_pmf_tracked final : public slot_base +{ + public: + template + constexpr slot_pmf_tracked(cleanable &c, F &&f, P &&p, group_id gid) : + slot_base(c, gid), + pmf{ std::forward(f) }, + ptr{ std::forward

(p) } + { + } + + bool connected() const noexcept override { return !ptr.expired() && slot_state::connected(); } + + protected: + void call_slot(Args... args) override + { + auto sp = ptr.lock(); + if (!sp) { + slot_state::disconnect(); + return; + } + if (slot_state::connected()) { + ((*sp).*pmf)(args...); + } + } + + func_ptr get_callable() const noexcept override { return get_function_ptr(pmf); } + + obj_ptr get_object() const noexcept override { return get_object_ptr(ptr); } + +#ifdef SIGSLOT_RTTI_ENABLED + const std::type_info &get_callable_type() const noexcept override + { + return typeid(pmf); + } +#endif + + private: + std::decay_t pmf; + std::decay_t ptr; +}; + +} // namespace detail + +/** + * signal_base is an implementation of the observer pattern, through the use + * of an emitting object and slots that are connected to the signal and called + * with supplied arguments when a signal is emitted. + * + * signal_base is the general implementation, whose locking policy must be + * set in order to decide thread safety guarantees. signal and signal_st + * are partial specializations for multi-threaded and single-threaded use. + * + * It does not allow slots to return a value. + * + * Slot execution order can be constrained by assigning group ids to the slots. + * The execution order of slots in a same group is unspecified and should not be + * relied upon, however groups are executed in ascending group ids order. When + * the group id of a slot is not set, it is assigned to the group 0. Group ids + * can have any value in the range of signed 32 bit integers. + * + * @tparam Lockable a lock type to decide the lock policy + * @tparam T... the argument types of the emitting and slots functions. + */ +template +class signal_base final : public detail::cleanable +{ + template + using is_thread_safe = std::integral_constant::value>; + + template + using cow_type = std::conditional_t::value, detail::copy_on_write, U>; + + template + using cow_copy_type = std::conditional_t::value, detail::copy_on_write, const U &>; + + using lock_type = std::unique_lock; + using slot_base = detail::slot_base; + using slot_ptr = detail::slot_ptr; + using slots_type = std::vector; + struct group_type + { + slots_type slts; + group_id gid; + }; + using list_type = std::vector; // kept ordered by ascending gid + + public: + using arg_list = trait::typelist; + using ext_arg_list = trait::typelist; + + signal_base() noexcept : + m_block(false) + { + } + ~signal_base() override { disconnect_all(); } + + signal_base(const signal_base &) = delete; + signal_base &operator=(const signal_base &) = delete; + + signal_base(signal_base &&o) /* not noexcept */ + : + m_block{ o.m_block.load() } + { + lock_type lock(o.m_mutex); + using std::swap; + swap(m_slots, o.m_slots); + } + + signal_base &operator=(signal_base &&o) /* not noexcept */ + { + lock_type lock1(m_mutex, std::defer_lock); + lock_type lock2(o.m_mutex, std::defer_lock); + std::lock(lock1, lock2); + + using std::swap; + swap(m_slots, o.m_slots); + m_block.store(o.m_block.exchange(m_block.load())); + return *this; + } + + /** + * Emit a signal + * + * Effect: All non blocked and connected slot functions will be called + * with supplied arguments. + * Safety: With proper locking (see pal::signal), emission can happen from + * multiple threads simultaneously. The guarantees only apply to the + * signal object, it does not cover thread safety of potentially + * shared state used in slot functions. + * + * @param a... arguments to emit + */ + template + void operator()(U &&...a) + { + if (m_block) { + return; + } + + // Reference to the slots to execute them out of the lock + // a copy may occur if another thread writes to it. + cow_copy_type ref = slots_reference(); + + for (const auto &group : detail::cow_read(ref)) { + for (const auto &s : group.slts) { + s->operator()(a...); + } + } + } + + /** + * Connect a callable of compatible arguments + * + * Effect: Creates and stores a new slot responsible for executing the + * supplied callable for every subsequent signal emission. + * Safety: Thread-safety depends on locking policy. + * + * @param c a callable + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t, connection> connect(Callable &&c, group_id gid = 0) + { + using slot_t = detail::slot; + auto s = make_slot(std::forward(c), gid); + connection conn(s); + add_slot(std::move(s)); + return conn; + } + + /** + * Connect a callable with an additional connection argument + * + * The callable's first argument must be of type connection. This overload + * the callable to manage it's own connection through this argument. + * + * @param c a callable + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t, connection> connect_extended(Callable &&c, group_id gid = 0) + { + using slot_t = detail::slot_extended; + auto s = make_slot(std::forward(c), gid); + connection conn(s); + std::static_pointer_cast(s)->conn = conn; + add_slot(std::move(s)); + return conn; + } + + /** + * Overload of connect for pointers over member functions derived from + * observer + * + * @param pmf a pointer over member function + * @param ptr an object pointer derived from observer + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t && trait::is_observer_v, connection> connect(Pmf &&pmf, Ptr &&ptr, group_id gid = 0) + { + using slot_t = detail::slot_pmf; + auto s = make_slot(std::forward(pmf), std::forward(ptr), gid); + connection conn(s); + add_slot(std::move(s)); + ptr->add_connection(conn); + return conn; + } + + /** + * Overload of connect for pointers over member functions + * + * @param pmf a pointer over member function + * @param ptr an object pointer + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t && !trait::is_observer_v && !trait::is_weak_ptr_compatible_v, connection> connect(Pmf &&pmf, Ptr &&ptr, group_id gid = 0) + { + using slot_t = detail::slot_pmf; + auto s = make_slot(std::forward(pmf), std::forward(ptr), gid); + connection conn(s); + add_slot(std::move(s)); + return conn; + } + + /** + * Overload of connect for pointer over member functions and + * + * @param pmf a pointer over member function + * @param ptr an object pointer + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t && !trait::is_weak_ptr_compatible_v, connection> connect_extended(Pmf &&pmf, Ptr &&ptr, group_id gid = 0) + { + using slot_t = detail::slot_pmf_extended; + auto s = make_slot(std::forward(pmf), std::forward(ptr), gid); + connection conn(s); + std::static_pointer_cast(s)->conn = conn; + add_slot(std::move(s)); + return conn; + } + + /** + * Overload of connect for lifetime object tracking and automatic disconnection + * + * Ptr must be convertible to an object following a loose form of weak pointer + * concept, by implementing the ADL-detected conversion function to_weak(). + * + * This overload covers the case of a pointer over member function and a + * trackable pointer of that class. + * + * Note: only weak references are stored, a slot does not extend the lifetime + * of a suppied object. + * + * @param pmf a pointer over member function + * @param ptr a trackable object pointer + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t && trait::is_weak_ptr_compatible_v, connection> connect(Pmf &&pmf, Ptr &&ptr, group_id gid = 0) + { + using trait::to_weak; + auto w = to_weak(std::forward(ptr)); + using slot_t = detail::slot_pmf_tracked; + auto s = make_slot(std::forward(pmf), w, gid); + connection conn(s); + add_slot(std::move(s)); + return conn; + } + + /** + * Overload of connect for lifetime object tracking and automatic disconnection + * + * Trackable must be convertible to an object following a loose form of weak + * pointer concept, by implementing the ADL-detected conversion function to_weak(). + * + * This overload covers the case of a standalone callable and unrelated trackable + * object. + * + * Note: only weak references are stored, a slot does not extend the lifetime + * of a suppied object. + * + * @param c a callable + * @param ptr a trackable object pointer + * @param gid an identifier that can be used to order slot execution + * @return a connection object that can be used to interact with the slot + */ + template + std::enable_if_t && trait::is_weak_ptr_compatible_v, connection> connect(Callable &&c, Trackable &&ptr, group_id gid = 0) + { + using trait::to_weak; + auto w = to_weak(std::forward(ptr)); + using slot_t = detail::slot_tracked; + auto s = make_slot(std::forward(c), w, gid); + connection conn(s); + add_slot(std::move(s)); + return conn; + } + + /** + * Creates a connection whose duration is tied to the return object + * Use the same semantics as connect + */ + template + scoped_connection connect_scoped(CallArgs &&...args) + { + return connect(std::forward(args)...); + } + + /** + * Disconnect slots bound to a callable + * + * Effect: Disconnects all the slots bound to the callable in argument. + * Safety: Thread-safety depends on locking policy. + * + * If the callable is a free or static member function, this overload is always + * available. However, RTTI is needed for it to work for pointer to member + * functions, function objects or and (references to) lambdas, because the + * C++ spec does not mandate the pointers to member functions to be unique. + * + * @param c a callable + * @return the number of disconnected slots + */ + template + std::enable_if_t< + (trait::is_callable_v || trait::is_callable_v || trait::is_pmf_v)&&detail::function_traits::is_disconnectable, + size_t> + disconnect(const Callable &c) + { + return disconnect_if([&](const auto &s) { return s->has_full_callable(c); }); + } + + /** + * Disconnect slots bound to this object + * + * Effect: Disconnects all the slots bound to the object or tracked object + * in argument. + * Safety: Thread-safety depends on locking policy. + * + * The object may be a pointer or trackable object. + * + * @param obj an object + * @return the number of disconnected slots + */ + template + std::enable_if_t && !trait::is_callable_v && !trait::is_pmf_v, size_t> disconnect(const Obj &obj) + { + return disconnect_if([&](const auto &s) { return s->has_object(obj); }); + } + + /** + * Disconnect slots bound both to a callable and object + * + * Effect: Disconnects all the slots bound to the callable and object in argument. + * Safety: Thread-safety depends on locking policy. + * + * For naked pointers, the Callable is expected to be a pointer over member + * function. If obj is trackable, any kind of Callable can be used. + * + * @param c a callable + * @param obj an object + * @return the number of disconnected slots + */ + template + size_t disconnect(const Callable &c, const Obj &obj) + { + return disconnect_if([&](const auto &s) { return s->has_object(obj) && s->has_callable(c); }); + } + + /** + * Disconnect slots in a particular group + * + * Effect: Disconnects all the slots in the group id in argument. + * Safety: Thread-safety depends on locking policy. + * + * @param gid a group id + * @return the number of disconnected slots + */ + size_t disconnect(group_id gid) + { + lock_type lock(m_mutex); + for (auto &group : detail::cow_write(m_slots)) { + if (group.gid == gid) { + size_t count = group.slts.size(); + group.slts.clear(); + return count; + } + } + return 0; + } + + /** + * Disconnects all the slots + * Safety: Thread safety depends on locking policy + */ + void disconnect_all() + { + lock_type lock(m_mutex); + clear(); + } + + /** + * Blocks signal emission + * Safety: thread safe + */ + void block() noexcept { m_block.store(true); } + + /** + * Unblocks signal emission + * Safety: thread safe + */ + void unblock() noexcept { m_block.store(false); } + + /** + * Tests blocking state of signal emission + */ + bool blocked() const noexcept { return m_block.load(); } + + /** + * Get number of connected slots + * Safety: thread safe + */ + size_t slot_count() noexcept + { + cow_copy_type ref = slots_reference(); + size_t count = 0; + for (const auto &g : detail::cow_read(ref)) { + count += g.slts.size(); + } + return count; + } + + protected: + /** + * remove disconnected slots + */ + void clean(detail::slot_state *state) override + { + lock_type lock(m_mutex); + const auto idx = state->index(); + const auto gid = state->group(); + + // find the group + for (auto &group : detail::cow_write(m_slots)) { + if (group.gid == gid) { + auto &slts = group.slts; + + // ensure we have the right slot, in case of concurrent cleaning + if (idx < slts.size() && slts[idx] && slts[idx].get() == state) { + std::swap(slts[idx], slts.back()); + slts[idx]->index() = idx; + slts.pop_back(); + } + + return; + } + } + } + + private: + // used to get a reference to the slots for reading + inline cow_copy_type slots_reference() + { + lock_type lock(m_mutex); + return m_slots; + } + + // create a new slot + template + inline auto make_slot(A &&...a) + { + return detail::make_shared(*this, std::forward(a)...); + } + + // add the slot to the list of slots of the right group + void add_slot(slot_ptr &&s) + { + const group_id gid = s->group(); + + lock_type lock(m_mutex); + auto &groups = detail::cow_write(m_slots); + + // find the group + auto it = groups.begin(); + while (it != groups.end() && it->gid < gid) { + it++; + } + + // create a new group if necessary + if (it == groups.end() || it->gid != gid) { + it = groups.insert(it, { {}, gid }); + } + + // add the slot + s->index() = it->slts.size(); + it->slts.push_back(std::move(s)); + } + + // disconnect a slot if a condition occurs + template + size_t disconnect_if(Cond &&cond) + { + lock_type lock(m_mutex); + auto &groups = detail::cow_write(m_slots); + + size_t count = 0; + + for (auto &group : groups) { + auto &slts = group.slts; + size_t i = 0; + while (i < slts.size()) { + if (cond(slts[i])) { + std::swap(slts[i], slts.back()); + slts[i]->index() = i; + slts.pop_back(); + ++count; + } else { + ++i; + } + } + } + + return count; + } + + // to be called under lock: remove all the slots + void clear() { detail::cow_write(m_slots).clear(); } + + private: + Lockable m_mutex; + cow_type m_slots; + std::atomic m_block; +}; + +/** + * Freestanding connect function that defers to the `signal_base::connect` member. + */ +template +std::enable_if_t>, connection> connect(signal_base &sig, Arg &&arg, Args &&...args) +{ + return sig.connect(std::forward(arg), std::forward(args)...); +} + +/** + * Freestanding connect function that chains one signal to another. + */ +template +connection connect(signal_base &sig1, signal_base &sig2, Args &&...args) +{ + return sig1.connect(detail::signal_wrapper>{ std::addressof(sig2) }, std::forward(args)...); +} + +/** + * Specialization of signal_base to be used in single threaded contexts. + * Slot connection, disconnection and signal emission are not thread-safe. + * The performance improvement over the thread-safe variant is not impressive, + * so this is not very useful. + */ +template +using signal_st = signal_base; + +/** + * Specialization of signal_base to be used in multi-threaded contexts. + * Slot connection, disconnection and signal emission are thread-safe. + * + * Recursive signal emission and emission cycles are supported too. + */ +template +using signal = signal_base; + +} // namespace libscratchcpp::sigslot From 30279211edaece7c48043dfb9b2b4d28b5b7b2bb Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Thu, 21 Mar 2024 16:32:13 +0100 Subject: [PATCH 2/8] Replace redraw handler with a signal --- include/scratchcpp/iengine.h | 5 +++-- src/engine/internal/engine.cpp | 7 +++---- src/engine/internal/engine.h | 4 ++-- test/engine/engine_test.cpp | 2 +- test/mocks/enginemock.h | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index 987fba22..1370a346 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -8,6 +8,7 @@ #include #include "global.h" +#include "signal.h" namespace libscratchcpp { @@ -109,8 +110,8 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Stops the event loop which is running in another thread. */ virtual void stopEventLoop() = 0; - /*! Sets the function which is called on every frame. */ - virtual void setRedrawHandler(const std::function &handler) = 0; + /*! Emits when rendering should occur. */ + virtual sigslot::signal<> &aboutToRender() = 0; /*! Returns true if the project is currently running. */ virtual bool isRunning() const = 0; diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index 1ad56431..de4c5346 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -486,8 +486,7 @@ void Engine::step() stepThreads(); // Render - if (m_redrawHandler) - m_redrawHandler(); + m_aboutToRedraw(); } void Engine::run() @@ -509,9 +508,9 @@ void Engine::stopEventLoop() m_stopEventLoopMutex.unlock(); } -void Engine::setRedrawHandler(const std::function &handler) +sigslot::signal<> &Engine::aboutToRender() { - m_redrawHandler = handler; + return m_aboutToRedraw; } std::vector> Engine::stepThreads() diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index d73cc196..f0f627a0 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -49,7 +49,7 @@ class Engine : public IEngine void runEventLoop() override; void stopEventLoop() override; - void setRedrawHandler(const std::function &handler) override; + sigslot::signal<> &aboutToRender() override; bool isRunning() const override; @@ -268,7 +268,7 @@ class Engine : public IEngine bool m_running = false; bool m_redrawRequested = false; - std::function m_redrawHandler = nullptr; + sigslot::signal<> m_aboutToRedraw; bool m_stopEventLoop = false; std::mutex m_stopEventLoopMutex; diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index 1e44ec6f..006f90d0 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -341,7 +341,7 @@ TEST(EngineTest, FpsProject) engine->setFps(10); RedrawMock redrawMock; auto handler = std::bind(&RedrawMock::redraw, &redrawMock); - engine->setRedrawHandler(std::function(handler)); + engine->aboutToRender().connect(handler); std::chrono::steady_clock::time_point time7(std::chrono::milliseconds(100)); std::chrono::steady_clock::time_point time8(std::chrono::milliseconds(200)); std::chrono::steady_clock::time_point time9(std::chrono::milliseconds(300)); diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index be7a2aa3..7ee6c02f 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -32,7 +32,7 @@ class EngineMock : public IEngine MOCK_METHOD(void, runEventLoop, (), (override)); MOCK_METHOD(void, stopEventLoop, (), (override)); - MOCK_METHOD(void, setRedrawHandler, (const std::function &), (override)); + MOCK_METHOD(sigslot::signal<> &, aboutToRender, (), (override)); MOCK_METHOD(bool, isRunning, (), (const, override)); From 9949e33a976c1a98f861f1fbcc7572028c3f2a5b Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Thu, 21 Mar 2024 16:40:30 +0100 Subject: [PATCH 3/8] Replace monitor added/removed handlers with signals --- include/scratchcpp/iengine.h | 8 ++++---- src/engine/internal/engine.cpp | 31 ++++++++++++------------------- src/engine/internal/engine.h | 8 ++++---- test/engine/engine_test.cpp | 6 +++--- test/mocks/enginemock.h | 4 ++-- 5 files changed, 25 insertions(+), 32 deletions(-) diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index 1370a346..f50d864c 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -346,11 +346,11 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Sets the list of monitors. */ virtual void setMonitors(const std::vector> &newMonitors) = 0; - /*! Sets the function which is called when a monitor is added. */ - virtual void setAddMonitorHandler(const std::function &handler) = 0; + /*! Emits when a monitor is added. */ + virtual sigslot::signal &monitorAdded() = 0; - /*! Sets the function which is called when a monitor is removed. */ - virtual void setRemoveMonitorHandler(const std::function &handler) = 0; + /*! Emits when a monitor is removed. */ + virtual sigslot::signal &monitorRemoved() = 0; /*! Returns the function which is called when a question is asked, for example using the 'ask and wait' block. */ virtual const std::function &questionAsked() const = 0; diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index de4c5346..a63cd2b4 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -60,10 +60,8 @@ void Engine::clear() { stop(); - if (m_removeMonitorHandler) { - for (auto monitor : m_monitors) - m_removeMonitorHandler(monitor.get(), monitor->impl->iface); - } + for (auto monitor : m_monitors) + m_monitorRemoved(monitor.get(), monitor->impl->iface); m_sections.clear(); m_targets.clear(); @@ -1220,28 +1218,25 @@ const std::vector> &Engine::monitors() const void Engine::setMonitors(const std::vector> &newMonitors) { - if (m_addMonitorHandler) { - m_monitors.clear(); + m_monitors.clear(); - for (auto monitor : newMonitors) { - m_monitors.push_back(monitor); - m_addMonitorHandler(monitor.get()); - } - } else - m_monitors = newMonitors; + for (auto monitor : newMonitors) { + m_monitors.push_back(monitor); + m_monitorAdded(monitor.get()); + } // Create missing monitors createMissingMonitors(); } -void Engine::setAddMonitorHandler(const std::function &handler) +sigslot::signal &Engine::monitorAdded() { - m_addMonitorHandler = handler; + return m_monitorAdded; } -void Engine::setRemoveMonitorHandler(const std::function &handler) +sigslot::signal &Engine::monitorRemoved() { - m_removeMonitorHandler = handler; + return m_monitorRemoved; } const std::function &Engine::questionAsked() const @@ -1633,9 +1628,7 @@ void Engine::addVarOrListMonitor(std::shared_ptr monitor, Target *targe monitor->setY(rect.top()); m_monitors.push_back(monitor); - - if (m_addMonitorHandler) - m_addMonitorHandler(monitor.get()); + m_monitorAdded(monitor.get()); monitor->setVisible(false); } diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index f0f627a0..79b4fb88 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -143,8 +143,8 @@ class Engine : public IEngine const std::vector> &monitors() const override; void setMonitors(const std::vector> &newMonitors) override; - void setAddMonitorHandler(const std::function &handler) override; - void setRemoveMonitorHandler(const std::function &handler) override; + sigslot::signal &monitorAdded() override; + sigslot::signal &monitorRemoved() override; const std::function &questionAsked() const override; void setQuestionAsked(const std::function &f) override; @@ -272,8 +272,8 @@ class Engine : public IEngine bool m_stopEventLoop = false; std::mutex m_stopEventLoopMutex; - std::function m_addMonitorHandler = nullptr; - std::function m_removeMonitorHandler = nullptr; + sigslot::signal m_monitorAdded; + sigslot::signal m_monitorRemoved; std::function m_questionAsked = nullptr; std::function m_questionAnswered = nullptr; }; diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index 006f90d0..ba0f0ab8 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -106,7 +106,7 @@ TEST(EngineTest, Clear) AddRemoveMonitorMock removeMonitorMock; auto handler = std::bind(&AddRemoveMonitorMock::monitorRemoved, &removeMonitorMock, std::placeholders::_1, std::placeholders::_2); - engine.setRemoveMonitorHandler(std::function(handler)); + engine.monitorRemoved().connect(handler); engine.setMonitors({ monitor1, monitor2, monitor3, monitor4 }); EXPECT_CALL(removeMonitorMock, monitorRemoved(monitor1.get(), &iface1)); @@ -1581,7 +1581,7 @@ TEST(EngineTest, Monitors) AddRemoveMonitorMock addMonitorMock; auto handler = std::bind(&AddRemoveMonitorMock::monitorAdded, &addMonitorMock, std::placeholders::_1); - engine.setAddMonitorHandler(std::function(handler)); + engine.monitorAdded().connect(handler); engine.setMonitors({}); EXPECT_CALL(addMonitorMock, monitorAdded(m1.get())); @@ -1691,7 +1691,7 @@ TEST(EngineTest, CreateMissingMonitors) Engine engine; AddRemoveMonitorMock addMonitorMock; auto handler = std::bind(&AddRemoveMonitorMock::monitorAdded, &addMonitorMock, std::placeholders::_1); - engine.setAddMonitorHandler(std::function(handler)); + engine.monitorAdded().connect(handler); EXPECT_CALL(addMonitorMock, monitorAdded(m1.get())); EXPECT_CALL(addMonitorMock, monitorAdded(m2.get())); diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index 7ee6c02f..ceaff437 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -124,8 +124,8 @@ class EngineMock : public IEngine MOCK_METHOD(const std::vector> &, monitors, (), (const, override)); MOCK_METHOD(void, setMonitors, (const std::vector> &), (override)); - MOCK_METHOD(void, setAddMonitorHandler, (const std::function &), (override)); - MOCK_METHOD(void, setRemoveMonitorHandler, (const std::function &), (override)); + MOCK_METHOD(sigslot::signal &, monitorAdded, (), (override)); + MOCK_METHOD((sigslot::signal &), monitorRemoved, (), (override)); MOCK_METHOD(const std::function &, questionAsked, (), (const, override)); MOCK_METHOD(void, setQuestionAsked, (const std::function &), (override)); From 118be3e2a3152b8d679a97b3300199c9fdbc5afa Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Thu, 21 Mar 2024 18:29:25 +0100 Subject: [PATCH 4/8] Replace question asked/answered handlers with signals --- include/scratchcpp/iengine.h | 14 ++++---------- src/blocks/sensingblocks.cpp | 10 ++++------ src/engine/internal/engine.cpp | 14 ++------------ src/engine/internal/engine.h | 11 ++++------- test/blocks/sensing_blocks_test.cpp | 30 ++++++++++++----------------- test/engine/engine_test.cpp | 28 --------------------------- test/mocks/enginemock.h | 7 ++----- 7 files changed, 28 insertions(+), 86 deletions(-) diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index f50d864c..56df1aa4 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -352,17 +352,11 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Emits when a monitor is removed. */ virtual sigslot::signal &monitorRemoved() = 0; - /*! Returns the function which is called when a question is asked, for example using the 'ask and wait' block. */ - virtual const std::function &questionAsked() const = 0; + /*! Emits when a question is asked, for example using the 'ask and wait' block. */ + virtual sigslot::signal &questionAsked() = 0; - /*! Sets the function which is called when a question is asked, for example using the 'ask and wait' block. */ - virtual void setQuestionAsked(const std::function &f) = 0; - - /*! Returns the function which should be called when a question is answered. */ - virtual const std::function &questionAnswered() const = 0; - - /*! Sets the function which should be called when a question is answered. */ - virtual void setQuestionAnswered(const std::function &f) = 0; + /*! Emits when a question is answered. */ + virtual sigslot::signal &questionAnswered() = 0; /*! Returns the list of extension names. */ virtual const std::vector &extensions() const = 0; diff --git a/src/blocks/sensingblocks.cpp b/src/blocks/sensingblocks.cpp index 180da8b6..7b91c8b0 100644 --- a/src/blocks/sensingblocks.cpp +++ b/src/blocks/sensingblocks.cpp @@ -88,7 +88,7 @@ void SensingBlocks::registerBlocks(IEngine *engine) engine->addFieldValue(this, "backdrop name", BackdropName); // Callbacks - engine->setQuestionAnswered(&onAnswer); + engine->questionAnswered().connect(&onAnswer); } void SensingBlocks::compileDistanceTo(Compiler *compiler) @@ -946,7 +946,7 @@ void SensingBlocks::askNextQuestion() Question *question = m_questionList.front().get(); Target *target = question->vm->target(); - auto ask = question->vm->engine()->questionAsked(); + IEngine *engine = question->vm->engine(); // If the target is visible, emit a blank question and show // a bubble unless the target was the stage @@ -954,10 +954,8 @@ void SensingBlocks::askNextQuestion() target->setBubbleType(Target::BubbleType::Say); target->setBubbleText(question->question); - if (ask) - ask(""); + engine->questionAsked()(""); } else { - if (ask) - ask(question->question); + engine->questionAsked()(question->question); } } diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index a63cd2b4..3b4d757f 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -1239,26 +1239,16 @@ sigslot::signal &Engine::monitorRemoved() return m_monitorRemoved; } -const std::function &Engine::questionAsked() const +sigslot::signal &Engine::questionAsked() { return m_questionAsked; } -void Engine::setQuestionAsked(const std::function &f) -{ - m_questionAsked = f; -} - -const std::function &Engine::questionAnswered() const +sigslot::signal &Engine::questionAnswered() { return m_questionAnswered; } -void Engine::setQuestionAnswered(const std::function &f) -{ - m_questionAnswered = f; -} - const std::vector &Engine::extensions() const { return m_extensions; diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index 79b4fb88..1ac3aef3 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -146,11 +146,8 @@ class Engine : public IEngine sigslot::signal &monitorAdded() override; sigslot::signal &monitorRemoved() override; - const std::function &questionAsked() const override; - void setQuestionAsked(const std::function &f) override; - - const std::function &questionAnswered() const override; - void setQuestionAnswered(const std::function &f) override; + sigslot::signal &questionAsked() override; + sigslot::signal &questionAnswered() override; const std::vector &extensions() const override; void setExtensions(const std::vector &newExtensions) override; @@ -274,8 +271,8 @@ class Engine : public IEngine sigslot::signal m_monitorAdded; sigslot::signal m_monitorRemoved; - std::function m_questionAsked = nullptr; - std::function m_questionAnswered = nullptr; + sigslot::signal m_questionAsked; + sigslot::signal m_questionAnswered; }; } // namespace libscratchcpp diff --git a/test/blocks/sensing_blocks_test.cpp b/test/blocks/sensing_blocks_test.cpp index c94efc07..ad13634f 100644 --- a/test/blocks/sensing_blocks_test.cpp +++ b/test/blocks/sensing_blocks_test.cpp @@ -102,14 +102,6 @@ struct QuestionSpy MOCK_METHOD(void, asked, (const std::string &), ()); }; -template -size_t getAddress(std::function f) -{ - typedef T(fnType)(U...); - fnType **fnPointer = f.template target(); - return (size_t)*fnPointer; -} - TEST_F(SensingBlocksTest, Name) { ASSERT_EQ(m_section->name(), "Sensing"); @@ -181,13 +173,13 @@ TEST_F(SensingBlocksTest, RegisterBlocks) EXPECT_CALL(m_engineMock, addFieldValue(m_section.get(), "backdrop name", SensingBlocks::BackdropName)); // Callbacks - std::function questionAnsweredRef = &SensingBlocks::onAnswer; - std::function questionAnswered; - EXPECT_CALL(m_engineMock, setQuestionAnswered(_)).WillOnce(SaveArg<0>(&questionAnswered)); + sigslot::signal questionAnswered; + EXPECT_CALL(m_engineMock, questionAnswered()).WillOnce(ReturnRef(questionAnswered)); m_section->registerBlocks(&m_engineMock); - ASSERT_TRUE(questionAnswered); - ASSERT_EQ(getAddress(questionAnsweredRef), getAddress(questionAnswered)); + + ASSERT_EQ(questionAnswered.slot_count(), 1); + ASSERT_EQ(questionAnswered.disconnect(&SensingBlocks::onAnswer), 1); } TEST_F(SensingBlocksTest, DistanceTo) @@ -381,14 +373,16 @@ TEST_F(SensingBlocksTest, AskAndWaitAndAnswerImpl) sprite.setBubbleType(Target::BubbleType::Think); Stage stage; QuestionSpy spy; - std::function asked = std::bind(&QuestionSpy::asked, &spy, std::placeholders::_1); + auto asked = std::bind(&QuestionSpy::asked, &spy, std::placeholders::_1); + sigslot::signal askedSignal; + askedSignal.connect(asked); VirtualMachine vm1(&sprite, &m_engineMock, nullptr); vm1.setFunctions(functions); vm1.setConstValues(constValues); // Ask 3 questions (2 where the sprite is visible and 1 where it's invisible) - EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(asked)); + EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(askedSignal)); EXPECT_CALL(spy, asked("")); sprite.setVisible(true); vm1.setBytecode(bytecode1); @@ -435,7 +429,7 @@ TEST_F(SensingBlocksTest, AskAndWaitAndAnswerImpl) ASSERT_EQ(sprite.bubbleText(), "test1"); // Answer the questions - EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(asked)); + EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(askedSignal)); EXPECT_CALL(spy, asked("")); SensingBlocks::onAnswer("hi"); ASSERT_EQ(sprite.bubbleType(), Target::BubbleType::Say); @@ -447,7 +441,7 @@ TEST_F(SensingBlocksTest, AskAndWaitAndAnswerImpl) ASSERT_EQ(vm1.registerCount(), 1); ASSERT_EQ(vm1.getInput(0, 1)->toString(), "hi"); - EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(asked)); + EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(askedSignal)); EXPECT_CALL(spy, asked("test3")); SensingBlocks::onAnswer("hello"); ASSERT_TRUE(sprite.bubbleText().empty()); @@ -457,7 +451,7 @@ TEST_F(SensingBlocksTest, AskAndWaitAndAnswerImpl) ASSERT_EQ(vm1.registerCount(), 1); ASSERT_EQ(vm1.getInput(0, 1)->toString(), "hello"); - EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(asked)); + EXPECT_CALL(m_engineMock, questionAsked()).WillOnce(ReturnRef(askedSignal)); EXPECT_CALL(spy, asked("test2")); SensingBlocks::onAnswer("world"); ASSERT_TRUE(sprite.bubbleText().empty()); diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index ba0f0ab8..d76eb00b 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -54,14 +54,6 @@ class AddRemoveMonitorMock MOCK_METHOD(void, monitorRemoved, (Monitor *, IMonitorHandler *)); }; -template -size_t getAddress(std::function f) -{ - typedef T(fnType)(U...); - fnType **fnPointer = f.template target(); - return (size_t)*fnPointer; -} - TEST(EngineTest, Clock) { Engine engine; @@ -1715,26 +1707,6 @@ void questionFunction(const std::string &) { } -TEST(EngineTest, QuestionAsked) -{ - Engine engine; - ASSERT_EQ(engine.questionAsked(), nullptr); - - static const std::function f = &questionFunction; - engine.setQuestionAsked(&questionFunction); - ASSERT_EQ(getAddress(engine.questionAsked()), getAddress(f)); -} - -TEST(EngineTest, QuestionAnswered) -{ - Engine engine; - ASSERT_EQ(engine.questionAnswered(), nullptr); - - static const std::function f = &questionFunction; - engine.setQuestionAnswered(f); - ASSERT_EQ(getAddress(engine.questionAnswered()), getAddress(f)); -} - TEST(EngineTest, Clones) { Project p("clones.sb3"); diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index ceaff437..ebda814a 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -127,11 +127,8 @@ class EngineMock : public IEngine MOCK_METHOD(sigslot::signal &, monitorAdded, (), (override)); MOCK_METHOD((sigslot::signal &), monitorRemoved, (), (override)); - MOCK_METHOD(const std::function &, questionAsked, (), (const, override)); - MOCK_METHOD(void, setQuestionAsked, (const std::function &), (override)); - - MOCK_METHOD(const std::function &, questionAnswered, (), (const, override)); - MOCK_METHOD(void, setQuestionAnswered, (const std::function &), (override)); + MOCK_METHOD(sigslot::signal &, questionAsked, (), (override)); + MOCK_METHOD(sigslot::signal &, questionAnswered, (), (override)); MOCK_METHOD(std::vector &, extensions, (), (const, override)); MOCK_METHOD(void, setExtensions, (const std::vector &), (override)); From d3e1e251d25f7d48f1a805a9e28618a62a070d5f Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Fri, 22 Mar 2024 15:53:43 +0100 Subject: [PATCH 5/8] Replace download progress callback with a signal --- include/scratchcpp/project.h | 3 ++- src/internal/iprojectdownloader.h | 3 ++- src/internal/projectdownloader.cpp | 14 +++----------- src/internal/projectdownloader.h | 5 ++--- src/internal/projectdownloaderstub.cpp | 3 ++- src/internal/projectdownloaderstub.h | 5 ++++- src/project.cpp | 6 +++--- src/project_p.cpp | 4 ++-- src/project_p.h | 2 +- test/mocks/projectdownloadermock.h | 2 +- test/project/project_test.cpp | 10 +++++----- 11 files changed, 27 insertions(+), 30 deletions(-) diff --git a/include/scratchcpp/project.h b/include/scratchcpp/project.h index 39ed3996..414ae47b 100644 --- a/include/scratchcpp/project.h +++ b/include/scratchcpp/project.h @@ -7,6 +7,7 @@ #include "spimpl.h" #include "global.h" +#include "signal.h" namespace libscratchcpp { @@ -42,7 +43,7 @@ class LIBSCRATCHCPP_EXPORT Project std::shared_ptr engine() const; - void setDownloadProgressCallback(const std::function &&f); + sigslot::signal &downloadProgressChanged(); private: spimpl::unique_impl_ptr impl; diff --git a/src/internal/iprojectdownloader.h b/src/internal/iprojectdownloader.h index 84fedbfd..bafc4562 100644 --- a/src/internal/iprojectdownloader.h +++ b/src/internal/iprojectdownloader.h @@ -5,6 +5,7 @@ #include #include #include +#include namespace libscratchcpp { @@ -18,7 +19,7 @@ class IProjectDownloader virtual bool downloadAssets(const std::vector &assetIds) = 0; virtual void cancel() = 0; - virtual void setDownloadProgressCallback(const std::function &f) = 0; + virtual sigslot::signal &downloadProgressChanged() = 0; virtual const std::string &json() const = 0; virtual const std::vector &assets() const = 0; diff --git a/src/internal/projectdownloader.cpp b/src/internal/projectdownloader.cpp index 92ed4dff..15e31246 100644 --- a/src/internal/projectdownloader.cpp +++ b/src/internal/projectdownloader.cpp @@ -149,13 +149,7 @@ bool ProjectDownloader::downloadAssets(const std::vector &assetIds) m_assets[index] = downloader->text(); m_downloadedAssetCount++; std::cout << "Downloaded assets: " << m_downloadedAssetCount << " of " << count << std::endl; - - m_downloadProgressCallbackMutex.lock(); - - if (m_downloadProgressCallback) - m_downloadProgressCallback(m_downloadedAssetCount, count); - - m_downloadProgressCallbackMutex.unlock(); + m_downloadProgressChanged(m_downloadedAssetCount, count); m_assetsMutex.unlock(); } } @@ -182,11 +176,9 @@ void ProjectDownloader::cancel() m_downloadedAssetCount = 0; } -void ProjectDownloader::setDownloadProgressCallback(const std::function &f) +sigslot::signal &ProjectDownloader::downloadProgressChanged() { - m_downloadProgressCallbackMutex.lock(); - m_downloadProgressCallback = f; - m_downloadProgressCallbackMutex.unlock(); + return m_downloadProgressChanged; } const std::string &ProjectDownloader::json() const diff --git a/src/internal/projectdownloader.h b/src/internal/projectdownloader.h index 2d1c111f..15b47472 100644 --- a/src/internal/projectdownloader.h +++ b/src/internal/projectdownloader.h @@ -23,7 +23,7 @@ class ProjectDownloader : public IProjectDownloader bool downloadAssets(const std::vector &assetIds) override; void cancel() override; - void setDownloadProgressCallback(const std::function &f) override; + sigslot::signal &downloadProgressChanged() override; const std::string &json() const override; const std::vector &assets() const override; @@ -38,8 +38,7 @@ class ProjectDownloader : public IProjectDownloader std::atomic m_downloadedAssetCount = 0; bool m_cancel = false; std::mutex m_cancelMutex; - std::function m_downloadProgressCallback = nullptr; - std::mutex m_downloadProgressCallbackMutex; + sigslot::signal m_downloadProgressChanged; }; } // namespace libscratchcpp diff --git a/src/internal/projectdownloaderstub.cpp b/src/internal/projectdownloaderstub.cpp index 67026fa3..a0d503b8 100644 --- a/src/internal/projectdownloaderstub.cpp +++ b/src/internal/projectdownloaderstub.cpp @@ -22,8 +22,9 @@ void ProjectDownloaderStub::cancel() { } -void ProjectDownloaderStub::setDownloadProgressCallback(const std::function &) +sigslot::signal &ProjectDownloaderStub::downloadProgressChanged() { + return m_downloadProgressChanged; } const std::string &ProjectDownloaderStub::json() const diff --git a/src/internal/projectdownloaderstub.h b/src/internal/projectdownloaderstub.h index 32660b1f..b02e80db 100644 --- a/src/internal/projectdownloaderstub.h +++ b/src/internal/projectdownloaderstub.h @@ -16,11 +16,14 @@ class ProjectDownloaderStub : public IProjectDownloader bool downloadAssets(const std::vector &) override; void cancel() override; - virtual void setDownloadProgressCallback(const std::function &) override; + sigslot::signal &downloadProgressChanged() override; const std::string &json() const override; const std::vector &assets() const override; unsigned int downloadedAssetCount() const override; + + private: + sigslot::signal m_downloadProgressChanged; }; } // namespace libscratchcpp diff --git a/src/project.cpp b/src/project.cpp index a489c842..b5d94afe 100644 --- a/src/project.cpp +++ b/src/project.cpp @@ -100,10 +100,10 @@ std::shared_ptr Project::engine() const } /*! - * Sets the function which will be called when the asset download progress changes. + * Emits when the asset download progress changes. * \note The first parameter is the number of downloaded assets and the latter is the number of all assets to download. */ -void Project::setDownloadProgressCallback(const std::function &&f) +sigslot::signal &Project::downloadProgressChanged() { - impl->setDownloadProgressCallback(f); + return impl->downloadProgressChanged(); } diff --git a/src/project_p.cpp b/src/project_p.cpp index f581f117..951c3f3f 100644 --- a/src/project_p.cpp +++ b/src/project_p.cpp @@ -183,7 +183,7 @@ void ProjectPrivate::setScratchVersion(ScratchVersion version) std::cerr << "Unsupported Scratch version: " << static_cast(version) << std::endl; } -void ProjectPrivate::setDownloadProgressCallback(const std::function &f) +sigslot::signal &ProjectPrivate::downloadProgressChanged() { - downloader->setDownloadProgressCallback(f); + return downloader->downloadProgressChanged(); } diff --git a/src/project_p.h b/src/project_p.h index 8eddebd0..6ccabfff 100644 --- a/src/project_p.h +++ b/src/project_p.h @@ -28,7 +28,7 @@ struct ProjectPrivate void detectScratchVersion(); void setScratchVersion(ScratchVersion version); - void setDownloadProgressCallback(const std::function &f); + sigslot::signal &downloadProgressChanged(); ScratchVersion scratchVersion = ScratchVersion::Invalid; std::string fileName; diff --git a/test/mocks/projectdownloadermock.h b/test/mocks/projectdownloadermock.h index 349c55ea..d9897b8f 100644 --- a/test/mocks/projectdownloadermock.h +++ b/test/mocks/projectdownloadermock.h @@ -12,7 +12,7 @@ class ProjectDownloaderMock : public IProjectDownloader MOCK_METHOD(bool, downloadAssets, (const std::vector &), (override)); MOCK_METHOD(void, cancel, (), (override)); - MOCK_METHOD(void, setDownloadProgressCallback, (const std::function &), (override)); + MOCK_METHOD((sigslot::signal &), downloadProgressChanged, (), (override)); MOCK_METHOD(const std::string &, json, (), (const, override)); MOCK_METHOD(const std::vector &, assets, (), (const, override)); diff --git a/test/project/project_test.cpp b/test/project/project_test.cpp index 8d99bca5..c4346a22 100644 --- a/test/project/project_test.cpp +++ b/test/project/project_test.cpp @@ -9,6 +9,7 @@ using namespace libscratchcpp; using ::testing::Return; +using ::testing::ReturnRef; class ProjectTest : public testing::Test { @@ -158,7 +159,7 @@ TEST_F(ProjectTest, ScratchVersion) ASSERT_EQ(p.scratchVersion(), ScratchVersion::Scratch3); } -TEST(LoadProjectTest, DownloadProgressCallback) +TEST(LoadProjectTest, DownloadProgressChanged) { ProjectDownloaderFactoryMock factory; auto downloader = std::make_shared(); @@ -168,8 +169,7 @@ TEST(LoadProjectTest, DownloadProgressCallback) ProjectPrivate p; ProjectPrivate::downloaderFactory = nullptr; - auto lambda = [](unsigned int, unsigned int) {}; - // TODO: Check the function parameter, if possible (std::function doesn't have operator== for this) - EXPECT_CALL(*downloader, setDownloadProgressCallback); - p.setDownloadProgressCallback(lambda); + sigslot::signal signal; + EXPECT_CALL(*downloader, downloadProgressChanged).WillOnce(ReturnRef(signal)); + ASSERT_EQ(&p.downloadProgressChanged(), &signal); } From 5d0ac931b49b122b8c6b3ca52f0801925aa064f3 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Fri, 22 Mar 2024 16:43:32 +0100 Subject: [PATCH 6/8] Add stop all bypass test --- test/engine/engine_test.cpp | 20 ++++++++++++++++++++ test/stop_all_bypass.sb3 | Bin 0 -> 1608 bytes 2 files changed, 20 insertions(+) create mode 100644 test/stop_all_bypass.sb3 diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index d76eb00b..49816ab3 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -1876,6 +1876,26 @@ TEST(EngineTest, StopAll) ASSERT_FALSE(engine->isRunning()); } +TEST(EngineTest, StopAllBypass) +{ + Project p("stop_all_bypass.sb3"); + ASSERT_TRUE(p.load()); + p.run(); + + auto engine = p.engine(); + + Stage *stage = engine->stage(); + ASSERT_TRUE(stage); + + ASSERT_VAR(stage, "i"); + ASSERT_EQ(GET_VAR(stage, "i")->value().toInt(), 1); + + ASSERT_VAR(stage, "j"); + ASSERT_EQ(GET_VAR(stage, "j")->value().toInt(), 5); + + ASSERT_FALSE(engine->isRunning()); +} + TEST(EngineTest, StopOtherScriptsInSprite) { Project p("stop_other_scripts_in_sprite.sb3"); diff --git a/test/stop_all_bypass.sb3 b/test/stop_all_bypass.sb3 new file mode 100644 index 0000000000000000000000000000000000000000..b2c826897e2f2c5466081224c97e4b9c621388e8 GIT binary patch literal 1608 zcma)+X;2af6vqMa0MWc@ph+0QkuQ0I@uoPL>Z`)tWKq$A({$L}7~0D%B@`&ZwVrMaaLP zrD5GJOe^)(R~SE4Ize%S04F3riK5AO-hZzpAdC8ftWSzk7l#I0-ahh$XWH0Nk8W{) zjjU7k{wv^`$ZEXUdPHnu8$0r=3gg=JOFM{ii$>s9KDcq>I9>o(`rKCw{Vz|86m-3d)( z^FopAaOz@9{^IvntkhFU7(iL{%HZ|KXidI?86rkHz%#?PHIs|>_ejICIknV9rvL~u z*=%mrqtxe!!+mMKz+pR>_pBp0wh0&4sZz){;`zmEcJUNW15=#Wq@uA?$WO!gj2VRAKHWIYsl`rfP&*kf!rC$oZ<$Qp6e$`e~``lnaB6B3eOYyUT>{|Bf z^n1g_j6F|=5$vc|klyln^z1HN`NMC&n;;6%M8s-fjJ`6~a7d(ym$t61;A+ z^}~%kYyaG$DcHIdsKQ`AD`_u&%jxbp2@)e$N0=g@jJ8&CM;Ap{>wBGv;}EC9J{lAqDW|K5x9W+qW9;Wlv#m5UPi z=E!_ocoCxiYGA1ReFqT#&?&cUe+IGT4?J}ClJe;dxj2RIl27LRJ;@tj`;-v`B`28f z*5G#-#?pIgJ4O+K`BLwfGvc<_p^gr}Y6`Zf1BQqd88cV<@->@wv!(|Pl!0sGOxeh; zHB0p~X)y7W>U4SB_Je!bdNs7=kCIsGa^yQsF(qfG`V~~88F%&z1s&Y|mTr;@s%gW_ z;%LG$oRUD>V`pzjnNgrTv#vDYxl41yE^$G+wtcE9#O|?Y1WRVfJS`~Rsh0qSqT4#E zE|O~bcuKNk^0#|)jl-$k*#~nqwHN?qX{F1#7twYl35~Tmjmn(RhAQk$q8djyrz)4n zyF_ z{)r@Vw7aWpwwlt*RLbZL$sVrB!)j;GCyE^|Ik?^~&Xb8z|8nKV|=(&YsZdt_994GDNgg4f(YMmjUW8IFmyqb$u z8(D4KsH>gz(iy8g@W2(J7-Pqd*qe#d9+$->9L>Ljtu2MDbV#t9>Y~Us9pL2(1StXj po70=0{Zv0w`&Z|$f&628y2<_j6OWgxxWo?^XtN79CrkXt_BV48w?6;? literal 0 HcmV?d00001 From 85f8b134c250faffe95d6b67680a50bb1ed8e390 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Fri, 22 Mar 2024 23:06:37 +0100 Subject: [PATCH 7/8] Add thread about to stop signal --- include/scratchcpp/iengine.h | 3 ++ src/engine/internal/engine.cpp | 30 ++++++++++++- src/engine/internal/engine.h | 2 + test/3_threads.sb3 | Bin 0 -> 908 bytes test/engine/engine_test.cpp | 74 +++++++++++++++++++++++++++++++-- test/mocks/enginemock.h | 1 + 6 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 test/3_threads.sb3 diff --git a/include/scratchcpp/iengine.h b/include/scratchcpp/iengine.h index 56df1aa4..88e95521 100644 --- a/include/scratchcpp/iengine.h +++ b/include/scratchcpp/iengine.h @@ -113,6 +113,9 @@ class LIBSCRATCHCPP_EXPORT IEngine /*! Emits when rendering should occur. */ virtual sigslot::signal<> &aboutToRender() = 0; + /*! Emits when a script is about to stop. */ + virtual sigslot::signal &threadAboutToStop() = 0; + /*! Returns true if the project is currently running. */ virtual bool isRunning() const = 0; diff --git a/src/engine/internal/engine.cpp b/src/engine/internal/engine.cpp index 3b4d757f..4665d438 100644 --- a/src/engine/internal/engine.cpp +++ b/src/engine/internal/engine.cpp @@ -63,6 +63,11 @@ void Engine::clear() for (auto monitor : m_monitors) m_monitorRemoved(monitor.get(), monitor->impl->iface); + for (auto thread : m_threads) { + if (!thread->atEnd()) + m_threadAboutToStop(thread.get()); + } + m_sections.clear(); m_targets.clear(); m_broadcasts.clear(); @@ -328,6 +333,11 @@ void Engine::stop() } else { // If there isn't any active thread, it means the project was stopped from the outside // In this case all threads should be removed and the project should be considered stopped + for (auto thread : m_threads) { + if (!thread->atEnd()) + m_threadAboutToStop(thread.get()); + } + m_threads.clear(); m_running = false; } @@ -511,6 +521,11 @@ sigslot::signal<> &Engine::aboutToRender() return m_aboutToRedraw; } +sigslot::signal &Engine::threadAboutToStop() +{ + return m_threadAboutToStop; +} + std::vector> Engine::stepThreads() { // https://github.com/scratchfoundation/scratch-vm/blob/develop/src/engine/sequencer.js#L70-L173 @@ -548,8 +563,14 @@ std::vector> Engine::stepThreads() } // Remove threads in m_threadsToStop - for (auto thread : m_threadsToStop) + for (auto thread : m_threadsToStop) { + if (!thread->atEnd()) + m_threadAboutToStop(thread.get()); + m_threads.erase(std::remove(m_threads.begin(), m_threads.end(), thread), m_threads.end()); + } + + m_threadsToStop.clear(); // Remove inactive threads (and add them to doneThreads) m_threads.erase( @@ -1658,6 +1679,10 @@ void Engine::stopThread(VirtualMachine *thread) { // https://github.com/scratchfoundation/scratch-vm/blob/f1aa92fad79af17d9dd1c41eeeadca099339a9f1/src/engine/runtime.js#L1667-L1672 assert(thread); + + if (!thread->atEnd()) + m_threadAboutToStop(thread); + thread->kill(); } @@ -1668,6 +1693,9 @@ std::shared_ptr Engine::restartThread(std::shared_ptratEnd()) + m_threadAboutToStop(thread.get()); + auto i = it - m_threads.begin(); m_threads[i] = newThread; return newThread; diff --git a/src/engine/internal/engine.h b/src/engine/internal/engine.h index 1ac3aef3..69b05dd6 100644 --- a/src/engine/internal/engine.h +++ b/src/engine/internal/engine.h @@ -50,6 +50,7 @@ class Engine : public IEngine void stopEventLoop() override; sigslot::signal<> &aboutToRender() override; + sigslot::signal &threadAboutToStop() override; bool isRunning() const override; @@ -266,6 +267,7 @@ class Engine : public IEngine bool m_running = false; bool m_redrawRequested = false; sigslot::signal<> m_aboutToRedraw; + sigslot::signal m_threadAboutToStop; bool m_stopEventLoop = false; std::mutex m_stopEventLoopMutex; diff --git a/test/3_threads.sb3 b/test/3_threads.sb3 new file mode 100644 index 0000000000000000000000000000000000000000..10caf280b96ad18151ead2015bd848025b60b988 GIT binary patch literal 908 zcmWIWW@h1HU|`^25L{apQC3;Z_nMJ`L4}opfd?pBP?VpQnp~onRh*x=|S41 zzj>>ZW8Yy37vuY#c6VG}1ejdxpTWsKEw`{EHHmj$d#mHIcSo-0zYot}{r>NtpI2X< z?XLR#>F)ABQ}5bYH(Jd9arf+#yLQ6M_&c+XKRYCx!MeZ3Im>iu+n0ro6aC)oa1(zo zKkY>n^IW67Q461C$>b^hkK16`klvGVC&Ndr%DiX!lH7hb`L`QbeEAn;9%K;{&)o6s z&NG!V|dxjPCvS4#g;j-B5U(*yGBmioFp zewQ62d6DnP`o6QJ>nG;f?l9OIvGnDfLwj12vKTBUp38ZpdzYsptCDw}^zyY+mrb6b zyVGuxeUY(9>;tC$eMc>FXEd1gZW9#Wc_#H%{xsQ{(5n0u?i3;rx+QU8k(dSm>L_VrKF`L85tQFn4}t{CYz?FSePX06_=&w`g-#h(Y6&@dVD*LD*Q2KUrWwx6m%h#zCcNnLW~Y;?PDQqnt)fk) zc^kG|zB;qyW2EBb4GWumPhYlhZ=IHr+2W!3vE-Q@&myhUK{31%;!hUGv(y+ca%Q#o6E}PZvKzO&hpW+dd>tN@weZ->lp&P8JR>FaHn;k l&yWCe%176U9v}#9qD;gEPk=Wo8%P%u5cUA+S3rFX3;+ljceDTi literal 0 HcmV?d00001 diff --git a/test/engine/engine_test.cpp b/test/engine/engine_test.cpp index 49816ab3..2b0064fe 100644 --- a/test/engine/engine_test.cpp +++ b/test/engine/engine_test.cpp @@ -47,6 +47,12 @@ class RedrawMock MOCK_METHOD(void, redraw, ()); }; +class ThreadAboutToStopMock +{ + public: + MOCK_METHOD(void, threadRemoved, (VirtualMachine *)); +}; + class AddRemoveMonitorMock { public: @@ -109,6 +115,44 @@ TEST(EngineTest, Clear) ASSERT_TRUE(engine.monitors().empty()); } +TEST(EngineTest, ClearThreadAboutToStopSignal) +{ + Project p("3_threads.sb3"); + ASSERT_TRUE(p.load()); + auto engine = p.engine(); + + engine->start(); + engine->step(); + + ThreadAboutToStopMock threadRemovedMock; + EXPECT_CALL(threadRemovedMock, threadRemoved(_)).Times(3).WillRepeatedly(WithArgs<0>(Invoke([](VirtualMachine *vm) { + ASSERT_TRUE(vm); + ASSERT_FALSE(vm->atEnd()); + }))); + + engine->threadAboutToStop().connect(&ThreadAboutToStopMock::threadRemoved, &threadRemovedMock); + engine->clear(); +} + +TEST(EngineTest, StopThreadAboutToStopSignal) +{ + Project p("3_threads.sb3"); + ASSERT_TRUE(p.load()); + auto engine = p.engine(); + + engine->start(); + engine->step(); + + ThreadAboutToStopMock threadRemovedMock; + EXPECT_CALL(threadRemovedMock, threadRemoved(_)).Times(3).WillRepeatedly(WithArgs<0>(Invoke([](VirtualMachine *vm) { + ASSERT_TRUE(vm); + ASSERT_FALSE(vm->atEnd()); + }))); + + engine->threadAboutToStop().connect(&ThreadAboutToStopMock::threadRemoved, &threadRemovedMock); + engine->stop(); +} + TEST(EngineTest, CompileAndExecuteMonitors) { Engine engine; @@ -1840,10 +1884,18 @@ TEST(EngineTest, BroadcastsProject) { Project p("broadcasts.sb3"); ASSERT_TRUE(p.load()); - p.run(); auto engine = p.engine(); + + ThreadAboutToStopMock threadRemovedMock; + EXPECT_CALL(threadRemovedMock, threadRemoved(_)).Times(21).WillRepeatedly(WithArgs<0>(Invoke([](VirtualMachine *vm) { + ASSERT_TRUE(vm); + ASSERT_FALSE(vm->atEnd()); + }))); + + engine->threadAboutToStop().connect(&ThreadAboutToStopMock::threadRemoved, &threadRemovedMock); engine->setFps(1000); + p.run(); Stage *stage = engine->stage(); ASSERT_TRUE(stage); @@ -1880,10 +1932,18 @@ TEST(EngineTest, StopAllBypass) { Project p("stop_all_bypass.sb3"); ASSERT_TRUE(p.load()); - p.run(); auto engine = p.engine(); + ThreadAboutToStopMock threadRemovedMock; + EXPECT_CALL(threadRemovedMock, threadRemoved(_)).Times(2).WillRepeatedly(WithArgs<0>(Invoke([](VirtualMachine *vm) { + ASSERT_TRUE(vm); + ASSERT_FALSE(vm->atEnd()); + }))); + + engine->threadAboutToStop().connect(&ThreadAboutToStopMock::threadRemoved, &threadRemovedMock); + p.run(); + Stage *stage = engine->stage(); ASSERT_TRUE(stage); @@ -1900,10 +1960,18 @@ TEST(EngineTest, StopOtherScriptsInSprite) { Project p("stop_other_scripts_in_sprite.sb3"); ASSERT_TRUE(p.load()); - p.run(); auto engine = p.engine(); + ThreadAboutToStopMock threadRemovedMock; + EXPECT_CALL(threadRemovedMock, threadRemoved(_)).Times(4).WillRepeatedly(WithArgs<0>(Invoke([](VirtualMachine *vm) { + ASSERT_TRUE(vm); + ASSERT_FALSE(vm->atEnd()); + }))); + + engine->threadAboutToStop().connect(&ThreadAboutToStopMock::threadRemoved, &threadRemovedMock); + p.run(); + Stage *stage = engine->stage(); ASSERT_TRUE(stage); diff --git a/test/mocks/enginemock.h b/test/mocks/enginemock.h index ebda814a..1a3d67ee 100644 --- a/test/mocks/enginemock.h +++ b/test/mocks/enginemock.h @@ -33,6 +33,7 @@ class EngineMock : public IEngine MOCK_METHOD(void, stopEventLoop, (), (override)); MOCK_METHOD(sigslot::signal<> &, aboutToRender, (), (override)); + MOCK_METHOD(sigslot::signal &, threadAboutToStop, (), (override)); MOCK_METHOD(bool, isRunning, (), (const, override)); From 049fc346557cc6c0cd2bf9df3788d58f32b64c59 Mon Sep 17 00:00:00 2001 From: adazem009 <68537469+adazem009@users.noreply.github.com> Date: Fri, 22 Mar 2024 23:31:24 +0100 Subject: [PATCH 8/8] Remove stopped threads in blocks implementation --- src/blocks/looksblocks.cpp | 14 ++++++++++++++ src/blocks/looksblocks.h | 1 + src/blocks/motionblocks.cpp | 8 ++++++++ src/blocks/motionblocks.h | 1 + src/blocks/soundblocks.cpp | 10 +++++++++- 5 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/blocks/looksblocks.cpp b/src/blocks/looksblocks.cpp index 0072ed45..cc1eb369 100644 --- a/src/blocks/looksblocks.cpp +++ b/src/blocks/looksblocks.cpp @@ -85,6 +85,20 @@ void LooksBlocks::registerBlocks(IEngine *engine) engine->addFieldValue(this, "backward", Backward); } +void LooksBlocks::onInit(IEngine *engine) +{ + engine->threadAboutToStop().connect([](VirtualMachine *vm) { + m_timeMap.erase(vm); + + for (auto it = m_waitingBubbles.begin(); it != m_waitingBubbles.end();) { + if (it->second == vm) + m_waitingBubbles.erase(it); + else + it++; + } + }); +} + void LooksBlocks::compileSayForSecs(Compiler *compiler) { compiler->addInput(MESSAGE); diff --git a/src/blocks/looksblocks.h b/src/blocks/looksblocks.h index 91a598b4..60d54fc5 100644 --- a/src/blocks/looksblocks.h +++ b/src/blocks/looksblocks.h @@ -61,6 +61,7 @@ class LooksBlocks : public IBlockSection std::string name() const override; void registerBlocks(IEngine *engine) override; + void onInit(IEngine *engine) override; static void compileSayForSecs(Compiler *compiler); static void compileSay(Compiler *compiler); diff --git a/src/blocks/motionblocks.cpp b/src/blocks/motionblocks.cpp index c49a3dd0..012d436d 100644 --- a/src/blocks/motionblocks.cpp +++ b/src/blocks/motionblocks.cpp @@ -71,6 +71,14 @@ void MotionBlocks::registerBlocks(IEngine *engine) engine->addFieldValue(this, "all around", AllAround); } +void MotionBlocks::onInit(IEngine *engine) +{ + engine->threadAboutToStop().connect([](VirtualMachine *vm) { + m_timeMap.erase(vm); + m_glideMap.erase(vm); + }); +} + void MotionBlocks::compileMoveSteps(Compiler *compiler) { compiler->addInput(STEPS); diff --git a/src/blocks/motionblocks.h b/src/blocks/motionblocks.h index 6c3e4cac..0b62ee59 100644 --- a/src/blocks/motionblocks.h +++ b/src/blocks/motionblocks.h @@ -47,6 +47,7 @@ class MotionBlocks : public IBlockSection std::string name() const override; void registerBlocks(IEngine *engine) override; + void onInit(IEngine *engine) override; static void compileMoveSteps(Compiler *compiler); static void compileTurnRight(Compiler *compiler); diff --git a/src/blocks/soundblocks.cpp b/src/blocks/soundblocks.cpp index 4543f6c3..fab7302e 100644 --- a/src/blocks/soundblocks.cpp +++ b/src/blocks/soundblocks.cpp @@ -36,7 +36,15 @@ void SoundBlocks::registerBlocks(IEngine *engine) void SoundBlocks::onInit(IEngine *engine) { m_waitingSounds.clear(); - // TODO: Remove stopped threads from m_waitingSounds + + engine->threadAboutToStop().connect([](VirtualMachine *vm) { + for (auto it = m_waitingSounds.begin(); it != m_waitingSounds.end();) { + if (it->second == vm) + m_waitingSounds.erase(it); + else + it++; + } + }); } bool SoundBlocks::compilePlayCommon(Compiler *compiler, bool untilDone, bool *byIndex)