2
\$\begingroup\$

This is my first game ever. I' learning C++ now and I decided to make a game. I saw a snake game that works with the draw-input-logic system and I really liked it. I tried to make a tic tac toe game.

Please rate it and show me how can I improve it, or give me any other simple game ideas.

input: x y;

#include <iostream>
#include <vector>

// 1 -> X | -1 -> O | 0 -> Empty
bool GameOver;
bool checkDraw(std::vector<std::vector<int>>&);

void draw(std::vector<std::vector<int>>& v) {
    system("CLS");
    for (int i = 0; i < 11; i++) {
        std::cout << " ";
        if (i == 4)
            std::cout << "1";
        else if (i == 7)
            std::cout << "2";
        else if (i == 10)
            std::cout << "3\n";
    }
    int num = 1, val = 0;
    for (int i = 0; i < 7; i++) {
        if (i % 2 == 0) {
            std::cout << "   ";
            for (int j = 0; j < 3; j++) {
                std::cout << "+---";
            }
            std::cout << "+\n";
        }
        else {
            std::cout << " " << num << " ";
            num++;
            for (int j = 0; j < 3; j++) {
                if (v[val][j] == 1)
                    std::cout << "| X ";
                else if (v[val][j] == -1)
                    std::cout << "| O ";
                else
                    std::cout << "|   ";
            }
            std::cout << "|" << std::endl;
            val++;
        }
    }
    std::cout << std::endl;
}

void logic(std::vector<std::vector<int>>& v) {
    int num1 = 0, num2 = 0;
    int XorO_1 = 0, XorO_2 = 0, XorO_3 = 0;
    bool x, y, z = false;
    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            if (v[i][j] == v[i][0] && v[i][0] != 0) {
                x = true;
                XorO_1 = v[i][0];
                num1++;
                if (num1 == 3)
                    break;
            }
            else {
                x = false;
                break;
            }
        }
        num1 = 0;
        if (x)
            break;
    }

    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            if (v[j][i] == v[0][i] && v[0][i] != 0) {
                y = true;
                XorO_2 = v[0][i];
                num2++;
                if (num2 == 3)
                    break;
            }
            else {
                y = false;
                break;
            }
        }
        num2 = 0;
        if (y)
            break;
    }

    if (v[0][0] == v[1][1] && v[1][1] == v[2][2] && v[1][1] != 0) {
        if (v[0][0] == 1) {
            z = true;
            XorO_3 = 1;
        }
        else if (v[0][0] == -1) {
            z = true;
            XorO_3 = -1;
        }
    }
    else if (v[0][2] == v[1][1] && v[1][1] == v[2][0] && v[1][1] != 0) {
        if (v[0][2] == 1) {
            z = true;
            XorO_3 = 1;
        }
        else if (v[0][2] == -1) {
            z = true;
            XorO_3 = -1;
        }
    }

    if (x || y || z) {
        if (XorO_1 == 1 || XorO_2 == 1 || XorO_3 == 1) {
            std::cout << "USER 1 (X) IS THE WINNER :D\n";
            GameOver = true;
        }
        else if (XorO_1 == -1 || XorO_2 == -1 || XorO_3 == -1) {
            std::cout << "USER 2 (O) THE IS WINNER :D\n";
            GameOver = true;
        }
    }
    else if (!checkDraw(v)) {
        std::cout << "DRAW...\n";
        GameOver = true;
    }
}

void input(std::vector<std::vector<int>>& v) {
    static int turn = 0;
    int x, y;
    if (turn % 2 == 0) {
        do {
            std::cout << "User 1 (X) its your turn, Enter position: ";
            std::cin >> x >> y;
        } while ((x > 3 || x <= 0) || (y > 3 || y <= 0));
        while (v[y - 1][x - 1] == 1 || v[y - 1][x - 1] == -1) {
            std::cout << "Wrong input User 1 (X), Try Again : ";
            std::cin >> x >> y;
        }
        v[y - 1][x - 1] = 1;
    }
    else {
        do {
            std::cout << "User 2 (O) its your turn, Enter position: ";
            std::cin >> x >> y;
        } while ((x > 3 || x <= 0) || (y > 3 || y <= 0));
        while (v[y - 1][x - 1] == 1 || v[y - 1][x - 1] == -1) {
            std::cout << "Wrong input User 2 (O), Try Again : ";
            std::cin >> x >> y;
        }
        v[y - 1][x - 1] = -1;
    }
    turn++;
}

bool checkDraw(std::vector<std::vector<int>>& v) {
    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            if (v[i][j] == 0)
                return 1;
        }
    }
    return 0;
}

int main() {
    std::vector<std::vector<int>> game{
        {0, 0, 0},
        {0, 0, 0},
        {0, 0, 0}
    };
    draw(game); // i did this for a reason, if not u can't see the winnig position, try it yourself 

    while (!GameOver) {
        input(game);
        draw(game);
        logic(game);
    }
}
\$\endgroup\$
3
\$\begingroup\$

I'll start with the good.

  1. You did not have using namespace std;.
  2. You are passing std::vector<std::vector<int>> game by a reference, nice!

Now for the bad, and unfortuanatly there is quite a bit.

This looks like it started life as C code. The only thing C++ about this is the use of std::vector instead of int[] and std::cout instead of printf.

In C++ we have classes. Consider creating one:

class Game
{
public:
    void draw();
    void input();
    int logic();

private:
    bool gameOver;
    std::vector<std::vector<int>> game;
};

Draw

system("CLS"); will only work on Windows, you can find some alternatives here.

This could have been much simpler if you used X, O and for the symbols instead of 1, -1, and a zero.

logic

This is a hot mess! Consider what you actually need to do here.

  1. Check if player 1 has won
  2. Check if player 2 has won
  3. Check if it's a draw

I would create the following function:

bool checkIfPlayerWon(std::vector<std::vector<int>>& v, int player);

and then simplify logic to the following

void logic(std::vector<std::vector<int>>& v) {
    if (checkIfPlayerWon(v, 1)) {
        std::cout << "USER 1 (X) IS THE WINNER :D\n";
        GameOver = true;    
    }
    else if (checkIfPlayerWon(v, -1)) {
        std::cout << "USER 0 (O) IS THE WINNER :D\n";
        GameOver = true;    
    }
    else if (!checkDraw(v)) {
        std::cout << "DRAW :-(\n";
        GameOver = true;
    }
}

Input

You are basically repeating yourself here, don't do that!

\$\endgroup\$
4
  • \$\begingroup\$ Thanks! althought im not familiar with oop just yet im kinda new, and i heared that it's not necessary to use classes while making simple games like this one no? i will try to rewrite it with more functions and simplify the code. \$\endgroup\$
    – ZakiMkn
    Oct 13 '21 at 17:42
  • \$\begingroup\$ Cool, experimenting is the best way to learn! A logical next step would be to see if you can create a computer opponent… \$\endgroup\$
    – jdt
    Oct 13 '21 at 18:41
  • \$\begingroup\$ i would like to know how can i do that? it requires ai stuff no? \$\endgroup\$
    – ZakiMkn
    Oct 13 '21 at 19:48
  • \$\begingroup\$ start with a simple one that picks random spaces \$\endgroup\$
    – jdt
    Oct 13 '21 at 20:44
1
\$\begingroup\$
   for (int i = 0; i < 11; i++) {
        std::cout << " ";
        if (i == 4)
            std::cout << "1";
        else if (i == 7)
            std::cout << "2";
        else if (i == 10)
            std::cout << "3\n";
    }

If I'm reading that right, it does nothing more than:
cout << " 1 2 3\n";
Why would you write it the way you did? Was it supposed to do something more that I'm not seeing?


 if (v[val][j] == 1)
     std::cout << "| X ";
 else if (v[val][j] == -1)
     std::cout << "| O ";
 else
     std::cout << "|   ";

Here is an example where you are repeating way too much. The different branches are referring to v[val][j], and repeat the result of sending something to cout, and even repeat the fact that what it sends is "| ? ". The only difference is what character is used for the ?. That is, there is only one character different in the three different branches.

This is a general thing to learn, not specific to this example.
Abstract out the thing you really want to vary.
Here, you just want to print "| ? " with ? meaning the proper character depending on another value.

That should be as simple as:
std::cout << "| " << mark_for(v[val][j]) << ' ';

This has decomposed the problem into a simple call to a function that maps a cell value to the character to be shown. You could write it like this:

char mark_for (int x)
{
    switch (x) {
        case -1: return 'O';
        case 1 : return 'X';
        default: return ' ';
    }
}

Note that this concentrates the logic of the actual problem into a simple function. It doesn't care where the value came from; that is, it has nothing to do with the board vector or j. The specific value is found by giving the parameter to the function call.

So, you started out with the program decomposed into input, logic, and draw functions, which is good. But, you did not further decompose any of those functions into simpler steps when implementing it.

Don't think of implementing a major function as a brutal thing where you grab one end and chew your way through to the end. Rather, decompose it into logical steps such that the function has a few number of steps that are just one level simpler than the original function.


It does not make good sense for your board to use a vector of vectors.
You have a fixed size for the board and have to take extra steps to set that up, since the whole point of a vector is to be variable sized and only hold what you added. The overhead of the nested vectors is significant, and requires that each row be allocated on the heap separately.

Since you have a fixed size known at compile time, this is a perfect use of arrays. You can just change std::vector to std::array and get the same two-subscript behavior you want. The arrays solution will have zero overhead and not use memory indirectly but rather will store the whole thing inside the variable itself.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ i just realised how stupid does the draw func looks lol, i will try to rewrite the code with more functions and better code. Thanks for the advices tho! \$\endgroup\$
    – ZakiMkn
    Oct 13 '21 at 17:44
  • \$\begingroup\$ @ZakiMkn keep at it! \$\endgroup\$
    – JDługosz
    Oct 14 '21 at 14:24
1
\$\begingroup\$

Binary Tic-Tac-Toe

You can greatly simplify your code by representing moves as bits.

Rather than have vectors of vector and all the loops in loops and indexes needed to search for patters bit field collapse all the logic into very simple math operations.

9 bits per player

There are 9 positions so each player needs only 9 bits to represent there current moves.

To set a played position add or bitwise OR the players bit values with player |= 1 << cell where cell is a board position 0 to 8.

To check is a position is free add the two player bit values and AND the result with the bit pos of the cell you want to check if (!((playerO + playerX) & (1 << cell))) { /* cell is empty */ }

Bit logic

This also makes game logic much simpler as all winning moves can be represent with only 8 9bit values.

Thus to check the bottom horizontal row for a win if ((player & 0b111000000) == 0b111000000) { /* player wins */ }

A drawn game can be found by checking if the sum of both players move is equal to 511 (0b111111111 or 0x1FF)

In the rewrite the bit locations from 0 to 8 represent positions on the board from top left down to bottom right

0|1|2   X| |O    Bits.......876543210            
-+-+-   -+-+-    Player X 0b101000001
3|4|5    |O|     Player O 0b000010100 
-+-+-   -+-+-
6|7|8   X| |X

The bits are from low (bit 0) to high bit (bit 8) thus setting bit 8 is 0b100000000 or 0x100 or 256 or 1 << 8

Display

To display the board you can pre define a string representing the empty board and have an array point to the position of each cell in that string.

Then for each bit set in the two player bit values set the appropriate character to X, or O

Input

Needing the player to enter 2 values to make a move is rather demanding, There are 9 position so ask for one of nine values. Show the player a board with all the numbered position to help if needed.

Rewrite

Using binary math and simplifying the display there is much less code.

I use binary notation for some of the numbers to make it easier to read if you are not comfortable with binary. Once you get used to it these can be entered as decimal, eg the wins are unsigned wins[8] {292, 146, 73, 448, 56, 7, 273, 84};

std::string movePositions = "1|2|3\n-+-+-\n4|5|6\n-+-+-\n7|8|9\n";
std::string board = "\n"
                    " | | \n"    // 1  3   5
                    "-+-+-\n"
                    " | | \n"    // 13 15  17
                    "-+-+-\n"
                    " | | \n\n"; // 25 27  29
size_t positions[9] { // 9 cells 0 - 8
    1, 3, 5, 13, 15, 17, 25, 27, 29 // position on board for each cell 0 to 8
}; 
unsigned wins[8] { // All wining plays
    0b100100100, 0b010010010, 0b001001001, // vertical wins
    0b111000000, 0b111000, 0b111,          // horizontal wins
    0b100010001, 0b1010100                 // diagonal wins
};

void draw(unsigned plyX, unsigned plyO) {
    std::string copy = board;
    size_t i = 9;
    while (i--) {
        copy[positions[i]] = plyX & 1 << i ? 'X' : plyO & 1 << i ? 'O' : ' ';
    }
    std::cout << copy;
}
bool isWinner(unsigned player) {
    for (auto wbits : wins) {
        if ((player & wbits) == wbits) { return true; }
    }
    return false;
}
bool gameInPlay(unsigned plyX, unsigned plyO) {
    bool inPlay {false};
    if (isWinner(plyX)) {          std::cout << "Player X wins.\n"; }
    else if (isWinner(plyO)) {     std::cout << "Player O wins.\n"; }
    else if (plyX + plyO == 511) { std::cout << "Game is drawn.\n"; }
    else { inPlay = true; }
    return inPlay;
}
unsigned getInput(unsigned player, char playerSide, unsigned plyB) {
    int pMove;
    unsigned usedPositions = player + plyB;
    while (true) {
        std::cout << "Player " << playerSide << " Enter move option [1-9]? ";
        std::cin >> pMove;
        if (pMove >= 1 && pMove <= 9) {
            const unsigned bit = 1 << (pMove - 1);
            if (usedPositions & bit) { std::cout << "Position " << pMove << " is occupied.\n"; }
            else { return player + bit; }
        } else { std::cout << "Enter a value between [1 - 9]\n"; }
    }
}
void play() {
    unsigned moveCount{1}; // 1 X first 0 O first
    unsigned plyX{0}, plyO{0};
    std::cout << movePositions;
    while (gameInPlay(plyX, plyO)) {
        if (moveCount++ % 2) {  plyX = getInput(plyX, 'X', plyO); }
        else { plyO = getInput(plyO, 'O', plyX); }
        draw(plyX, plyO);
    }
    std::cout << "Game done!\n";
}
\$\endgroup\$
0

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.