4
\$\begingroup\$

I am currently working on the implementation of Event System on a game engine. I have zero experience in Event System implementation or the patterns that are used. What I have done is what I've seen online. This is also the first time I am posting here, so apologies if I've accidentally violated any rules for posting.

Currently the system is capable sending event using EventDispatcher, while EventListener is capable of receiving those events and pass to a function bounded to it. My current focus is to make sure my implementations are exception safe (as much as possible).

Here's the general task of each class:

  • EventDispatcher: Dispatch events to the list of EventListeners.
  • EventListener: Receives notification from EventDispatcher and calls a function stored in EventDispatcher.
  • EventHandler: The middle man. Keeps track of a list of EventDispatchers and their attached EventListeners.

Concern

My biggest concern was that EventListener and EventDispatcher is stored in side other classes as member variables. For example, EventListener can be stored inside an AI class to listen from the dispatcher in Player class. Trouble comes when the AI is destructed, taking out the EventListener with it. Therefore, EventDispatcher would be accessing deleted memory.

My solution

So, I created EventHandler to keep track of the EventDispatchers and their EventListeners. If a EventDispatcher is destructed, it will remove all the EventListeners associated with it. If a EventListener is destructed, it will detach itself from the EventDispatcher and EventHandler.

Questions

  1. Should I do away EventHandler and have EventListener to keep a pointer to EventDispatcher, and move all the functions and list of EventListener to EventDispatcher?
  2. Other than EventHandler being deleted (not very concern of this at the moment), would there still be memory leaked or invalid data access?
  3. When should I delete the fired events? And who should be responsible for it?

Code sample

I would appreciate if anyone could answer my questions or points out any flaws in my implementation as I am really unsure what would be the underlying issues.

Assuming I have all the necessary class and libraries header, here are my implementations. Also the implementations are likely not following Rule of 3/5 as I need to get the basics to work first.

EventListener.cpp

using FuncCallback = std::function<void(Event*)>;

EventListener::EventListener()
    :handler{ nullptr } {}

EventListener::EventListener(EventHandler *h)
    :handler{ h } {}

// Detach from the handler
EventListener::~EventListener()
{
    if (handler)
        handler->RemoveListener(this);
}

void EventListener::AttachHandler(EventHandler *h)
{
    // Removes previously bounded handler
    if (handler && (handler != h))
        handler->RemoveListener(this);

    handler = h;
}

void EventListener::DetachHandler()
{
    handler = nullptr;
}

void EventListener::Bind(EventListener::FuncCallback f)
{
    func = f;
}

void EventListener::OnNotify(Event *e)
{
    func(e);
}

EventDispatcher.cpp

EventDispatcher::~EventDispatcher()
{
    handler.ClearDispatcher(this);
}

void EventDispatcher::MessageListener(std::list<EventListener*>& listeners, Event *e)
{
    // Dispatch events to the listeners
    for (EventListener* listener : listeners)
    {
        listener->OnNotify(e);
    }
}

EventHandler.cpp

void EventHandler::RegisterListener(EventDispatcher *d, EventListener *l)
{
    // Add to the dispatcher
    dl_map[d].push_back(l);

    // Pass handler to listener
    l->AttachHandler(this);
}

void EventHandler::RemoveListener(EventListener *l)
{
    for (auto& key_it : dl_map)
    {
        // Get the listeners
        auto& list = key_it.second;

        // Find the specific listener
        auto val_it = std::find(list.begin(), list.end(), l);

        // Remove the listener from the list, if found
        if (val_it != list.end())
        {
            list.erase(val_it);
            break;
        }
    }
}

void EventHandler::ClearDispatcher(EventDispatcher *d)
{
    auto& it = dl_map.find(d);
    if (it != dl_map.end())
    {
        for (auto& listener : it->second)
        {
            listener->DetachHandler();
        }
    }
}    

void EventHandler::QueueEvent(Event *e)
{
    waitQueue.push_back(e);
}

void EventHandler::FireAllQueuedEvents()
{
    std::cout << "Firing events" << std::endl;
    activeQueue.splice(activeQueue.begin(), waitQueue);

    for (Event *e : activeQueue)
    {
        // Send event to each dispatcher
        for (auto& key_it : dl_map)
        {
            // Dispatcher send messages to listeners
            auto& dispatcher = key_it.first;
            dispatcher->MessageListener(key_it.second, e);
        }
    }

    // TEMP, looking for a way to improve this
    for (Event *e : activeQueue)
        delete e;

    activeQueue.clear();
}

Sample usage:

struct Position
{
    float x, y, z;
};

struct AI
{
    EventListener movement_listener;

    AI (EventHandler *h)
        :movement_listener{ h }
    {
        movement_listener.Bind(std::bind(&AI::HandleMovement, this, std::placeholders::_1));
    }

    void HandleMovement(Event *e)
    {
        Position* pos = static_cast<Position*>(e->GetData());
        std::cout << "Position: " << pos->x << ", " << pos->y << ", " << pos->z << std::endl;
    }
};

int main(){
    EventHandler AI_EVENT_HANDLER;

    // Whenever an AI moves, dispatch the event with the movement informations
    EventDispatcher ai_movement_dispatcher;


    AI normal_ai{ &AI_EVENT_HANDLER };

    EventListener independent_listener{ &AI_EVENT_HANDLER };
    independent_listener.Bind(
        [](Event*e){
            Position* pos = static_cast<Position*>(e->GetData());
            std::cout << "Position: " << pos->x << ", " << pos->y << ", " << pos->z << std::endl;
        }
    );

    AI_EVENT_HANDLER.RegisterListener(&ai_movement_dispatcher, &normal_ai.movement_listener);
    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{1,2,3} });
    AI_EVENT_HANDLER.FireAllQueuedEvents();

    AI_EVENT_HANDLER.RegisterListener(&ai_movement_dispatcher, &independent_listener);
    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 1,2,3 } });
    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 3,4,5 } });
    AI_EVENT_HANDLER.FireAllQueuedEvents();

    AI_EVENT_HANDLER.RemoveListener(&normal_ai.movement_listener);
    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 1,2,3 } });
    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 3,4,5 } });
    AI_EVENT_HANDLER.FireAllQueuedEvents();

    {
        AI oos_ai{ &AI_EVENT_HANDLER };
        AI_EVENT_HANDLER.RegisterListener(&ai_movement_dispatcher, &oos_ai.movement_listener);
        AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 1,2,3 } });
        AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 3,4,5 } });
        AI_EVENT_HANDLER.FireAllQueuedEvents();
    } // oos_ai out of scope, therefore triggers the destructor and detach itself from the handler

    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 1,2,3 } });
    AI_EVENT_HANDLER.QueueEvent(new Event{ new Position{ 3,4,5 } });
    AI_EVENT_HANDLER.FireAllQueuedEvents();

Sample output

Firing event
Position: 1, 2, 3 // normal_ai
Firing event      // added independent_listener
Position: 1, 2, 3 // normal_ai
Position: 1, 2, 3 // independent_listener
Position: 3, 4, 5 // normal_ai
Position: 3, 4, 5 // independent_listener
Firing event      // normal_ai removed at this point
Position: 1, 2, 3 // independent_listener
Position: 3, 4, 5 // independent_listener
Firing event      // added oos_ai within another scope
Position: 1, 2, 3 // independent_listener
Position: 1, 2, 3 // oos_ai 
Position: 3, 4, 5 // independent_listener
Position: 3, 4, 5 // oos_ai 
Firing event      // oos_ai removed as it went out of scope
Position: 1, 2, 3 // independent_listener
Position: 1, 2, 3 // independent_listener
\$\endgroup\$
  • \$\begingroup\$ Would be nice to also have the h/hpp-files. \$\endgroup\$ – Darius Duesentrieb Sep 11 '18 at 22:49
  • \$\begingroup\$ Also, how does the Event-type look like? Would AI_EVENT_HANDLER.QueueEvent(new Event{ new Velocity{ 1,2,4 } }) compile provided that a Velocity-struct exists? If the Event-type stores the data just as void* then you might get bugs that could be avoided by making your event system typesafe. For example, if an EventHandler only handles one kind of types then you could make a template class template<typename T> EventHandler. ThenEventListener and EventDispatcher should be template-classes as well. If you want a short code example I could write one. \$\endgroup\$ – Darius Duesentrieb Sep 11 '18 at 23:11
  • \$\begingroup\$ @DariusDuesentrieb I might add in the header files later on, one I'm done with a revised version. Although, the revised version is much more different than what I have posted here and I wasn't sure whether to change everything here or make a new post. \$\endgroup\$ – Kenny Wong KennyDBacon Sep 12 '18 at 17:46
  • \$\begingroup\$ And yes, we can pass in any class types as long as they exist. I didn't use template because we are aiming to store EventDispatcher or EventListener in one single list, and templated version of those wouldn't be possible as far as I know. Hence we decided to try void*. Likely I would have to re-think my approach in this matter, since we sacrifice type-safe for convinience. I would likely make my Event class know what type it held with something like using type = T so that the Event could carry the type to cast it later on. \$\endgroup\$ – Kenny Wong KennyDBacon Sep 12 '18 at 17:56

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.