Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event actions with one or more arguments #2

Open
philippjbauer opened this issue Mar 12, 2021 · 8 comments
Open

Event actions with one or more arguments #2

philippjbauer opened this issue Mar 12, 2021 · 8 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@philippjbauer
Copy link
Owner

philippjbauer commented Mar 12, 2021

I added an event system to enable pub/sub on all components of Photino.

It generally works at the moment but it's not quite how I imagine this to work. At the moment, an event action can only accept exactly two arguments, TEventClass *sender, std::string message. I want to be able to use one or more parameters, depending on the need of the emitted event (of any type, may be templated). I will add a code example at the end.

At the moment, the message argument may be empty (defaults to nullptr). But when you are adding an action, you have to define the message parameter in its method signature even when you expect no message at all. That isn't a great experience!

Currently I define the message argument std::string *empty for actions that expect no message.

The class Events is templated and can be attached to a component class like App or Window:

// src/Photino/App/App.mm
App *App::Init()
{
    _events = new Photino::Events<App, AppEvents>(this);
    ...
}

A new event action can be registered like this ...

// src/main.mm
App *app = new App();
app
    ->Events()
    ->AddEventAction(AppEvents::WillRun, [](App *sender, std::string *empty)
    {
        Log::WriteLine("Application is about to run.");
    });

And emitted like this ...

// src/Photino/App/App.mm
void App::Run()
{
    this->Events()->EmitEvent(AppEvents::WillRun);
    
    // Example for emitted event with message, for illustration only.
    // std::string message = "Hello World!";
    // this->Events()->EmitEvent(AppEvents::LogMessage, &message);
    
    [_application run];
}

I would like to be able to add actions like this:

