Graphical Tic-Tac-Toe game using SFML

Question

First of all, I'm really new to SFML and making GUIs. This is my first real program using this.

Right now, when I click outside the window when the program is first run, the first piece is placed in the top left corner without clicking that square. It works as I want it to every time after that, with an error message appearing when you click on a square already taken. In the takeTurn() function, I set a value to the left variable as a way to detect that the position is already taken.

I know that's the problem, but I can't think of another way right now. Maybe if anyone can help point me in the right direction? Also, I know this code is pretty ugly, so if anyone is familiar with SFML, I'd appreciate some pointers on how to clean this up.

#include <SFML/Graphics.hpp>
#include <iostream>

class Game {
    private:
        sf::RenderWindow window;
        sf::Texture gameBoard, pieceX, pieceO;
        sf::Sprite addBoard, addX, addO, piece[9];
        sf::Font fontType;
        sf::Text message;
        sf::IntRect square[9];
        sf::Vector2i position[9];
        sf::Event event;    
    public:
        void loadGameWindow();
        bool loadPieceFiles();
        bool loadFontFile();
        void setGamePieces();        
        void setUserPrompt();
        void rectangleCoordinates();
        void setRectCoordinates(sf::IntRect &rect, int left, int top, int width, int height);
        void setPositionPieces(sf::Vector2i &pos, int left, int top);
        void positionPiece();
        bool isClickInBounds(int boardPos);
        bool takeTurn(int player);
        void updateUserPrompt(int turn, bool invalidPos);
        void gameLoop();

};

void Game::loadGameWindow(){
    window.create(sf::VideoMode(400, 500), "Tic-Tac-Toe");
}

bool Game::loadPieceFiles(){
    return (gameBoard.loadFromFile("board.png") && 
        pieceX.loadFromFile("x.png") && pieceO.loadFromFile("o.png"));
}

bool Game::loadFontFile(){
    return (fontType.loadFromFile("3rd Man.ttf"));
}

void Game::setGamePieces(){
    addBoard.setTexture(gameBoard);
    addX.setTexture(pieceX);
    addO.setTexture(pieceO);
}

void Game::setUserPrompt(){
    message.setString("Player 1's turn");
    message.setFont(fontType);
    message.setCharacterSize(40);
    message.setColor(sf::Color::Black);
    message.move(95, 410);
}

void Game::updateUserPrompt(int turn, bool invalidPos){
    std::string playerPrompt;
    if(invalidPos){
        playerPrompt = "Position already\n     taken!";
    } else {   
        playerPrompt = turn % 2 == 0? "Player 1's turn" : "Player 2's turn";
    }

    message.setString(playerPrompt);

}

void Game::setRectCoordinates(sf::IntRect &rect, int rectLeft, int rectTop, int rectWidth, int rectHeight){
    rect.left = rectLeft;
    rect.top = rectTop;
    rect.width = rectWidth;
    rect.height = rectHeight; 
}

void Game::rectangleCoordinates(){
    setRectCoordinates(square[0], 0, 0, 113, 120);
    setRectCoordinates(square[1], 126, 0, 262, 120);
    setRectCoordinates(square[2], 276, 0, 400, 120);

    setRectCoordinates(square[3], 0, 133, 113, 270);
    setRectCoordinates(square[4], 126, 133, 262, 270);
    setRectCoordinates(square[5], 276, 133, 400, 270);

    setRectCoordinates(square[6], 0, 283, 113, 400);
    setRectCoordinates(square[7], 126, 283, 262, 400);
    setRectCoordinates(square[8], 276, 283, 400, 400);
}

void Game::setPositionPieces(sf::Vector2i &pos, int left, int top){
    pos.x = left;
    pos.y = top; 

}

void Game::positionPiece(){
   setPositionPieces(position[0], 15, 20);
   setPositionPieces(position[1], 162, 20);
   setPositionPieces(position[2], 300, 20);

   setPositionPieces(position[3], 15, 166);
   setPositionPieces(position[4], 162, 166);
   setPositionPieces(position[5], 300, 166);

   setPositionPieces(position[6], 15, 316);
   setPositionPieces(position[7], 162, 316);
   setPositionPieces(position[8], 300, 316);


}

bool Game::isClickInBounds(int boardPos){   
    return event.mouseButton.x >= square[boardPos].left 
        && event.mouseButton.x <= square[boardPos].width 
        && event.mouseButton.y >= square[boardPos].top 
        && event.mouseButton.y <= square[boardPos].height;

}

bool Game::takeTurn(int turn){
    for(int boardPos = 0; boardPos < 9; boardPos++){
        if(isClickInBounds(boardPos)){
            if(square[boardPos].left != -500){
                piece[turn] = turn % 2 == 0? addX : addO;
                piece[turn].move(position[boardPos].x, position[boardPos].y);
                square[boardPos].left = -500;
                updateUserPrompt(turn, false);
                return true;
            } else {
                updateUserPrompt(turn, true);
                return false;
            }
        }  
    }
}

void Game::gameLoop(){
    int turn = 0;
    setUserPrompt();
    rectangleCoordinates();
    positionPiece(); 

    while(window.isOpen()){
        while(window.pollEvent(event)){
            if(event.type == sf::Event::Closed){
                window.close();
            }

            if(sf::Mouse::isButtonPressed(sf::Mouse::Left) && turn < 9){
                if(takeTurn(turn)){
                    turn++;
                } 
            }

        }
        window.clear(sf::Color(0, 150, 0)); 
        window.draw(addBoard);
        window.draw(message);
        for(int pieceIdx = 0; pieceIdx < 9; pieceIdx++)
            window.draw(piece[pieceIdx]); 
        window.display(); 

    }

}

int main(int argc, char *argv[]){ 
    Game game;
    game.loadGameWindow();
    if(!game.loadPieceFiles())
        return EXIT_FAILURE;

    game.setGamePieces();

    if(!game.loadFontFile())  
        return EXIT_FAILURE;

    game.gameLoop();

    return EXIT_SUCCESS;
}

Answer 1

Click Event

I apologize, the computer I'm on doesn't have SFML installed but I'll try to get to this later(probably tomorrow). One thing I noticed, though, is in isClickInBounds you do:

   event.mouseButton.x >= square[boardPos].left 
&& event.mouseButton.x <= square[boardPos].width
...

Here you're reinventing the wheel, and I'm not entirely sure it's correct. It could be, but I can't test it atm so I'm skeptical. One thing you can try for sure is this(add parenthesis - see parenthesis section below):

return (event.mouseButton.x >= square[boardPos].left 
    && event.mouseButton.x <= square[boardPos].width 
    && event.mouseButton.y >= square[boardPos].top 
    && event.mouseButton.y <= square[boardPos].height);

The documentation for sf::Rect has a .contains that takes an x, y coordinate, so you can just call that. Ie.

return (square[boardPos].contains(event.mouseButton.x, event.mouseButton.y));

Player Turn

Also of note, using an int as a decider of whose turn it is isn't a very clear way of determining whose turn it is. Usually in a more complex game, there would be a Player class, which would have some sort of .takeTurn() method. Then it's a simple matter of iterating through the list of players(in this case 2), calling the .takeTurn() method on each player, and checking for a win condition after each turn.

In your case, you could use an enum. The previous link is a good source of information on enums in C++. In this enum I added a default value just for the sake of putting one in there:

enum players_t {NO_PLAYER, PLAYER_ONE, PLAYER_TWO};

Then you can have a nextPlayer function that toggles between PLAYER_ONE and PLAYER_TWO. It then becomes easier to check whose turn it is, and the code is more descriptive:

playerPrompt = (turn == PLAYER_ONE) ? "Player 1's turn" : "Player 2's turn";

Though I will admit this will require quite a few changes to your code.

Parenthesis

And on that note, it's always a good idea to surround conditional statements with (). Some examples:

int result = (ternary) ? 1 : 2;
if(somethingIsTrue){
if( (oneThingIsTrue) && (anotherThingIsTrue) ){
return ( (someInt > 0) && (anotherInt < 0) );

Similarly, it's a good idea to use curly braces on all if..else statements, even if they happen to be one-liners. You were good on this point when it came to if..elses(I don't see one that doesn't use curly braces), but you do have a for loop that doesn't use curly braces.

Conventions

Overall your coding conventions are great. You use proper indentations, you're consistent with putting the opening brace on the same line as the statement(Ie. if(...){), but there are a few places where trailing white space should be removed. At the end of any code blocks, you should get rid of any empty lines just for the sake of cleanliness.

But I would say this code looks pretty good as-is.

Again, I apologize about the SFML. I'll take a better look at it later.

Edit #1:

Sorry it took so long to get back to this. I just tried running your application. I don't have your various resource files like board.png, x.png or o.png. Is there any way you could send those to me(imgur will host em for free)?

Also, there are two warnings from compiling. In takeTurn, there is an implicit cast from int to float:

piece[turn].move(position[boardPos].x, position[boardPos].y);

Which should instead read as:

piece[turn].move((float)position[boardPos].x, (float)position[boardPos].y);

Also, takeTurn has a path that doesn't return a value. It's easy to fix though:

bool Game::takeTurn(int turn){
    for(int boardPos = 0; boardPos < 9; boardPos++){
        if(isClickInBounds(boardPos)){
            if(square[boardPos].left != -500){
                piece[turn] = turn % 2 == 0? addX : addO;
                piece[turn].move((float)position[boardPos].x, (float)position[boardPos].y);
                square[boardPos].left = -500;
                updateUserPrompt(turn, false);
                return true;
            } else {
                updateUserPrompt(turn, true);
                return false;
            }
        }  
    }
    return false;
}

I know it may seem silly, but you should get in the habit of treating any and all warnings as if they're errors. As you become more comfortable with various compiler settings, I would encourage you to turn on as many warnings as possible. GCC has some great ones, like -Wall. For things like -Wall, you don't have to fix everything it complains about, but you should be aware that there's a large enough body that considers those things to be a warning about code in order to make its way into a mainstream compiler's warning system.