using orxEvent_AddHandler in a header/class

edited January 2015 in Help request
I'm having a bit of trouble adding my event handlers when they've been moved to c++ styled classes and headers, for OO reuse.

For example, the following is fine, if you set everything up in your main project .cpp file:

orxSTATUS orxFASTCALL EventHandler(const orxEVENT *_pstEvent) {
    //stuff
}

orxSTATUS orxFASTCALL Init(){
    orxEvent_AddHandler(orxEVENT_TYPE_ANIM, EventHandler);
}

And that works fine.

Say for instance you try to move the code into a header and class files so that everything is declared and initialised separately:

mystuff.h
=========
#ifndef MYSTUFF_H
#define MYSTUFF_H

class MyStuff
{
public:
    MyStuff();
    ~MyStuff();
    
    orxSTATUS orxFASTCALL EventHandler(const orxEVENT *_pstEvent);
}

#endif // MYSTUFF_H

mystuff.cpp
===========
void MyStuff::MyStuff()
{
    orxEvent_AddHandler(orxEVENT_TYPE_ANIM, EventHandler);  <------ error
}

orxSTATUS orxFASTCALL MyStuff::EventHandler(const orxEVENT *_pstEvent) {
    //stuff
}



So the declaration of the EventHandler function is fine, the compiler is happy with that. But it is not happy with the fact that I tried to use it with orxEvent_AddHandler. The error is:
C:/Work/project/mystuff.cpp:45:54: error: cannot convert 'MyStuff::EventHandler' from type 'orxSTATUS (MyStuff:: )(const orxEVENT*) {aka __orxSTATUS_t (MyStuff:: )(const __orxEVENT_t*)}' to type 'orxEVENT_HANDLER {aka __orxSTATUS_t (__attribute__((__fastcall__)) *)(const __orxEVENT_t*)}'
I'm probably missing something very simple. I've fixed up all other aspects, but adding events in the code files is really dogging me.

