I've been trying to decide how to format my event messaging system for a while now, but could never settle on a way to do it. My goals for this system were to make it fast, intuitive, and easily expandable (meaning I can easily add listeners for whatever event type I want). I ended up settling on the observer pattern to handle the way in which objects get informed of an event because it seems fast, but more so because it conforms to the design pattern of the rest of my engine.

So here is what I have. To me, it still seems like a strange way of doing it, as it seems template heavy, but it has performed well in my tests and is pretty easy to use. I would appreciate any suggestions, criticisms, improvements, other designs, etc.

Event classes:

// Description: All events derive from this base class. This can be used to
// store events of different types together.
//
// Inheritance: None
//
// Example Usage:
// std::vector<EventBase*> events;
// events.push_back(new KeyDownEvent());
// events.push_back(new KeyUpEvent());
class EventBase {
public:
// ctors/dtor: =============================================================
EventBase() : handled_(false) {}
virtual ~EventBase() {}

// public functions: =======================================================
void SetHandled(bool handled) { handled_ = handled; }
bool IsHandled() const { return handled_; }

protected:
// protected static functions: =============================================
static unsigned int GenerateUniqueID();

private:
// vars: ===================================================================
bool handled_;
};

unsigned int EventBase::GenerateUniqueID() {
static unsigned int id = 0;
++id;
return id;
}

// Description: Generates a unique integer ID for an event that derives from
// this class. All concrete events should derive from this class.
//
// Inheritance: EventBase -> Event<EventT>
//
// Example Usage:
// class SomeEvent : public Event<SomeEvent> {
// public:
// SomeEvent(int data) : data_(data) {}
//
// void SetData(int data) { data_ = data; }
// int GetData() const { return data_; }
//
// private:
// int data_;
// };
template<typename EventT>
class Event : public EventBase {
public:
// public static constants: ================================================
static const unsigned int kID;
};

template<typename EventT>
const unsigned int Event<EventT>::kID = Event<EventT>::GenerateUniqueID();

Listener classes:

// Description: All event listeners derive from this base class. This can be
// used to store listeners of different types together.
//
// Inheritance: None
//
// Example Usage:
// std::vector<EventListenerBase_Interface*> listeners;
// listeners.push_back(new KeyDownListener());
// listeners.push_back(new KeyUpListener());
class EventListenerBase_Interface {
public:
// ctors/dtor: =============================================================
virtual ~EventListenerBase_Interface() {}
};

// Description: Generates a unique OnEvent method for each event input for
// EventT. All event listeners should derived from this class.
//
// Inheritance: EventListenerBase_Interface -> EventListener_Interface<EventT>
//
// Example Usage:
// class SomeEventListener
// : public EventListener_Interface<KeyDownEvent>,
// public EventListener_Interface<KeyUpEvent> {
// public:
// SomeEventListener() {}
//
// void OnEvent(KeyDownEvent* event) {
// // do something
// }
//
// void OnEvent(KeyUpEvent* event) {
// // do something
// }
// };
template<typename EventT>
class EventListener_Interface : public EventListenerBase_Interface {
public:
// dtors: ==================================================================
virtual ~EventListener_Interface() {}

// public functions: =======================================================
virtual void OnEvent(EventT* event) = 0;
};

EventDispatcher:


// Description: Used to register events listeners and dispatch events.
//
// Inheritance: None
//
// Example Usage:
// EventDispatcher dispatcher;
// SomeListener listener;
//
// dispatcher.Register<SomeEvent>(SomeEvent::kID, &listener);
// dispatcher.Dispatch(SomeEvent::kID, new SomeEvent());
class EventDispatcher {
public:
// typedefs: ===============================================================
typedef std::vector<EventListenerBase_Interface*> ListenerVector;
typedef std::map<int, ListenerVector> ListenerMap;

// ctors/dtor: =============================================================
EventDispatcher() {}

// public functions: =======================================================
// - dispatch event
template<typename EventT>
void Dispatch(int type, EventT* event);

// - register listeners
template<typename EventT>
void Register(int type, EventListener_Interface<EventT>* listener);

private:
// vars: ===================================================================
ListenerMap listeners_;
};

template<typename EventT>
void EventDispatcher::Dispatch(int type, EventT* event) {
if (event == NULL) { return; } // disallow a NULL event

ListenerMap::iterator itr = listeners_.find(type);
if (itr == listeners_.end()) { return; } // event type not found

// dispatch event to the listeners
for (unsigned int x = 0; x < itr->second.size(); ++x) {
static_cast<EventListener_Interface<EventT>*>(
itr->second.at(x))->OnEvent(event);
}
}

template<typename EventT>
void EventDispatcher::Register(int type, EventListener_Interface<EventT>* listener) {
if (listener == NULL) { return; } // disallow a NULL listener
listeners_[type].push_back(listener);

}

Again, feedback would be greatly appreciated. I would prefer not to use RTTI, but I'm open to suggests.

Thanks.

