Page 1 of 1

function argument number

Posted: Wed Jun 01, 2011 02:17
by jarwulf
hi, I'm currently attempting to lay the framework for a UI which is a lot bigger than anything I've done before. I've run into some problems. I have a file called UImanager.cc which handles all the loading of window layouts and button registrations. One of the first errors I get when attempting to compile is this


Code: Select all

void UIManager::UIMenuReg()
{
CEGUI::Window* pMainWnd = CEGUI::WindowManager::getSingleton().getWindow("AOF_GUI");//sequence?
    OgreFramework::getSingletonPtr()->m_pGUISystem->setGUISheet(pMainWnd);

CEGUI::PushButton* button = (CEGUI::PushButton*)pMainWnd->getChild("BExit");

/*error right here!!! */   button->subscribeEvent(CEGUI::PushButton::EventClicked,

CEGUI::Event::Subscriber(&MenuState::onExitButton, this));
   button = (CEGUI::PushButton*)pMainWnd->getChild("BEnter");
   button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&MenuState::onEnterButton, this));   
}



Code: Select all

1>c:\documents and settings\minion\my documents\visual studio 2008\projects\ogregamelev(100810)\ogregamelev(100810)\uimanager.cc(52) : error C2661: 'CEGUI::SubscriberSlot::SubscriberSlot' : no overloaded function takes 2 arguments


As you can see there is no CEGUI::SubscriberSlot::SubscriberSlot' in sight and I don't recall messing with the function directly and introducing the wrong number of arguments so any ideas what could be amiss?

Thanks alot

Re: function argument number

Posted: Wed Jun 01, 2011 07:46
by Kulik
Make sure onExitButton has a signature of bool onExitButton(const EventArgs& args). This is a common mistake.

If it matches that, please post the declaration of it.

Re: function argument number

Posted: Wed Jun 01, 2011 17:15
by Jamarr
jarwulf wrote:void UIManager::UIMenuReg()
{
...
button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&MenuState::onExitButton, this));
...
button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&MenuState::onEnterButton, this));


The bold items look like potential conflicts. You are subscribing handlers from a MenuState object but you are subscribing them as if they belong to an instance of the UIManager object (this). When subscribing methods of an object you must provide two things: 1) the method address and 2) a pointer to an instance of the object the method belongs to; how else would the code know how to invoke a non-static object method?

jarwulf wrote:OgreFramework::getSingletonPtr()->m_pGUISystem->setGUISheet(pMainWnd);

Also I hate to nit pick but this code is just asking for trouble by breaking encapsulation aka the Law of Demeter. If this is project is just a tech demo and/or R&D then happy coupling. But if it is more than that I would recommend you strive to reduce coupling by any and all means necessary; you will save yourself hours and hours of frustration down the road.

Re: function argument number

Posted: Fri Jun 03, 2011 03:16
by jarwulf
Kulik wrote:Make sure onExitButton has a signature of bool onExitButton(const EventArgs& args). This is a common mistake.

If it matches that, please post the declaration of it.



Code: Select all

//MenuState.h

bool onExitButton(const CEGUI::EventArgs &args);



//MenuState.cpp

bool MenuState::onExitButton(const CEGUI::EventArgs &args)
{
   m_bQuit = true;
   return true;
}



Jamarr wrote:

Code: Select all

void UIManager::UIMenuReg()
{
...
button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&MenuState::onExitButton, this));
...
button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&MenuState::onEnterButton, this));



The bold items look like potential conflicts. You are subscribing handlers from a MenuState object but you are subscribing them as if they belong to an instance of the UIManager object (this). When subscribing methods of an object you must provide two things: 1) the method address and 2) a pointer to an instance of the object the method belongs to; how else would the code know how to invoke a non-static object method?



Okay how about I move onExitButton and onEnterButton to UIManager and change the code to this?

Code: Select all

button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&UIManager::onEnterButton, this));




Jamarr wrote:

Code: Select all

    OgreFramework::getSingletonPtr()->m_pGUISystem->setGUISheet(pMainWnd);



Also I hate to nit pick but this code is just asking for trouble by breaking encapsulation aka the Law of Demeter. If this is project is just a tech demo and/or R&D then happy coupling. But if it is more than that I would recommend you strive to reduce coupling by any and all means necessary; you will save yourself hours and hours of frustration down the road.



Main flow of program goes main->Demoapp demo->create GameState create MenuState->MenuState. Would it be a better idea to place the setGUISheet code directly in MenuState rather than having UIManager handle it? Might not be a big deal for MenuState but I planned on having several different 'sheets' for Gamestate ala 'Inventory' 'Map' etc so I thought having UIManager handle all would be cleaner but I'd rather follow encapsulation I guess.

Re: function argument number

Posted: Fri Jun 03, 2011 18:44
by Jamarr
jarwulf wrote:Okay how about I move onExitButton and onEnterButton to UIManager and change the code to this?

Code: Select all

button->subscribeEvent(CEGUI::PushButton::EventClicked, CEGUI::Event::Subscriber(&UIManager::onEnterButton, this));



That is one option. Another option would be to keep the handlers in MenuState and have UIManager pass a pointer (instead of this) to the MenuState object. And yet another option, again leaving the handlers in MenuState, would be to move the subscription calls into the MenuState. The best approach is dependent on how your code is setup and you want to organize it. Let us know if this works out for you.

jarwulf wrote:
Jamarr wrote:

Code: Select all

    OgreFramework::getSingletonPtr()->m_pGUISystem->setGUISheet(pMainWnd);


Also I hate to nit pick but this code is just asking for trouble by breaking encapsulation aka the Law of Demeter. If this is project is just a tech demo and/or R&D then happy coupling. But if it is more than that I would recommend you strive to reduce coupling by any and all means necessary; you will save yourself hours and hours of frustration down the road.


Main flow of program goes main->Demoapp demo->create GameState create MenuState->MenuState. Would it be a better idea to place the setGUISheet code directly in MenuState rather than having UIManager handle it? Might not be a big deal for MenuState but I planned on having several different 'sheets' for Gamestate ala 'Inventory' 'Map' etc so I thought having UIManager handle all would be cleaner but I'd rather follow encapsulation I guess.


It is not necessarily a bad idea to have the GUISheet set in UIManager and if this is just a demo it may not be worth the effort. The main issue with coupling is maintenance, such that if objects are overly coupled then when one thing changes you have to update more code than would be necessary in a decoupled system. That said, I do think it is a good idea to get into the habit of recognizing potentially over-coupled objects and decoupling them when found as your overall design and development abilities will improve greatly over time.

If we look at the specifics for this issue: the CEGUI object resides in the OgreFramework object, and the UIManager updates the GUISheet via the CEGUI object by reaching through the OgreFramework object. The problem with this is that the UIManager must implicitly know the internal structure of OgreFramework. Instead of accessing the underlying GUI object through OgreFramework you could encapsulate this by creating OgreFramework::setGUISheet which you would call from UIManager; this hides the internal structure of OgreFramework from UIManager. Thus if the underlying GUI system and/or it's API changes you would only need to update the implementation of the encapsulating class.

However in this particular case, it may be that the UIManager needs to implicitly know the underlying GUI system. Does it make more sense to encapsulate CEGUI within the OgreFramework or within the UIManager? As I am not intimate with your code I cannot determine what coupling is/not necessary, merely that this smells like an area where unnecessary coupling is occurring.