• Advertisement
Sign in to follow this  

Should my button class know about my mouse class

This topic is 4174 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Howdy everyone, I have just recently stopped learning DirectX(D3D mostly) to learn SDL. I came to a little snag in my mess about project, I've created a button class and got most things set up but in my function CButton::Clicked(CMouse* mouse) I need to know what the mouse's current state is so I can check whether the mouses position is currently over the button and if the left button has been clicked. So should I just pass a pointer to a CMouse object into my Clicked function like i have, or is there a better way of solving this. Heres my source code so far:
#include <SDL/SDL.h>

class CMouse
{
protected:
          
          int m_xpos, m_ypos;
          bool m_lbutton, m_rbutton;

public:
          CMouse();
          ~CMouse();
          
          void Update();
          int GetXPos();
          int GetYPos();
          bool GetButton();
          
private:
          void SetPosition();
          void SetMouseStates();
};

CMouse::CMouse()
{
        m_xpos = m_ypos = 0;
        m_lbutton = m_rbutton = false;
}

CMouse::~CMouse()
{
        
}

void CMouse::Update()
{
        SetPosition();
        SetMouseStates();
}

int CMouse::GetXPos()
{
        return m_xpos;
}

int CMouse::GetYPos()
{
        return m_ypos;
}

bool CMouse::GetButton()
{
        return m_lbutton;
}

void CMouse::SetPosition()
{
        SDL_GetMouseState(&m_xpos, &m_ypos);
}

void CMouse::SetMouseStates()
{
        if(SDL_GetMouseState(NULL,NULL)&SDL_BUTTON(1))
           m_lbutton = true;
        else
           m_lbutton = false;
}

class CButton
{
protected:
          int m_xpos, m_ypos;
          int m_width, m_height;

public:
          CButton();
          ~CButton();
          
          void SetValues(int x, int y, int w, int h);
          void Draw(SDL_Surface* scr);
          bool Clicked(CMouse* mouse);

private:
          
};

CButton::CButton()
{
        m_xpos = m_ypos = 0;
        m_width = m_height = 0;
}

CButton::~CButton()
{
        
}

void CButton::SetValues(int x, int y, int w, int h)
{
        m_xpos = x;
        m_ypos = y;
        m_width = w;
        m_height = h;
}

void CButton::Draw(SDL_Surface* scr)
{
        SDL_Rect draw = {m_xpos, m_ypos, m_width, m_height};
        SDL_FillRect(scr, &draw, 0xff0000);
}

bool CButton::Clicked(CMouse* mouse)
{
        if((mouse->GetXPos() >= m_xpos)&&
           (mouse->GetXPos() <= m_xpos+m_width)&&
           (mouse->GetYPos() >= m_ypos)&&
           (mouse->GetYPos() <= m_ypos+m_height)&&
           (mouse->GetButton() == true))
           {
                 return true;
           }
           else
           {
                 return false;
           }
}

int main(int argc, char* argv[])
{
        SDL_Init(SDL_INIT_EVERYTHING);
        SDL_Surface* screen = SDL_SetVideoMode(640, 480, 32, SDL_HWSURFACE | SDL_DOUBLEBUF);
        
        CMouse* mouse;
        mouse = new CMouse();

        CButton button;
        button.SetValues(50, 50, 100, 40);
        
        SDL_Event event;
        bool quit = false;

        while(quit != true)
        {
                while(SDL_PollEvent(&event))
                {
                      if(event.type == SDL_QUIT)
                         quit = true;
                }
            mouse->Update();
            
            SDL_FillRect(screen, NULL, 0xffffff);
        
            button.Draw(screen);
        
            if(button.Clicked(mouse))
               quit = true;
            
            SDL_Flip(screen);
        }
    
    delete mouse;
    SDL_Quit();
    return 0;
}


Thanks for any help or suggestions. PS. hehe I know I need to comment my code and seperate it into files before it gets to confusing to look through, but this is just a simple mess about program to see how i could implement buttons into one of my gamesin the future.

Share this post


Link to post
Share on other sites
Advertisement
Frankly I dont think so. Of course, this is all opinion. The button should not be aware of the mouse, especially if you perhaps later down the road want to control the game with say, a gamepad. At the same time, the mouse shouldnt really need to ever know what it is clicking.

My recommendation is, add an OnClicked method that takes the x,y coordinates of the clicker ( if you even need these ) and if it was a single or double click, again, if you even need these.

I would put an interface somewhere in the middle. This interface is what would be updated on mouse ( keyboard, joystick, etc... ) move, click, over, mouseUp etc... actions. Also allow the button to register both a hit rectangle and a callback function ( onHit ). Then this middle manager handles all the mouse polling and delegation of what to do. This will allow funky things like overlapped buttons, etc. Also, it keeps your IO management code in one location, simplifies the Mouse class and the Button class, plus allows you to easily create new GUI elements, without recreating a ton of code.

Share this post


Link to post
Share on other sites
The strategy you tend to see in most GUI libraries is that when a mouse event happens the function(s) that get called to handle the event take a mouse event structure as an argument. This structure may look something like this:


struct MouseEvent
{
MouseEventType Type; //could be MOUSE_DOWN, MOUSE_CLICKED etc
ButtonMask Buttons;
int xPos;
int yPos;
};



The important thing is it's just a data container rather than an actual object with methods as it's meant to just tell the function about the mouse state at the time of the event.

Share this post


Link to post
Share on other sites
It's OK to pass in an object pointer, if the semantics of that object pointer are well defined. Typically, this means that it's expressed in a protocol, which in C++ is easiest implemented as an abstract base class.

Thus, you don't pass in a CMouse; you might pass in an IMouse (or an IClicker). Or, if all you need to pass in is data, an event-specific struct works, too -- but if you find yourself using switch(), you should consider how you could be using virtual functions instead.

Share this post


Link to post
Share on other sites
Quote:
Original post by cNoob
CButton::Clicked(CMouse* mouse)

I need to know what the mouse's current state is so I can check whether the mouses position is currently over the button and if the left button has been clicked.


Normally, you arrange such that if the button isn't clicked, then the function doesn't get called at all. After all, look at the function name. Then, you can pass the actual location information in, instead of a Mouse object. (Don't pass things by pointer, anyway; pass by reference when you can, which is almost all the time.)

Also, don't name your classes with a C prefix (seriously, if "hey, this type is a class type!" is something you need to be constantly aware of, you're probably doing something wrong anyway), actually use constructors for constructing things (instead of setting everything later), use initializer lists when you can (but here it's more appropriate to just Update() upon construction), don't factor out really trivial things, don't write out destructors that don't actually do anything, don't use 'protected:' unless you're planning to derive from that class (and why would you need a subclass of Mouse?), and don't write stupid if-logic (appreciate how booleans work instead).

Oh, and use point and rect classes. SDL has some. Don't only use them where SDL says you have to, but whereever it's appropriate to bind information together in that way - whereever you have actual points and rects.


#include <SDL/SDL.h>

// I'm just going to use a std::pair for a point, because I'm too lazy to look
// up SDL's definition. :P

typedef std::pair<int, int> point;
typedef SDL_Rect rect;

class Mouse {
point m_position;
bool m_left_button_down; // currently the right button can't be detected;
// don't add data until you have code to support it

public:
// Right now all this stuff is short enough to justify inlining
Mouse() { Update(); }

void Update() {
SDL_GetMouseState(&m_position.first, &m_position.second);
m_left_button_down = bool(SDL_GetMouseState(NULL,NULL) & SDL_BUTTON(1));
}
point Position() { return m_position; }
bool LeftClicked() { return m_left_button_down; }
};

class Button : rect {
// That's a hack, but it's sort of necessitated by SDL_Rect sucking and not
// having a constructor. Another option is to make a wrapper just for
// SDL_Rect, and use an instance of the wrapped version as a member.
// Another option is to make your own version properly (but keeping identical
// member types and order) and reinterpret_cast pointers.

public:
Button(int x, int y, int w, int h) : x(x), y(y), w(w), h(h) {}

void Draw(SDL_Surface* scr) {
SDL_FillRect(scr, static_cast<rect*>(this), 0xff0000);
}

bool Clicked(point p) {
// This still isn't "right", because normally you want to subclass your
// Button and have Clicked() be void; if the button is actually clicked as
// determined by this logic, Clicked() then calls a protected virtual
// doClick() that causes the button to have the effect assigned to that
// button.
return ((p.first >= x) &&
(p.first <= x + w) &&
(p.second >= y) &&
(p.second <= y + h));
}
};

int main(int argc, char* argv[]) {
SDL_Init(SDL_INIT_EVERYTHING);
SDL_Surface* screen = SDL_SetVideoMode(640, 480, 32, SDL_HWSURFACE | SDL_DOUBLEBUF);

// 0) Don't dynamically allocate things that you don't need to.
// 1) Don't declare a variable and then "initialize" it on the next line.
// That is NOT initialization. That is assignment.

Mouse m;
Button b(50, 50, 100, 40);

SDL_Event event;
bool quit = false;

while (!quit) { // Don't compare booleans to literals; that's the other part
// of not doing stupid logic with them
while (SDL_PollEvent(&event)) {
if (event.type == SDL_QUIT) {
quit = true;
}
}

m.Update();

SDL_FillRect(screen, NULL, 0xffffff);
b.Draw(screen);

// See how the responsibility is decomposed now and the classes avoid
// knowledge of each other.
if (m.LeftClicked() && b.Clicked(m.Position())) {
quit = true;
}

SDL_Flip(screen);
}

// Now there is no memory management of your own to do.
SDL_Quit();
// Also, you don't need to return 0 explicitly.
}

Share this post


Link to post
Share on other sites
Wow thanks for all the suggestions guys.

@Zahlman, I like the way you have done that maybe I should read up more on C++ and give it another go soon.

Thanks guys.

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
Also, don't name your classes with a C prefix (seriously, if "hey, this type is a class type!" is something you need to be constantly aware of, you're probably doing something wrong anyway)...