EDIT: De-virtualized SetHandled and IsHandled in EventBase and removed the ctor and dtor for Event at Ftn's suggestion.
EDIT: Altered Register in EventDispatcher in the way __sprite suggested.

I have only some nitpicking that could possible simplify the public interfaces.

These does not need to be virtual. I think it would be simpler if they weren't.

virtual void SetHandled(bool handled) { handled_ = handled; }
virtual bool IsHandled() const { return handled_; }


Unnecessary code.

Event() {}
virtual ~Event() {}


I find the "Interface" word in the class names redundant.
If the event parameter in EventListener would be a reference, you could omit all checks for null event.
Likewise in EventDispatcher Register method, if the listener parameter would be a reference, it would indicate to the user that the EventDispatcher will not call delete to given listeners.

These does not need to be virtual. I think it would be simpler if they weren't.

virtual void SetHandled(bool handled) { handled_ = handled; }
virtual bool IsHandled() const { return handled_; }


You're absolutely right. Those functions are no longer virtual.

Unnecessary code.

Event() {}
virtual ~Event() {}


Right again, the constructor and destructor have been removed. I typically write a virtual destructor for any class with virtual functions, but, since this one doesn't have any, it's not necessary.

I find the "Interface" word in the class names redundant.

I agree, but I believe in making code as explicit as possible. I generally only use multiple inheritance when at most one of the base classes has implementation and all others are pure interface classes, so I tag pure interface classes with the "Interface" suffix to make it easier to keep track of that policy.

If the event parameter in EventListener would be a reference, you could omit all checks for null event.
Likewise in EventDispatcher Register method, if the listener parameter would be a reference, it would indicate to the user that the EventDispatcher will not call delete to given listeners.

My problem with references as parameters is that references act like values in syntax but behave like pointers when it comes to semantics. On top of that, it goes back to my want for code to be explicit; If I call Register<SomeEvent>(SomeEvent::kID, listener) it is not obvious that the function can change listener or keep track of it via references or pointers. In fact, I would assume, if I only looked at that line of code, that it would either be passed as a constant reference or copied and, therefore, could not be manipulated. Now, if I call Register<SomeEvent>(SomeEvent::kID, &listener) it is clear to me that listener can be manipulated. With that in mind, I wanted the OnEvent function to be able to change the event passed into it so it could set it as handled if need be. If I'm not using reference parameters, pointers are the only way left. That's my take on it anyway.

Thanks for your suggestions.

I think your Register function can be simplified to just this?



template<typename EventT>
void EventDispatcher::Register(int type, EventListener_Interface<EventT>* listener) {
if (listener == NULL) { return; } // disallow a NULL listener

// I think this is equivalent to what you're doing atm... (insert default constructed item if the thing isn't there)
listeners_[type].second.push_back(listener);
}


If you want to make sure your listeners are unique, you could use a set instead of a vector. That might be faster than checking if something's already registered yourself (the commented out code).

Personally I'd use references instead of pointers. I don't really see how pointers are clearer -> you have to check that they're valid everywhere, and if you're looking up the parameter (because you don't know that it's a reference / pointer already), you'd immediately see that it's a non-const reference anyway.

Also, I'm curious to know what "handled" actually gives you. I'm not sure that things passing in the event should care about whether the event is handled or not (and we can't tell whether that's by one listener or more than one). Do you have a specific use in mind for it?

I think your Register function can be simplified to just this?

Aha! Thanks, I wasn't aware that map::operator[] inserted a new value for that key if the key wasn't found. That makes that function a hell of a lot smaller and a tiny bit faster.

If you want to make sure your listeners are unique, you could use a set instead of a vector. That might be faster than checking if something's already registered yourself (the commented out code).

Your right, but it also makes dispatching the events a much slower. I tried iterating through a set instead of a vector to dispatch events and it was almost four times slower in many separate tests. Since EventDispatcher should be dispatching events much more often than registering listeners, it is probably best I use vectors and just allow duplicates. Of course, as long as I'm paying attention I shouldn't be registering a listener more than once anyway.

Personally I'd use references instead of pointers. I don't really see how pointers are clearer -> you have to check that they're valid everywhere, and if you're looking up the parameter (because you don't know that it's a reference / pointer already), you'd immediately see that it's a non-const reference anyway.

Yeah, I think it's just a person preference thing for me really. That and I tend to follow the Google C++ Style Guide and it frowns on reference parameters.

Also, I'm curious to know what "handled" actually gives you. I'm not sure that things passing in the event should care about whether the event is handled or not (and we can't tell whether that's by one listener or more than one). Do you have a specific use in mind for it?

I have an idea in mind that I have not implemented as of the time I'm writing this. Basically, I want OnEvent to have the option to set the event as being handled. If it does this, I would have a check in the DispatchEvent function that would break out of the loop if the event was handled, preventing it from being passed to other listeners. It's kind of a work in progress for me at the moment. Don't know if it is going to be in the final version or not.

Thanks for your suggestions. I was really over thinking my register function .