Comments

  • edited January 2015
    It is because you cannot pass the non-static member function pointer to orxEvent_AddHandler(). You could use static member function thought (but it wouldn't be tied to the concrete isntance).

    I think that orxEvent_AddHandler() is missing the "context" paramere where you could pass the this pointer to be able to use as member function (with some minor wrapping code, either using lambda or helper static function).
  • edited January 2015
    Hi Trigve, I think I get some of what you are saying (I'm a simple man :)). But it does give me something to read up on.
  • edited January 2015
    Trigve wrote:
    It is because you cannot pass the non-static member function pointer to orxEvent_AddHandler(). You could use static member function thought (but it wouldn't be tied to the concrete isntance).

    Please excuse my limited understanding of C++.

    What confuses me is: why is it valid for orxEvent_AddHandler() to accept EventHandler in the first example, yet in the second example, where the class and definitions are separate, it is somehow different?

    Is EventHandler in a different form between the two examples?
  • edited January 2015
    In the first example, the EventHandler() function isn't member function of the class. It is free function. There is is distinction between member function and free functions. The non-static member function could be called only "on" valid class instance.
    In short, you cannot use non-static member function as event handlers, now. If you change the function to static
    class MyStuff 
    { 
    ...    
        static orxSTATUS orxFASTCALL EventHandler(const orxEVENT *_pstEvent); 
    } 
    
    you could use it as the event handler, but will not be tied to any instance (the behaviour id the same as free function but because it is part of the class, it could access static class members).
    edit: To be it more clear, to be able to call member function, you need some class instance.
    class A
    {
    public:
      void func() {}
    };
    A a;
    a.func(); // called on instance
    
    orxEvent_AddHandler() accept only function pointer you could pass there only free function pointer or static member function pointer. If the function has addition context parameter, for instance void *, you could then write something like this
    class A
    {
    public:
      static void event_handler(const orxEVENT *_pstEvent, void *Context)
      {
        static_cast<A *>(Context)->func(_pstEvent);
      }
    
      void func(const orxEVENT *_pstEvent) {...}
    };
    A a;
    orxEvent_AddHandler(&A::event_handler, &a);
    
  • edited January 2015
    Thanks, Trigve. That makes a lot of sense. Appreciate the help with that.

    I will definitely need to tie a particular eventhander to a specific class.

    I'll experiment with your code snippets and see how I go.

    Thanks again.
  • edited January 2015
    sausage wrote:
    Thanks, Trigve. That makes a lot of sense. Appreciate the help with that.

    I will definitely need to tie a particular eventhander to a specific class.

    I'll experiment with your code snippets and see how I go.

    Thanks again.
    You're welcome.

    I really think that orxEvent_AddHandler() should have additional parameter as for instance orxClock_Register() has. Otherwise I don't think it will be possible to register the event handler per instance basis.
  • edited January 2015
    I can add a context to event handlers, however I'm curious to know in which case you'd need it (not saying they don't exist, just that right now I can't think of any that wouldn't be solved by a "singleton" approach).
  • edited January 2015
    This is for the upcoming pinball games.

    My current need is that I eventually want to have four tables in the game that the user can select to play. Most of the logic would be shared between all the tables, functions like:

    void LaunchBall();
    void DestroyBall();
    ...

    etc etc.

    Currently it seems like the best approach would be to move everything into a base class and each pinball table class inherit from the base class and just override the functions that are different. (or add ones that are new)

    But the event methods can't be used in this way.

    Perhaps the events don't need to be part of the classes. Perhaps the derived class can pass the event functions outside to the event handlers as part of it's setup.

    I am happy to try and work with a singleton approach as my current way may not be architecturally the best. I only understand c# singletons so I'll read up.
  • edited January 2015
    If there can only be a single instance of any table at a time, a singleton should suffice. You can implement a static event handler which polymorphically dispatches on the singleton instance.
    class BaseTable
    {
    private:
      static BaseTable* m_singletonInstance;
    
      BaseTable() //Private constructor
      {
        //Register the event handlers in the base constructor.
         orxEvent_AddHandler(orxEVENT_TYPE_ANIM, eventHandlerDispatcher); 
      }
    
      //The actual event handler, dispatches to the singleton instance
      static orxSTATUS orxFASTCALL eventHandlerDispatcher(const orxEVENT *_pstEvent)
      {
         return m_singletonInstance->EventHandler(_pstEvent);
      }
    
    protected:
      virtual orxSTATUS EventHandler(const orxEVENT *_pstEvent)
      {
         //Handle events here, override the virtual function in your derived classes as necessary.
      }
    
    public:
      ~BaseTable();
    
      static void createTable(enum table_type)
      {
         switch(table_type) {
         case TABLE1: m_singletonInstance = new DerivedTable1(); break;
         case TABLE2: m_singletonInstance = new DerivedTable2(); break; 
         }
        //Or however you want to handle that logic.
        //In this case, you would need to define the function after your 
        //derived classes are declared, as well as make the derived 
        //class constructors protected rather than private (I think....) 
      }
    
      static BaseTable* getTable()
      { 
        //check for null here, and etc...
        return m_singletonInstance;
      }
    
      static void deleteTable()  
      {
         if(m_singletonInstance) delete m_singletonInstance;
         m_singletonInstance = NULL;
      }  
    };
    

    The above will add an extra layer of function indirection for your events, but I doubt that it will affect performance.

    If you have multiple tables at the same time, you can switch the singleton into a container (a linked list for instance,) handle insertion/removal of tables in the base class, and instead of dispatching the event handler on a single instance, you would iterate the container and dispatch on every instance.
  • edited January 2015
    Nice to see you post here, 4babce! Your previous post was over 2 years old. ;)
  • edited January 2015
    For me, It would be nice to make the event handler per instance. I have for example the class, which is responsible for the sprite rendering/processing. I will have tenths instances of this class in my project. And I want to handle each animation event in given instance. So it would be easier if I could create event handled in the constructor of the class for given instance, than to have some singleton and then propagate the event to some children etc.
  • edited January 2015
    Thanks, 4babce for spelling all this out. I like this approach, but I'll have to study into it to ensure I understand it completely. I prefer to write it this way rather than change orx.

    You are correct that there is only one instance of a table at a time. So I'm really using inheritance for re-use, not for creating many instances of the class at once. A singleton pattern sounds ideal.
  • edited January 2015
    I've added an optional context to event handlers: https://bitbucket.org/orx/orx/commits/7ee0d1eae9a562c1ba00bad75f9b7779b5325a6d

    This is a rather straightforward change and the context will be part of the event sent (so no need for a new signature for handlers).

    That being said, Trigve, keep an eye on the performances as all the handlers will be called for every event, resulting in many more calls than if you were doing the dispatch yourself.

    Sausage: in your case, as you'll only have a single instance of the handler there won't be any additional cost, so you can either go with the context or the singleton approach, they're equivalent in your case.
  • edited January 2015
    iarwain wrote:
    I've added an optional context to event handlers: https://bitbucket.org/orx/orx/commits/7ee0d1eae9a562c1ba00bad75f9b7779b5325a6d

    This is a rather straightforward change and the context will be part of the event sent (so no need for a new signature for handlers).

    That being said, Trigve, keep an eye on the performances as all the handlers will be called for every event, resulting in many more calls than if you were doing the dispatch yourself.

    Thanks iarwain for the response and change.
    I'm fully aware of the performance problems that could happens. But even when using the dispatch mechanism, you should somehow query if the event is targeted for the given instance or not. The question is, what will be faster (the dispatch mechanism could be theoretically faster, because if you find the first instance the event belongs to, you are done. In the other cases, all the events handler should run even if they aren't doing anything).
    Even using the dispatch mechanism I think this is good change (the context parameter), because I don't need to pollute my class with some static instances.

    Just my 2 cents. Would love to hear other opinions on this matter.
  • edited January 2015
    Well I guess it all depends on the events you're handling and the context. If you can guess your external class from the content of the event itself, the dispatch is rather straightforward.

    In Sausage's case, if he were to have more than a table at once, he could store in his objects the ID of the table so that when he handles an object event, he could access the right table directly.

    I don't mind the extra context parameter, it doesn't really have any perf impact on the system and was easy to add, I was just curious to know in which circumstances people would use it. With orx+Scroll I haven't had those situations yet: I could always somehow infer the context from the content of the event itself.
  • edited January 2015
    Big thanks to iarwain, Trigve and especially 4babce who's code snippet sent me off on a four day study of c++ singleton patterns, templates, statics, and a whole lot of passionate arguments on the internet.

    Bottom line is, it is all now working so sweet and I can start back to the business of the game.

    Thank you, gentlemen. Very much appreciated.
  • edited January 2015
    This is the basic code used, with two examples of event usage, one with a clock, and declaring either the base or derived class:

    main.cpp
        PinballBase::create();
        PinballBase::instance()->Init();
    
        //Or uncomment below to use the derived version
        //DerivedTable::create();
        //DerivedTable::instance()->Init();
    


    PinballBase.h
    #ifndef PINBALLBASE_H
    #define PINBALLBASE_H
    #include <sstream>
    
    class PinballBase
    {
    
    private:
        static orxSTATUS orxFASTCALL EventHandlerDispatcher(const orxEVENT *_pstEvent);
        static void orxFASTCALL UpdateDispatcher(const orxCLOCK_INFO *_pstClockInfo, void *_pstContext);
    
    protected:
        PinballBase();
        static PinballBase* _instance;
        virtual ~PinballBase(){};
    
        virtual orxSTATUS orxFASTCALL EventHandler(const orxEVENT *_pstEvent);
        virtual void orxFASTCALL Update(const orxCLOCK_INFO *_pstClockInfo, void *_pstContext);	
    
    public:
        static PinballBase* instance();
        static void create();	
    
        void Init();
    
    };
    
    #endif // PINBALLBASE_H
    


    PinballBase.h
    #include "orx.h"
    #include "this.h"
    #include "that.h"
    #include "and.h"
    #include "the.h"
    #include "other.h"
    #include "PinballBase.h"
    #include <sstream>
    #include <vector>
    
    PinballBase::PinballBase()
    {
    }
    
    PinballBase* PinballBase::_instance = NULL;
    
    PinballBase* PinballBase::instance(){
        if (_instance == NULL){
            orxLOG("Warning! Not initialised.");
        }
        return _instance;
    }
    
    void PinballBase::create()
    {
        if (_instance == NULL){
            _instance = new PinballBase();
        } 
    }
    
    orxSTATUS orxFASTCALL PinballBase::EventHandlerDispatcher(const orxEVENT *_pstEvent){
        return _instance->EventHandler(_pstEvent);
    }
    
    void orxFASTCALL PinballBase::UpdateDispatcher(const orxCLOCK_INFO *_pstClockInfo, void *_pstContext){
        _instance->Update(_pstClockInfo, _pstContext);
    }
    
    orxSTATUS orxFASTCALL PinballBase::EventHandler(const orxEVENT *_pstEvent) {
        return orxSTATUS_SUCCESS;
    }
    
    void orxFASTCALL PinballBase::Update(const orxCLOCK_INFO *_pstClockInfo, void *_pstContext){
    }
    
    void PinballBase::Init()
    {
        orxEvent_AddHandler(orxEVENT_TYPE_INPUT, EventHandlerDispatcher);
    
        orxCLOCK       *pstClock;
        pstClock = orxClock_Create(orx2F(0.01f), orxCLOCK_TYPE_USER);
        orxClock_Register(pstClock, UpdateDispatcher, orxNULL, orxMODULE_ID_MAIN, orxCLOCK_PRIORITY_NORMAL);
    }
    



    DerivedTable.h
    #ifndef DERIVEDTABLE_H
    #define DERIVEDTABLE_H
    
    #include "PinballBase.h"
    
    class DerivedTable: public PinballBase 
    {
    	
    public:
        static void create();
    
    protected:
        DerivedTable();
        virtual ~DerivedTable();
        
        virtual orxSTATUS orxFASTCALL EventHandler(const orxEVENT *_pstEvent);
    
    };
    
    #endif // DERIVEDTABLE_H
    


    DerivedTable.cpp
    #include "orx.h"
    #include "this.h"
    #include "that.h"
    #include "and.h"
    #include "the.h"
    #include "other.h"
    #include "derivedtable.h"
    #include <sstream>
    #include <vector>
    
    DerivedTable::DerivedTable() : PinballBase()
    {
    }
    
    DerivedTable::~DerivedTable()
    {
    }
    
    void DerivedTable::create()
    {
        if (_instance == NULL)
            _instance = new DerivedTable();
    }
    
    orxSTATUS orxFASTCALL PinballBase::EventHandler(const orxEVENT *_pstEvent) {
        // different routine to the pinballbase one. 
    }
    
  • edited January 2015
    For sake of completeness, here's a version using the context that should work with more than one table instance at the same time (though I haven't tested it as I wrote it directly here, so lemme know if you see any glaring issue :)):
    orxSTATUS orxFASTCALL PinballBase::EventHandlerDispatcher(const orxEVENT *_pstEvent){
        return ((PinballBase *)_pstEvent->pContext)->EventHandler(_pstEvent);
    }
    
    void orxFASTCALL PinballBase::UpdateDispatcher(const orxCLOCK_INFO *_pstClockInfo, void *_pContext){
        ((PinballBase *)_pContext)->Update(_pstClockInfo, _pContext);
    }
    
    void PinballBase::Init()
    {
        orxEvent_AddHandlerWithContext(orxEVENT_TYPE_INPUT, EventHandlerDispatcher, (void *)this);
        orxClock_Register(orxClock_Create(orx2F(0.01f), orxCLOCK_TYPE_USER), UpdateDispatcher, (void *)this, orxMODULE_ID_MAIN, orxCLOCK_PRIORITY_NORMAL);
    }
    
Sign In or Register to comment.