Event System using C++

Question

I am excited to share that I have developed an event system in C++. I have always been passionate about programming and have long aspired to create a low-level game engine solely using C++, OpenGL, and GLFW. This project has been a personal challenge and really fun too.

To begin with, I created my own Event System. The functioning of this system involves pushing events to the event queue. The event manager then publishes the events from the queue to the listeners. Each listener checks if the event is of the type it is interested in, and if so, it handles the event. Once an event is handled, it is removed from the queue.

I am open to any feedback or suggestions regarding the design or any other aspect of the system. Your input would be greatly appreciated.

In addition to any general feedback, I have a few specific questions I'd like to address:

1. In the Event.h file, can you explain the justification for using macros?

2. Although I am aware that .cpp files should be used, there are several objects in this codebase that either are or contain templated methods. Can you elaborate on how this should be handled?

3. Considering the principles of Object-Oriented programming, could you evaluate whether this system is well-written?

For those who are curious, I have also included the source code below:

Event.h

#include <string>

#define EVENT_TYPE_FUNCTION(type)                                                                           \
static const EventType getStaticType() { return EventType::##type; }                                        \
virtual const EventType getType() const override { return EventType::##type; }                              \
virtual const std::string getTypeName() const override { return #type; }

#define EVENT_CATEGORY_FUNCTION(category)                                                                   \
virtual ~category##Event() override = default;                                                              \
virtual const EventCategory getCategory() const override { return EventCategory::##category; }              \
virtual const EventType getType() const override{ return EventType::None; }

enum class EventCategory
{
    None,
    Input,
    Window,
    Mouse,
    Keyboard,
    Application,
};

enum class EventType
{
    None,
    WindowResize, WindowLostFocus, WindowGainedFocus, 
    WindowClose, WindowCursorEnter, WindowCursorLeave,
    MouseMove, MousePress, MouseRelease, MouseScroll,
    KeyboardPress, KeyboardRepeat, KeyboardRelease, CharPress,
};

class Event
{
public:
    virtual ~Event() = default;

    bool isHandeled() const { return handeled; }
    bool handle() { handeled = true; }

    virtual const EventCategory getCategory() const = 0;
    virtual const EventType getType() const = 0;
    virtual const std::string getTypeName() const = 0;

private:
    bool handeled = false;
};                                  

class NoneEvent : public Event
{
    const EventCategory getCategory() const override { return EventCategory::None; }
    EVENT_TYPE_FUNCTION(None)
};

Keyboardevent.h

#include "Event.h"

#include <Stdint.h>

class KeyboardEvent : public Event
{
protected:
    KeyboardEvent(int keyCode, int scancode, int mods)
        : keyCode(keyCode), scancode(scancode), mods(mods) { }

public:
    int getKeyCode() const { return keyCode; }
    int getScanCode() const { return scancode; }
    int getMods() const { return mods; }

    EVENT_CATEGORY_FUNCTION(Keyboard)

private:
    int keyCode;
    int scancode;
    int mods;
};

class KeyboardPressEvent : public KeyboardEvent
{
public:
    KeyboardPressEvent(const int keyCode, const int scancode, const int mods)
        : KeyboardEvent(keyCode, scancode, mods) { }

    EVENT_TYPE_FUNCTION(KeyboardPress)
};

class KeyboardRepeatEvent : public KeyboardEvent
{
    KeyboardRepeatEvent(const int keyCode, const int scancode, const int mods)
        : KeyboardEvent(keyCode, scancode, mods) { }

    EVENT_TYPE_FUNCTION(KeyboardRepeat)
};

class KeyboardReleaseEvent : public KeyboardEvent
{
public:
    KeyboardReleaseEvent(const int keyCode, const int scancode, const int mods)
        : KeyboardEvent(keyCode, scancode, mods) { }

    EVENT_TYPE_FUNCTION(KeyboardRelease)
};

class CharPressEvent : public Event
{
public:
    CharPressEvent(const int codePoint)
        : codePoint(codePoint) { }

    const int getCodePoint() const { return codePoint; }

    const EventCategory getCategory() const override { return EventCategory::Keyboard; }
    EVENT_TYPE_FUNCTION(CharPress)

private:
    const int codePoint;
};

MouseEvent.h

#pragma once

#include "Event.h"

class MouseEvent : public Event 
{
public:
    EVENT_CATEGORY_FUNCTION(Mouse)
};

// ===================== MOTION UPCOMING ==============================

class MouseMotionEvent : public MouseEvent
{
protected:
    MouseMotionEvent(const double mouseX, const double mouseY)
        : mouseX(mouseX), mouseY(mouseY) { }

public:
    const double getMouseX() const { return mouseX; }
    const double getMouseY() const { return mouseY; }

private:
    const double mouseX, mouseY;
};

class MouseMoveEvent : public MouseMotionEvent
{
public:
    MouseMoveEvent(const double mouseX, const double mouseY)
        : MouseMotionEvent(mouseX, mouseY) { }

    EVENT_TYPE_FUNCTION(MouseMove)
};

class MouseScrollEvent : public MouseMotionEvent
{
public:
    MouseScrollEvent(const double mouseX, const double mouseY)
        : MouseMotionEvent(mouseX, mouseY) { }

    EVENT_TYPE_FUNCTION(MouseScroll)
};

// ===================== BUTTON UPCOMING ==============================

class MouseButtonEvent : public MouseEvent
{
protected:
    MouseButtonEvent(const int button, const int mods)
        : button(button), mods(mods) { }

public:
    const int getButton() const { return button; }
    const int getMods() const { return mods; }

private:
    const int button, mods;
};

class MousePressEvent : public MouseButtonEvent
{
public: 
    MousePressEvent(const int button, const int mods)
        : MouseButtonEvent(button, mods) { }

    EVENT_TYPE_FUNCTION(MousePress)
};

class MouseReleaseEvent : public MouseButtonEvent
{
public: 
    MouseReleaseEvent(const int button, const int mods)
        : MouseButtonEvent(button, mods) { }

    EVENT_TYPE_FUNCTION(MouseRelease)
};

WindowEvent.h

#pragma once

#include "Event.h"

class WindowEvent : public Event 
{
public: 
    EVENT_CATEGORY_FUNCTION(Window)
};

class WindowResizeEvent : public WindowEvent 
{
public: 
    WindowResizeEvent(const int width, const int height) 
        : width(width), height(height) { }

    const int getWidth() const { return width; }
    const int getHeight() const { return height; }

    EVENT_TYPE_FUNCTION(WindowResize)

private:
    const int width, height;
};

class WindowCloseEvent : public WindowEvent
{
public:
    EVENT_TYPE_FUNCTION(WindowClose)
};

class WindowLostFocusEvent : public WindowEvent 
{
public: 
    EVENT_TYPE_FUNCTION(WindowLostFocus)
};

class WindowGainedFocusEvent : public WindowEvent 
{
public: 
    EVENT_TYPE_FUNCTION(WindowGainedFocus)
};

class WindowCursorEnterEvent : public WindowEvent
{
public:
    EVENT_TYPE_FUNCTION(WindowCursorEnter)
};

class WindowCursorLeaveEvent : public WindowEvent
{
public:
    EVENT_TYPE_FUNCTION(WindowCursorLeave)
};

EventListener.h

#pragma once

#include <functional>

#include "MouseEvent.h"
#include "KeyboardEvent.h"
#include "WindowEvent.h"

class BaseEventListener
{
public:
    virtual ~BaseEventListener() = default;
    virtual const void dispatchEvent(const std::shared_ptr<Event>&) const = 0;
    virtual bool isEventType(const std::shared_ptr<Event>& event) const = 0;
    virtual const int getID() const = 0;
};

template<typename EventType>
class EventListener : public BaseEventListener
{
public:
    using EventCallBackFn = std::function<void(const std::shared_ptr<EventType>&)>;
                                                                                        
    explicit EventListener(const EventCallBackFn& callBack, const int id) : callBack(callBack), id(id) { }
    const void dispatchEvent(const std::shared_ptr<Event>& event) const override { callBack(std::static_pointer_cast<EventType>(event)); }

    bool isEventType(const std::shared_ptr<Event>& event) const override { return std::dynamic_pointer_cast<EventType>(event) != nullptr; }

    const int getID() const override { return id; }

private:
    const EventCallBackFn callBack;
    const int id;
};

EventListenerRegister.h

#pragma once

#include <vector>
#include <memory>
#include <algorithm>

#include "EventListener.h"

class EventListenerRegister
{
public:
    using ListenerList = std::vector<std::unique_ptr<BaseEventListener>>;

    template<typename EventType>
    int registerListenerFor(const typename EventListener<EventType>::EventCallBackFn& callBack)
    {
        static int listenerID = 0;

        std::unique_ptr<BaseEventListener> listener = std::make_unique<EventListener<EventType>>(callBack, listenerID);
        listeners.push_back(std::move(listener));

        return listenerID++;
    }

    void unregisterListener(const int listenerID)
    {
        listeners.erase(
            std::remove_if(listeners.begin(), listeners.end(), [listenerID](const auto& listener) {
                return listenerID == listener->getID();
                }),
            listeners.end()
        );
    }

    ListenerList::iterator          begin()         { return listeners.begin(); }
    ListenerList::const_iterator    begin() const   { return listeners.begin(); }
    ListenerList::iterator          end()           { return listeners.end();   }
    ListenerList::const_iterator    end() const     { return listeners.end();   }

private:
    ListenerList listeners;
};

EventQueue.h

#pragma once

#include <queue>
#include <memory>

#include "MouseEvent.h"
#include "KeyboardEvent.h"
#include "WindowEvent.h"

class EventQueue
{
public:
    void push(const std::shared_ptr<Event>& event) { eventQueue.push(event); }

    std::shared_ptr<Event> pop()
    {
        if (eventQueue.empty())
            return nullptr;
        std::shared_ptr<Event> event = std::move(eventQueue.front());
        eventQueue.pop();
        return event;
    }

    bool isEmpty() const { return eventQueue.empty(); }

private:
    std::queue<std::shared_ptr<Event>> eventQueue;
};

EventBus.h

#pragma once

#include <memory>

#include "EventListenerRegister.h"
#include "EventQueue.h"

class EventBus
{
public:
    template<typename EventType>
    int registerListenerFor(const typename EventListener<EventType>::EventCallBackFn& callBack)
    {
        return listeners.registerListenerFor<EventType>(callBack);
    }

    void unregisterListener(const int listenerID)
    {
        listeners.unregisterListener(listenerID);
    }

    void publishEvent(EventQueue& eventQueue) const
    {
        while(const auto& event = eventQueue.pop()) {
            for (const auto& listener : listeners) {
                if (listener->isEventType(event))
                    listener->dispatchEvent(event);

                if (event->isHandeled())
                    break;
            }
        }
    }

private:
    EventListenerRegister listeners;
};

EventManager.h

#pragma once

#include "EventBus.h"
#include "EventQueue.h"

class EventManager
{
public:
    template<typename EventType>
    int registerListenerFor(const typename EventListener<EventType>::EventCallBackFn& callBack) 
    { 
        return eventBus.registerListenerFor<EventType>(callBack);
    }

    void unregisterListener(const int listenerID) 
    { 
        eventBus.unregisterListener(listenerID);
    }

    void pushEvent(const std::shared_ptr<Event>& event) 
    { 
        eventQueue.push(event); 
    }

    void publishEvents()
    {
        eventBus.publishEvent(eventQueue);
    }

private:
    EventBus eventBus;
    EventQueue eventQueue;
};

Answer 1

Avoid macros

There are many issues with macros, the general advice is to avoid them where possible.

Let's look at EVENT_TYPE_FUNCTION(): it's there to add a few member functions to query the event type. First, let's try to avoid adding three functions, and reduce it to one, that just returns the type. Then we can avoid a macro and just write:

class NoneEvent: public Event
{
    EventCategory getCategory() const override { return EventCategory::None; }
    EventType getType() const override { return EventType::None; }
};

Note how it is similar to the definition of getCategory(). So how do we get getStaticType() and getTypeName()?

First, I would remove getStaticType() completely. It is unsafe and/or unnecessary: if you allow further inheritance from types that already have a getStaticType() member function, it will give the wrong results:

class MouseDoubleClickEvent: public MousePressEvent {
    …
    EVENT_TYPE_FUNCTION(MouseDoubleClick);
};

MousePressEvent* event = new MouseDoubleCLickEvent(…);
EventType type = event.getStaticType(); // MousePress instead of MouseDoubleClick

If you don't want to allow further inheritance (you should use the final keyword for that), then I see little reason to have a getStaticType() member function; only the most derived classes will have that member function, so you can only call it if you already have a pointer-to-most-derived-class, and then you already know the type.

For getTypeName() you can have the base class Event get the derived class's type and convert it to a name, if you have some way to map EventTypes to strings:

static std::map<EventType, sd::string> eventNames = {
    {EventType::None, "None"},
    …
};

class Event {
    …
    std::string getTypeName() const {
        return eventNames[getType()];
    }
};

There are other ways to handle this. Instead of a getType() you could have a getDescription() virtual member function that returns both an EventType and a std::string, and have non-virtual member functions getType() and getTypeName() in class Event that call getDescription() and return either the type enum or the string. Then in derived classes you'd write something like:

struct EventDescription {
    EventType type;
    std::string name;
};

class NoneEvent: public Event
{
    EventDescription getDescription() const override {
        return {EventType::None, "None"};
    }
};

Another possibility is to use a helper template class:

template<EventType T>
class NamedEvent: public Event
{
    EventType getType() const override { return T; }
    std::string getTypeName() const override { return eventNames[T]; }
};

class NoneEvent: public NamedEvent<EventType::None>
{
};

Prefer std::unique_ptr over std::shared_ptr

You use std::shared_ptr in your code, but I don't think you actually need shared ownership semantics. Consider writing the event queue like so:

class EventQueue
{
public:
    void push(std::unique_ptr<Event> event)
    {
        eventQueue.push(std::move(event));
    }

    std::unique_ptr<Event> pop()
    {
        if (eventQueue.empty()
            return nullptr;

        std::unique_ptr<Event> event = std::move(eventQueue.front());
        eventQueue.pop();
        return event;
    }
    …
private:
    std::queue<std::unique_ptr<Event> event;
};

Note how the argument to push() is passed by value. The caller thus has to std::move() an existing std::unique_ptr<Event> as well, which makes the transfer of ownership explicit.

Note that if you dispatch an event to multiple listeners, then you don't give the listeners a smart pointer to the event, but instead just give them a const reference to the Event itself. This means they don't get (shared) ownership, they just get to borrow the event for the duration of the callback:

template<typename EventType>
class EventListener : public BaseEventListener
{
public:
    using EventCallBackFn = std::function<void(const EventType&)>;
    …
    void dispatchEvent(const Event& event) const override
    {
        callBack(static_cast<const EventType&>(event));
    }
    …
}

Listener IDs

The system you have for listener IDs will probably work fine in practice if you have only a limited number of listeners subscribed. However, consider that if you regularly subscribe new listeners, listenerID might wrap around and cause problems.

I would also avoid returning the listener ID as an int. Instead, create a class ListenerID with a private member variable holding the ID number, to which only EventListenerRegister has access. That makes it easier to change the ID type later, and also adds some type safety: consider that with implicit conversions, one might accidentily store the ID in a short or char.

Make it easier to push events

Currently you have to create a std::shared_ptr<Event> before you can pass it to pushEvent(). You could simplify it by writing something like:

class EventManager
{
    …
    template<typename T, typename... Args>
    requires std::derived_from<T, Event> // C++20
    void pushEvent(Args&&... args) {
        eventQueue.push(std::make_unique<T>(std::forward<Args>(args)...));
    }
    …
}

This allows you to push an event like so:

EventManager eventManager;
…
eventManager.pushEvent<MouseMoveEvent>(1, 2);

Unnecessary use of const

You have lots of functions that return const values. That makes no sense; the caller can just copy the return value into a non-const variable. It only makes sense to return something const if it is a pointer or reference. I even see some const void, which makes no sense at all.

You can use more auto

You can avoid some repetition by creating type aliases, which you already did. But sometimes it is even better to not have to specify types at all, for example by using more auto:

class EventListenerRegister
{
    …
    auto begin()       { return listeners.begin(); }
    auto begin() const { return listeners.begin(); }
    auto end()         { return listeners.end();   }
    auto end() const   { return listeners.end();   }
    …
};

Templates and source files

Although I am aware that .cpp files should be used, there are several objects in this codebase that either are or contain templated methods. Can you elaborate on how this should be handled?

It's considered fine to have some code in header files. However, it depends on how it is going to be used. If everything is compiled together, there is no problem. If you want to create a shared library of your event system, then consider that someone can upgrade the shared library separately from the binary that links with it. If the new version of the shared library requires different code in the templates, then that will create a problem: the binary doesn't know this and will probably do the wrong thing, leading to crashes, or worse: unexpected behavior.

You can in fact move the definition of templated functions into source files. However, you then have to tell the compiler to explicitly instantiate those templates. That also means you have to know all the types you need to instantiate up front, which sometimes is not possible.

Answer 2

General Observations

This code review request does not present any C++ source files (.cpp files). It appears that you are trying to write close code without the .cpp files, this is generally a bad idea. Having .cpp files allows the code to be maintained without altering the interfaces. This allows bug fixing to be less painful, not all the code needs to recompile in a build.

It isn't clear that any of this code is necessary, much of this code might be handled by user interface libraries such as QT. The events you really need to worry about are in the games themselves. If you want to have a wrapper so you can write the game code that will port to multiple user interface systems this might be a start, but it isn't necessary.

Include Guards

The first 2 header files presented (Event.h and KeyboardEvent.h) are missing the #pragma once directives. This means that this code can be included multiple times and that may cause linking errors due to multiply define objects. At least one header file includes multiple files that include Event.h (EventListener.h).

My person preference is to use the older style of include guards because it is more portable:

#ifndef EVENTLISTENER_H_
#define EVENTLISTENER_H_

#include <functional>

#include "MouseEvent.h"
#include "KeyboardEvent.h"
#include "WindowEvent.h"

class BaseEventListener
{
public:
    virtual ~BaseEventListener() = default;
    virtual const void dispatchEvent(const std::shared_ptr<Event>&) const = 0;
    virtual bool isEventType(const std::shared_ptr<Event>& event) const = 0;
    virtual const int getID() const = 0;
};

template<typename EventType>
class EventListener : public BaseEventListener
{
public:
    using EventCallBackFn = std::function<void(const std::shared_ptr<EventType>&)>;

    explicit EventListener(const EventCallBackFn& callBack, const int id) : callBack(callBack), id(id) { }
    const void dispatchEvent(const std::shared_ptr<Event>& event) const override { callBack(std::static_pointer_cast<EventType>(event)); }

    bool isEventType(const std::shared_ptr<Event>& event) const override { return std::dynamic_pointer_cast<EventType>(event) != nullptr; }

    const int getID() const override { return id; }

private:
    const EventCallBackFn callBack;
    const int id;
};

#endif // EVENTLISTENER_H_

Macros in C++

The use of macros in C++ should be avoided, they are not type safe. Code such as the following should be re-written to use templates:

#define EVENT_TYPE_FUNCTION(type)                                                                           \
static const EventType getStaticType() { return EventType::##type; }                                        \
virtual const EventType getType() const override { return EventType::##type; }                              \
virtual const std::string getTypeName() const override { return #type; }

#define EVENT_CATEGORY_FUNCTION(category)                                                                   \
virtual ~category##Event() override = default;                                                              \
virtual const EventCategory getCategory() const override { return EventCategory::##category; }              \
virtual const EventType getType() const override{ return EventType::None; }

The EventType Enum

Rather than have one enum for the EventType it might be better to have one for each EventCategory.