You might want to look into the boost implementation of the signal&slot concept.
Apart from that, on what level do you need communication? Are your entities(/components) communicating with each other or are unique compile-time-known systems sending and receiving messages?

You might want to look into the boost implementation of the signal&slot concept.
Apart from that, on what level do you need communication? Are your entities(/components) communicating with each other or are unique compile-time-known systems sending and receiving messages?

Well, that's a good question. At the moment I'm only theorizing about the practical applications of this system, so I don't have a specific answer other than to say it's probably more the latter. I know that I'm going to have a few systems set up for general use that use an event dispatcher, such as a SDL_Event dispatcher that will poll events in the main loop and dispatch to listeners that need information about key presses, mouse clicks, etc. Another example would be a central game event system where listeners could trigger an event that gets sent to an event dispatcher and dispatched back out to all the listeners (Player triggers AlarmEvent -> AlarmEvent dispatched through EventDispatcher to all Players -> Players handle AlarmEvent). As for listeners/entities communicating with each other, that's not really the intent of this system, unless you were referring to something like one of the previous examples.

I asked because I've been working on a framework for inter system communication that dispatches messages with constant time overhead, independent from the number of listeners. Unfortunately, it needs a good c++11 compiler (dependency on variadic template arguments and SFINAE) and still a lot of testing.
Here is my concept:
I put all my systems in a special container, e.g.


system_manager< SystemA , SystemB , ... , SystemX > manager;

Each system can derive from a 'sending' base, where the system has to specify which messages it wants to send, e.g.


class SystemA
: public sending< MessageA , MessageB >
{
void f()
{
send( MA() );
}
void handle( const MessageX &)
{
//code for handling MessageX and all messages derived from or convertible to MessageX
}
};



In system_manager I then check which messages each system has to send (with template meta programming), overwrite a virtual function void send( const MessageType &) for each MessageType that dispatches the message to all systems in the manager at the same time (with ordinary function calls & SFINAE, so with compiler optimizations pretty much just 1 virtual function call to send messages to any number of systems).
If you want to actually make a game I don't recommend implementing that though, it took me ages to make it work.

EDIT: Just talked about me, sry about that.
I guess your system will work the way you posted it. But why do you think you need a centralized message system? Both the sender and the receiver have to know which message they want to send/listen to, so it should be easy to have a special dispatcher for each event. That way, both registering and dispatching will be faster, because you don't have to do a map lookup.

EDIT: Just talked about me, sry about that.

No worries, I like to see other peoples' designs too and it's one of the things I ask for in my original post .

I guess your system will work the way you posted it. But why do you think you need a centralized message system? Both the sender and the receiver have to know which message they want to send/listen to, so it should be easy to have a special dispatcher for each event. That way, both registering and dispatching will be faster, because you don't have to do a map lookup.

You have a point, but this system came about as a result of having to write multiple classes and/or functions for each event class/structure that I wrote. I wanted a generic dispatcher that could handle any number of events, event types, and listeners, but that is not to say it is necessary. Also, I may have overplayed the "centralized" part a bit. I don't mean to have only one EventDispatcher that every listener registers to, but rather to inherit from EventDispatcher for specific event groups, e.g. SDLEventDispatcher to dispatch SDL events (SDL_KEYDOWN, SDL_MOUSEBUTTONDOWN, SDL_QUIT, etc.) or PlayerEventDispatcher to dispatch player events (player moved, player took damage, player died, etc.). Creating a special dispatcher for each event seems like overkill to me because SDL alone has at least 16 different event types, not to mention any I come up with. That's 16 different dispatchers at the very least that I would have to instantiate and maintain, whereas a centralized one for SDL events would do the same and probably wouldn't be that worse off due to the map lookup.

If your compiler supports variadic templates and you are ok to have all message types compiled into your dispatcher you could do something like this (just a demo):



template< class MessageType >
class Handler
{
public:
virtual void handle( const MessageType & ) = 0 ;
};

template< class ...MessageTypes >
class Dispatcher;

template<>
class Dispatcher<>
{
public:
void dispatch();
void add_listener();
//these functions are just dummies to be able to do 'using Base::dispatch' in derived classes
};

template< class Head , class ...Tail >
class Dispatcher< Head , Tail... >
: public Dispatcher< Tail... >
{
private:
std::vector< Handler< Head >* > handlers;
public:
void dispatch( const Head &Message )
{
//iterate over all handlers and call 'handle'
}
using Dispatcher< Tail... >::dispatch; //to avoid name hiding
void add_listener( Handler< Head > &HeadHandler )
{
//add HeadHandler to the vector of handlers
}
using Dispatcher< Tail... >::add_listener;
};

//using the Dispatcher:

typedef Dispatcher< MessageA , MessageB , MessageC > SDL_Events;



This would add the dependency on variadic templates (I know gcc supports it, but msvc++ 2010 not, for example).
In addition, you have to recompile all modules that depend on SDL_Events if you want to add new messages.
On the other hand, you save the map lookup. It depends on your requirements how much performance you are willing
to sacrifize for modularity.