// For illustration purposes
App *app = new App();
app
    ->Events()
    ->AddEventAction(AppEvents::WillRun, [](App *sender) // Only one param
    {
        Log::WriteLine("Application is about to run.");
    })
    ->AddEventAction(AppEvents::LogMessage, [](App *sender, std::string *message) // Default 2nd param type
    {
        Log::WriteLine("Logging message: " + *message);
    }
    ->AddEventAction<WindowSize>(AppEvents::LogMessage, [](App *sender, WindowSize *size) // Any 2nd param type
    {
        Log::WriteLine("Window size: " + *size->ToString());
    }
    ->AddEventAction<int, int>(AppEvents::LogMessage, [](App *sender, int *width, int *height) // N typed params
    {
        Log::WriteLine("Window size: " + *width + " × " + *height);
    });

This is where I currently hit the limit of my C++ knowledge of 2 weeks. Is all of this possible at all?

Another aspect is how the Events class defines the types for the Event System. It seems like you can't overload such a definition. Is it possible to avoid an explosion in verbose type definitions for this?

// src/Photino/Events/Events.h
// EventActions
template<class TEventClass>
using EventActions = std::vector<EventAction<TEventClass> >;

// EventTypeActions
template<class TEventClass, typename TEventTypeEnum>
using EventTypeActions = std::pair<TEventTypeEnum, EventActions<TEventClass>* >;

// EventMap
template<class TEventClass, typename TEventTypeEnum>
using EventMap = std::map<TEventTypeEnum, EventActions<TEventClass>* >;

Any help is much appreciated, thanks!

Current feature branch: https://github.com/philippjbauer/Photino.Native/tree/event-action-one-or-more-args

@philippjbauer philippjbauer added enhancement New feature or request help wanted Extra attention is needed labels Mar 12, 2021
@liff-engineer
Copy link

Save yours arguments to std::tuple, and store in std::any ; use std::apply warp your event handler as std::function<void(std::any&)>, example code :

#include <any>
#include <string>
#include <map>
#include <functional>
#include <tuple>
#include <type_traits>
#include <iostream>


class EventDispatcher
{
    std::map<std::size_t, std::function<void(std::any&)>> handlers;
public:
    template<typename... Ts, typename F>
    void registerHandler(F h)
    {
        static_assert(sizeof...(Ts) > 0, "cann't empty");
        handlers[typeid(std::tuple<Ts...>).hash_code()] = [=](std::any& arg) {
            if (std::tuple<Ts...>* e = std::any_cast<std::tuple<Ts...>>(&arg); e) {
                std::apply(h, *e);
            }
        };
    }

    template<typename... Ts>
    void post(Ts&&... v) {
        static_assert(sizeof...(Ts) > 0, "cann't empty");
        std::any e = std::make_tuple(std::forward<Ts>(v)...);
        if (auto it = handlers.find(e.type().hash_code()); it != handlers.end()) {
            it->second(e);
        }
    }
};


int main(int argc, char** argv) {
    EventDispatcher dispatcher;
    dispatcher.registerHandler<std::string>([](std::string const& v) {
        std::cout << v << "\n";
        });
    dispatcher.registerHandler<double>([](double v) {
        std::cout << v << "\n";
        });
    dispatcher.registerHandler<std::string, double>([](std::string const& sV, double dV) {
        std::cout << "sV:" << sV << ";";
        std::cout << "dV:" << dV << "\n";
        });
    const std::string author = "[email protected]";
    dispatcher.post(std::string{ "[email protected]" });
    dispatcher.post(3.1415926);
    dispatcher.post(author, 3.1415926);
    return 0;
}

@philippjbauer
Copy link
Owner Author

Thank you @liff-engineer!

I see how this works in general but I do have some questions about the example.

I will write them up later, life is happening around me. :)

@philippjbauer
Copy link
Owner Author

Active discussion on reddit: https://www.reddit.com/r/cpp/comments/m3tcxi/event_handlers_with_variable_arguments_without/

... and a lot appreciation for the community in front of this computer screen.

@philippjbauer
Copy link
Owner Author

@liff-engineer

I found a good explanation of lvalue/rvalue and move semantics today and this makes a lot more sense now.

I have two remaining questions, if you don't mind explaining?

The first is the use of the template type F here:

template<typename... Ts, typename F>
void registerHandler(F h) { ... }

Does this mean the last parameter type that gets defined becomes the return value of the function I pass? If there is just one parameter type, will the input and return types match?

I was trying to modify the example so I can add and post handlers without parameters. I get stuck on the type for handlers that expects a function with any type. Is there a way to make this work so that it also accepts this:

EventDispatcher dispatcher;
dispatcher.registerHandler([]() {
    std::cout << "Event fired!" << "\n";
});

dispatcher.post();

Thanks for the help!

@liff-engineer
Copy link

Let us assume that it is safe to use 0 to identify the handler without parameters, and use tag dispatch to registerHandler, like this:

class EventDispatcher
{
    std::map<std::size_t, std::function<void(std::any&)>> handlers;

    template<typename...>
    struct TypeTags {};
protected:
    template<typename F>
    void registerHandlerImpl(TypeTags<>, F h)
    {
        handlers[0] = [=](std::any& arg) {
            if (!arg.has_value()) {
                h();
            }
        };
    }

    template<typename... Ts, typename F>
    void registerHandlerImpl(TypeTags<Ts...>, F h)
    {
        handlers[typeid(std::tuple<Ts...>).hash_code()] = [=](std::any& arg) {
            if (std::tuple<Ts...>* e = std::any_cast<std::tuple<Ts...>>(&arg); e) {
                std::apply(h, *e);
            }
        };
    }

public:
    template<typename... Ts, typename F>
    void registerHandler(F h)
    {
        registerHandlerImpl(TypeTags<Ts...>{}, h);
    }

    template<typename... Ts>
    void post(Ts&&... v) {
        if constexpr (sizeof...(Ts) == 0) {
            if (auto it = handlers.find(0); it != handlers.end()) {
                it->second(std::any{});
            }
        }
        else
        {
            std::any e = std::make_tuple(std::forward<Ts>(v)...);
            if (auto it = handlers.find(e.type().hash_code()); it != handlers.end()) {
                it->second(e);
            }
        }
    }
};
int main(int argc, char** argv) {
    EventDispatcher dispatcher;
    dispatcher.registerHandler<std::string>([](std::string const& v) {
        std::cout << v << "\n";
        });
    dispatcher.registerHandler<std::string, double>([](std::string const& sV, double dV) {
        std::cout << "sV:" << sV << ";";
        std::cout << "dV:" << dV << "\n";
        });
    dispatcher.registerHandler([]() {
        std::cout << "empty message\n";
        });
    const std::string author = "[email protected]";
    dispatcher.post(std::string{ "[email protected]" });
    dispatcher.post(author, 3.1415926);
    dispatcher.post();
    return 0;
}

@liff-engineer
Copy link

As far as I know, when you call registerHandler,must provide F h argument , the type of F is specify by h ;and variardic arguments will eat all type argument , this means you can‘t specify F.
Sorry i can't give you a definitive answer about first question. You can ask other people for help.

@KonanM
Copy link

KonanM commented Mar 15, 2021

#include <unordered_map>
#include <functional>
#include <memory>

namespace props
{
    template<typename... Args>
    struct Signal
    {
        using IndexT = size_t;
        IndexT index = 0;
        std::unordered_map<IndexT, std::function<void(Args...)>> callbacks;

        //slots can be used to more easily disconnect callbacks
        struct Slot
        {
            Signal* signal = nullptr;
            IndexT idx{};

            Slot() = default;
            Slot(Signal* s, IndexT i) : signal(s), idx(i) {}
            ~Slot()
            {
                if (signal)
                    signal->disconnect(idx);
            }
            void disconnect()
            {
                if (signal)
                {
                    signal->disconnect(idx);
                    signal = nullptr;
                }
            }
            void release()
            {
                signal = nullptr;
            }
            void emit(const Args&... args)
            {
                if (signal)
                    signal->emit(args...);
            }

        };

        template<typename Func>
        IndexT connect(Func&& func)
        {
            callbacks.emplace(++index, std::forward<Func>(func));
            return index;
        }

        template<typename Func>
        [[nodiscard]] Slot connectWithSlot(Func&& func)
        {
            //TODO: support slots that outlive the signal, maybe via enable_shared_from_this?
            return Slot{ this, connect(std::forward<Func>(func)) };
        }

        bool disconnect(IndexT idx)
        {
            return callbacks.erase(idx);
        }

        void disconnectAll()
        {
            callbacks.clear();
        }

        void emit(const Args&... args)
        {
            for (auto& [idx, callback] : callbacks)
            {
                callback(args...);
            }
        }
    };

    //type erased base class for the signal
    struct EventSignalBase
    {
        virtual ~EventSignalBase() = default;
    };
    template<typename... Args>
    struct EventSignal : public EventSignalBase, Signal<Args...>
    {
    };
    //type erased base class for the event descriptor
    struct EventDescriptorBase
    {
        virtual ~EventDescriptorBase() = default;
    };

    template<typename... Args>
    struct EventDescriptor : public EventDescriptorBase
    {
        //giving the event descriptor a string or enum is optional
        //std::string name;
    };

    struct EventSystem
    {
        template<typename... Args>
        void EmitEvent(const EventDescriptor<Args...>& ed, const Args&... args)
        {
            auto it = events.find(&ed);
            if (it != events.end())
            {
                auto* eventSignal = static_cast<EventSignal<Args...>*>(it->second.get());
                eventSignal->emit(args...);
            }
        }

        template<typename Func, typename...Args>
        [[nodiscard]] typename Signal<Args...>::Slot AddEventAction(const EventDescriptor<Args...>& ed, Func&& func)
        {
            auto it = events.find(&ed);
            if (it == events.end())
            {
                auto [newIt, sucess] = events.emplace(&ed, std::make_unique<EventSignal<Args...>>());
                it = newIt;
            }
            auto* eventSignal = static_cast<EventSignal<Args...>*>(it->second.get());
            return eventSignal->connectWithSlot(std::forward<Func>(func));
        }

        std::unordered_map<const EventDescriptorBase*, std::unique_ptr<EventSignalBase>> events;
    };
}

#include <iostream>

int main()
{
    props::EventSystem es;

    props::EventDescriptor<int, int> WindowSizeChangedED;
    auto slot = es.AddEventAction(WindowSizeChangedED, [](int x, int y) { std::cout << x << ", " << y << '\n'; });
    es.EmitEvent(WindowSizeChangedED, 1024, 1024);
    //we can also use the slot to emit an event
    slot.emit(768, 768);
    //the callback will stay in the EventSystem when release is called on the slot
    slot.release();
    //use slot.disconnect() to disconnect the callback, which is also called when the slot is destroyed
    
    struct App {};
    props::EventDescriptor<App*> AppStartedED;
    //if we never want to disconnect the new Event we can simply directly release it
    es.AddEventAction(AppStartedED, [](App* /*sender*/) { }).release();


}

I already gave you an answer on the reddit post, but this is how I would design a rather generic event system. For type safety reasons I'm a big fan of using typed descriptors instead of an enum. I know std::any helps a bit by throwing when you mess up, but it's still a runtime error, which is avoidable with this kind of approach. This also allows very generic extensibility, because anyone can define a new EventDescriptor and it's not restricted to an enum.

Not sure if you need the ability to disconnect from events, but it's a very handy feature which I think comes from Qt (see also https://en.wikipedia.org/wiki/Signals_and_slots).

@philippjbauer
Copy link
Owner Author

@KonanM thank you a ton for this example!

I have been carefully studying this example (and others) and I have learned a lot in the process. I will definitely use this as a reference!

PS: I found this article very enlightening and leave it here as a breadcrumb for others who come through these parts: https://www.goldsborough.me/cpp/2018/05/22/00-32-43-type_erasure_for_unopinionated_interfaces_in_c++/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants