0
\$\begingroup\$

I have a game with a menu made up of 9 'tiles'. I want the tiles to turn white when the cursor hovers over them (and play a sound), and go back to default when the mouse moves off. My code works when the cursor is moved slowly, but when it is moved quickly the tiles don't turn back to the default colour when the mouse is moved off, resulting in multiple tiles staying white. I'm not sure if the problem is with my code or with SDL event polling. Below are the relevant code snippets, the full source can be found here

void MenuState::handleEvents(SDL_Event& e) {
    //(...)

    if (e.type == SDL_MOUSEMOTION) {
        int x; int y;
        SDL_GetMouseState(&x, &y);
        mousePos = getActiveTile(x, y);

        if (mousePos >= 0 && tiles[mousePos].tileType() != Tile::type::buttonpressed) {
            tiles[mousePos].setTileType(Tile::type::rollover);
        }
        else if (prevMousePos >= 0 && tiles[prevMousePos].tileType() != Tile::type::buttonpressed) {
            // if mouse is *no longer* inside button
            tiles[prevMousePos].setTileType(Tile::type::button);
            prevMousePos = -1;
        }
    }

    //(...)
}


int MenuState::getActiveTile(const int &x, const int &y) {
    int tilenum = -1;
    for (int i = 0; i < tiles.size(); ++i) {
        if ( !(x < tiles[i].position().x || x > tiles[i].position().x + tiles[i].position().w ||
               y < tiles[i].position().y || y > tiles[i].position().y + tiles[i].position().h) )
            tilenum = i ;
    }
    return tilenum;
}

void MenuState::update() {
    if (mousePos >= 0 && prevMousePos == -1) { // only play when mouse first touches button
        Mix_PlayChannel(-1, rollOver, 0);
        prevMousePos = mousePos;
    }
    //(...)

}

the render function (the 'buttons' are held in a vector of 'Tile's)

void Graphics::drawBoard(const std::vector<Tile>& t, const bool& drawnumber) {
    for (int i = 0; i < t.size(); ++i) {
        SDL_Rect temp = t[i].position();

        if      (t[i].tileType() == Tile::type::invisible)
            SDL_SetRenderDrawColor(renderer, 163, 118, 172, 1); // bg colour

        else if (t[i].tileType() == Tile::type::rollover || t[i].tileType() == Tile::type::buttonpressed)
            SDL_SetRenderDrawColor(renderer, 255, 255, 255, 1); // highlight colour

        else if (t[i].tileType() == Tile::type::shadow)
            SDL_SetRenderDrawColor(renderer, 100, 65, 107, 1); // shadow colour

        else
            SDL_SetRenderDrawColor(renderer, 237, 229, 239, 1); // default colour

        SDL_RenderFillRect(renderer, &temp);

        if (drawnumber){
            if (t[i].tileType() != Tile::type::invisible) {
                std::string num = std::to_string(t[i].number());
                tilenumber = renderText(num, "assets/Calibrib.ttf", fontcolour, 25);
                SDL_QueryTexture(tilenumber, NULL, NULL, &temp.w, &temp.h);
                renderTexture(tilenumber, temp.x+(TILE_SIZE/2 - temp.w/2), temp.y+(TILE_SIZE/2 - temp.h/2), nullptr);
            }
        }
    }

}

\$\endgroup\$
  • 1
    \$\begingroup\$ If you only have 9 tiles, just iterate over all 9 every time. Set the one currently being hovered to rollover, and reset the other 8 to button. Don't bother remembering the previous mouse position and trying to reset just one (unless you have hundreds of tiles, and even then I would recommend this approach until you empirically find it to be too slow). \$\endgroup\$ – Dan Oct 28 '16 at 12:46
  • \$\begingroup\$ @Dan that's exactly what I was thinking as well, you should probably write that down as an answer. \$\endgroup\$ – Tyyppi_77 Oct 28 '16 at 13:07
  • \$\begingroup\$ Thank you, this works much better. Out of curiosity, do you happen to know why my previous solution didn't work? \$\endgroup\$ – Adam Smith Oct 28 '16 at 13:25
  • \$\begingroup\$ I'm guessing it has something to do with how you're tracking prevMousePos. Without the full context, I can't say exactly. You'd have to write some debug output whenever you set or use that variable, reproduce the problem, then examine the output. \$\endgroup\$ – Dan Oct 28 '16 at 13:35
1
\$\begingroup\$

Adapted from my comment:

If you only have 9 tiles, just iterate over all 9 every time. Set the one currently being hovered to rollover, and reset the other 8 to button. Don't bother remembering the previous mouse position and trying to reset just one (unless you have hundreds of tiles, and even then I would recommend this approach until you empirically find it to be too slow).

I replaced getActiveTile() with setActiveTile(), which takes a mouse position and corrects highlighting for all tiles. It still returns the ID of the currently hovered tile in case you need that information to perform some other actions (e.g. marking it as pressed when a click event happens, without having to search by coords again).

void MenuState::handleEvents(SDL_Event& e) {
    //(...)

    if (e.type == SDL_MOUSEMOTION) {
        int x; int y;
        SDL_GetMouseState(&x, &y);

        int hoveredTile = setActiveTile(x, y);
    }

    //(...)
}

// Still returns tile ID in case you need it for something else
int MenuState::setActiveTile(const int &x, const int &y) {
    int hoveredTile = -1;

    int tileCount = tiles.size();
    for (int i = 0; i < tileCount; ++i) {
        if ( !(x < tiles[i].position().x || x > tiles[i].position().x + tiles[i].position().w ||
               y < tiles[i].position().y || y > tiles[i].position().y + tiles[i].position().h) )
        {
            // Don't change style when hovered button is already being pressed
            if (tiles[i].tileType() != Tile::type::buttonpressed)
            {
                tiles[i].setTileType(Tile::type::rollover);
            }
            hoveredTile = i;
        } else {
            // If the mouse isn't over this button, it can't be "buttonpressed",
            // so no need to check that. Just reset it to a regular button style.
            tiles[prevMousePos].setTileType(Tile::type::button);
        }
    }

    return hoveredTile;
}

Minor Tip:

You're doing ++i in your for loop, which I assume someone told you to do because it's "faster than i++". The more important optimization here is to take the .size() call out of the loop, because calculating the size of the (vector?) every iteration is wasteful.

When you don't care about processing order (we don't in this case), you can also write your loops like this:

for (int i = tiles.size() - 1; i >= 0; i--) {

Which will cause .size() to only be evaluated once. If this is actually an std::vector, you might consider using iterators as well.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.