Problem with Setting fillColor from a Member Function

[Tofy]

I have this Button class, which is just a text inside a rectange:

class Button {
private:
        sf::RectangleShape rect;

        sf::Font font;
        sf::Text text;

        float width;
        float hight;
        float x;
        float y;
        sf::Color idleColor;
        sf::Color hoverColor;

public:
        Button(
                std::string     string                                  ,
                float           x               = 0                     ,
                float           y               = 0                     ,
                float           width           = 100.f                 ,
                float           hight           = 100.f                 ,
                sf::Color       idleColor       = sf::Color::Green      ,
                sf::Color       hoverColor      = sf::Color::Red        );

 

Button::Button(
                std::string     string                  ,
                float           x                       ,
                float           y                       ,
                float           width                   ,
                float           hight                   ,
                sf::Color       idleColor               ,
                sf::Color       hoverColor              )
{      
        width = width;
        x = x;
        y = y;
        idleColor = idleColor;
        hoverColor = hoverColor;
        string = string;

        //initializing font
        //initializing size
        //initializing position ...etc
}

//If button contains mouse, set fillColor to hoverColor. Else, set it to idleColor
void Button::update(sf::RenderWindow* window) {
        if (rect.getGlobalBounds().contains(
                sf::Mouse::getPosition(*window).x, sf::Mouse::getPosition(*window).y))
        {              
                rect.setFillColor(hoverColor);
        }
        else
                rect.setFillColor(idleColor);
}
 

And in main:

std::unordered_map<std::string, Button*> Buttons;
Buttons["start"]        = new Button("Start"                                    );
Buttons["settings"]     = new Button("Settings",        0.f,    100.f);
Buttons["quit"]         = new Button("Quit",            0.f,    200.f);

for (auto& it:Buttons) {
        it.second->update(&window);
        it.second->draw(&window);
}
 

The problem is that the fill color doesn't appear. I just get black buttons.

[Hapax]

It's possible that it could be linked to this code:

        idleColor = idleColor;
        hoverColor = hoverColor;
 

These lines - and the ones around it - are setting themselves to themselves.
The issue with that is that your parameters have the same name as the class's members so there's no real way of the compiler knowing which one you meant each time. (i.e. member = parameter).
Since the colours aren't getting set/changed, they'll be set to the default colour: black.

A couple of ways to avoid this are:

• name the parameters differently to the members
• use the class's "this" pointer

- (1) Different names
Two common methods for this are:

• add a prefix to member, such as "m_" or just "m". e.g. m_idleColor
• make the parameter clear to be different. e.g. newIdleColor

- (2) "This"
"This" is a pointer that always points to the current object of the class so can be used to specify explicitly that you are refering to that object (and its class).
For example:

this->idleColor = idleColor;

Note that although the local variables (the parameters in this case) have priority and will be the ones accessed by the name, this code is still not particularly clear (idleColor is the parameter but looks like the class member) so the above method (1) is what I would recommend for clearer intentions.



In addition
I'd also recommend not storing the (large resource) font inside the button as that means having multiple copies of a font - one for each button - even if they use the same one.
Instead, store the font somewhere else and pass it to your button. When it is passed, set the text's font. You should probably do this in your constructor since the newest version of SFML (3) requires a font to be assigned to a text during construction (so you can't create an empty one) and it's worth getting used to.

Anyway, that would look something like:
(in Button):

class Button {
private:
    sf::RectangleShape rect;

    sf::Text text; // no font here!

    // others..

public:
    Button(
        std::string newString,
        sf::Font& font, // this does not need _new_ because we do not have a font in the class
        float newX,
        // others..
        sf::Color newHoverColor = sf::Color::Red);
}
Button::Button(
    std::string newString,
    sf::Font& font,
    float newX,
    // others..
    sf::Color newHoverColor)
    : text("", font)
{
    string = newString;
    // others..
}

(in main):

sf::Font font;
if (!font.loadFromFile("arial.ttf")) // put your actual font file here
{
    std::cerr << "Could not load font.\n";
    return EXIT_FAILURE;
}
std::unordered_map<std::string, Button*> Buttons;
Buttons["start"] = new Button("Start", font);
Buttons["settings"] = new Button("Settings", font, 0.f, 100.f);
Buttons["quit"] = new Button("Quit", font, 0.f, 200.f);