   Event System, is this a decent design?
  Iv had lots of troubles coming up with a good messaging design for event throughout my game, iv come to this conclusion, obviously if it works for my game, its all good, but i thought id get some feedback if its a decent design.I start off with a CEvent object. This is going to be a class with various variables that can be populated, it must have an EventName, but everythign else will be optional. class CEvent{    public:   std::string GetName(){return m_EventName;}   void SetName(std::string Name){m_EventName=Name;}    private:   std::string m_EventName;};Basically the event manager has a vector of events. And a map that ties event to listeners.std::map<std::string,sigslot::signal1<CEvent> > m_Listeners;std::vector<CEvent> m_Events;It also has functions for creating listeners (using the sigslot library for the signals and slots) and posting events.template<class desttype>void AddListener(std::string EventName,desttype* pclass,void (desttype::*pmemfun)(CEvent))void PostEvent(CEvent);Basically you can throw any EventName you want, as long as there is a listener it will emit the signal to all its listeners and send along the CEvent.My input manager allows you to bind EventNames to keys, so a simple quit setup might work like this. I set this up in my CGame class, which might need some rework, but thats a whole different issue. :)InputManager.BindKeyEvent(257,"QuitGame"); //escape key in GLFWEventManager.AddListener("QuitGame",this,&CGame::Quit);I think this will work, but i wondered if anyone sees a problem later down the road.EDIT:Forgot to mention the HandleEvents function in my main loop. This is run each frame.void CEventManager::HandleEvents(){    while(!m_Events.empty())    {   m_Listeners[m_Events.back().GetName()].emit(m_Events.back());   m_Events.pop_back();    }}
  If I forget the ugly C in front of the class names, then it sounds ok. Maybe this is a personal thing, but I'm not a fan of run time errors, which can be introduced likely (wrong name used). Of course you can throw an exception, but in my opinion, when there is a predefined set of events, wouldn't a system which's events are known at compile time make more sense?   ;  Yeah that might be a problem, but im not sure how to do it. I want to be able to make many different event names, like "PlayerMoveRight" "PlayerJump" etc. without having to hard code every event. Using strings i could define them in a text file.Although its unlikely to cause any errors, i can check for the existence of a key in the map before emitting the event.   ;  Your event system only has one "flaw" (as far as I can see) which is the use of strings.String compare will always be slower than int compare, and in a system that potentially compares hundreds of times per frame 30-60+ frames per second you could be taking a hit on performance.I would use enums defined in a commons header file or something. 1 for event names and 1 for systems.using Enums would save you space and run faster which is always good.Other than that it looks good and I wouldnt worry about "(wrong name used)" because, strings or not it can still happen (ie sending out of bounds number to reference an event that isnt there).   ;  A few ideas.Use const refs:std::string const& GetName() const {return m_EventName;}void SetName(std::string const& Name){m_EventName=Name;}If you need params:protected:CEventParams m_Params;CEventParams const& GetParams() const {return m_Params;}void SetParams(CEventParams const& params) { m_Params = params;}I you want to be able to subclass CEvent:virtual void ~CEvent() {}Quote:Original post by agaudreauYour event system only has one "flaw" (as far as I can see) which is the use of strings.String compare will always be slower than int compare, and in a system that potentially compares hundreds of times per frame 30-60+ frames per second you could be taking a hit on performance.I would use enums defined in a commons header file or something. 1 for event names and 1 for systems.using Enums would save you space and run faster which is always good.The suggestion has merit, but the disadvantage that the implementation is less flexible. In most cases you can use short strings with the confidence that it won't impair performance. (If you really want to optimize, use 4 or 8 character strings stored in ints. Still relatively mnemonic and easy though less flexible.)In this case it depends on the number of events fired per frame. It's guesswork what I say here, but I bet even with 10000 events per frame you won't need to steer clear from strings (but maybe start to use hash_map instead of map).   ;  thanks for the replies, ill see about making some of these changes. Quote:If you need params:protected:CEventParams m_Params;CEventParams const& GetParams() const {return m_Params;}void SetParams(CEventParams const& params) { m_Params = params;}Why separate the params from the CEvent? I guess i could have differing params subclasses, which could be useful, but then id get into needing to cast them to what i need to use.   ;Quote:Original post by schragnasherWhy separate the params from the CEvent? I guess i could have differing params subclasses, which could be useful, but then id get into needing to cast them to what i need to use.Depends on how far you want to take this. You could just typedef list<string>  CEventParams or list<pair<string, string> > CEventParams or make a more complex params class. In any case, you'll need to have your event receiver get access to event params somehow. This would be one (simplistic) way.;  I'm really not a fan of a centralized "event manager" which sticks all the events into one sprawling, inefficient container and then searches through for the one or two listeners out of thousands which actually give a damn about a given message. C++ gives you as many variables as you want. USE THEM! If you need an event that can be fired and listened to, make an event. You can put all the events in a special namespace if that makes things feel tidier to you. You'll get faster access, compile-time checking of event parameter types, better debuggability, and better separation of concerns.   ;Quote:Original post by SneftelI'm really not a fan of a centralized "event manager" which sticks all the events into one sprawling, inefficient container and then searches through for the one or two listeners out of thousands which actually give a damn about a given message. Straw man. Events can easily be placed in a hash container with O(1) lookup.Quote:C++ gives you as many variables as you want. USE THEM! If you need an event that can be fired and listened to, make an event. You can put all the events in a special namespace if that makes things feel tidier to you. You'll get faster access, compile-time checking of event parameter types, better debuggability, and better separation of concerns.I'm using MVC + an event manager to proxy the observer pattern between Views and Models and Views and Controllers. The event manager is an injected dependency for views and controllers. I have some tricks that pack parameters into std::vector<boost::any> that serves as an arguments parameter for all event handlers. Having to read them out in every handler looks kludgy and I don't get compile-time type checking on the arguments. I'll have to try this out.;Quote:Original post by aaron_dsStraw man. Events can easily be placed in a hash container with O(1) lookup.Asymptotic time complexity is not the only measure of efficiency. Show me a hash table with the same speed and memory usage as direct variable access. :)Quote:I'm using MVC + an event manager to proxy the observer pattern between Views and Models and Views and Controllers.There's nothing wrong with this approach, and it can't be beaten for ease of extension, but yeah, I'd argue that injecting a structure of listeners instead would give you greater code clarity. Whenever I find myself reaching for boost::any, I think to myself "If what I want is dynamic typing, why am I using a statically typed language?" [grin]   