While their usefulness is debatable the prefix means Concrete I believe, not class. So you have C for concrete, I for interface, and none for POD types.

Share this post


Link to post
Share on other sites
To that, I say: marking concrete classes as well as interface classes is redundant (and anyway, in what sense does a POD not qualify as a "concrete class"? Yet we don't make a distinction there); marking interfaces points to bad or inadequate class naming (consider how Java standard library classes are named); and if you're making your interface classes *properly*, you *still* don't need to know (because if you tried to "instantiate an interface class", you'd get a compile-time error, and there shouldn't be anything else really silly you can do if you have your polymorphism set up properly). Not to mention, there's no existing interface that's being implemented here. :
I would go so far as to say that, among those who are competent, over-design is the greatest enemy of code clarity. (Among those who aren't, it tends to be more to do with haphazard naming/commenting and redundant code.)

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
To that, I say: marking concrete classes as well as interface classes is redundant (and anyway, in what sense does a POD not qualify as a "concrete class"? Yet we don't make a distinction there); marking interfaces points to bad or inadequate class naming (consider how Java standard library classes are named); and if you're making your interface classes *properly*, you *still* don't need to know (because if you tried to "instantiate an interface class", you'd get a compile-time error, and there shouldn't be anything else really silly you can do if you have your polymorphism set up properly). Not to mention, there's no existing interface that's being implemented here. :
I would go so far as to say that, among those who are competent, over-design is the greatest enemy of code clarity. (Among those who aren't, it tends to be more to do with haphazard naming/commenting and redundant code.)


I won't say much about the prefixing thing, except that I use it to specify intent about how a class is to be used: tFoo is a struct-type, all-public member thing, rFoo is a binary resource (using fixed size types and explicit alignment padding), and cFoo is a "real" class with private data and the like. I don't use this approach to differentiate between abstract or concrete.

Although I have seen plenty of folks over-design stuff, and then get all tripped up in it, I must say that I've seen more people under-design than over-design. By this, I mean lots of people are "just trying to get something done" and don't take a broader look at what effect the code they're writing will have. I've seen tasks get done by sprinkling little bits of broken glass throughout many different pieces of code, to the detriment of the overall codebase.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement