6
\$\begingroup\$

I have finished a simple Tic Tac Toe game, and I'd like if anyone was to give advice to how I could improve my code.

main.cpp

#ifndef UNICODE
#define UNICODE
#endif

#include "gameInfo.h"

int main() {

    // Gets the screen buffer size (for somewhat of compatability)
    CONSOLE_SCREEN_BUFFER_INFO csbi;
    GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi);

    // Run the game
    gameInfo game(csbi.dwSize);
    game.start();
}

gameInfo.h

#pragma once

#include <string>
#include <Windows.h>

class gameInfo {
public:
    gameInfo(COORD size);
    ~gameInfo();
    void start();

private:

    // MEMBER VARIABLES

    // Defines the active player
    const enum playerID {
        P1 = 0,
        P2
    };

    // Defines the state of game over screen
    const enum gameOverState {
        PLAYING = -1,
        WIN,
        LOSE,
        STALEMATE
    };

    // Define sizes for screen buffer and board
    const COORD bufSize;
    const COORD boardSize;

    // Array whose elements are outputted in destructor
    const std::wstring GOArray[3] = {
        L"P1 wins!",
        L"P1 loses!",
        L"Stalemate!"
    };

    // Different member variables
    gameOverState GOState;
    COORD mouseCoords;
    std::wstring screenbuf;
    playerID activePlayer;
    HANDLE gameHandle;
    bool bGameOver;

    // GAME FUNCTIONS

    // Clears screen buffer
    void clrscr();

    /* draws map:

         | | 
        -----
         | | 
        -----
         | | 

        is how it should look
    */
    void drawMap();

    // Moves the cursor
    void movecursor(char input);

    // Draws the symbol in the correct cell
    void drawSymbol();

    // Checks the cell and returns a bool if true or false
    bool checkcell(COORD position) const;

    // Processes input
    void processinput();

    // Updates screen
    void update() const;

    void drawInfo();

    // Checks and sets the game over state
    void checkGameOverState();

};

gameInfo.cpp

#include "gameInfo.h"

#include <iostream>
#include <conio.h>

// Class constructor
gameInfo::gameInfo(COORD size)
    : activePlayer(P1), bufSize(size), boardSize({ 5, 5 }), bGameOver(true),
    mouseCoords({ 0, 0 }), GOState(PLAYING) {

    int choice;
    std::wcout << L"1: Play" << std::endl;
    std::wcout << L"2: Quit" << std::endl;

    // Tests if input is correct
    if (std::cin >> choice && (choice == 1 || choice == 2)) {
        if (choice == 1)
            bGameOver = false;      
    }

    std::wstring temp(size.X * size.Y, L' ');
    screenbuf = temp;

    gameHandle = CreateConsoleScreenBuffer(
        GENERIC_READ | GENERIC_WRITE,
        0,
        NULL,
        CONSOLE_TEXTMODE_BUFFER,
        NULL
    );

    SetConsoleActiveScreenBuffer(gameHandle);
}

// Class destructor
gameInfo::~gameInfo() {
    CloseHandle(gameHandle);    

    for (int i = 0; i < 10; ++i)
        std::wcout << L"\n\n\n\n\n\n\n" << std::endl;

    std::wcout << L"Game over! " << GOArray[GOState] << std::endl;
    Sleep(2000);
}

// Clears the screen buffer
void gameInfo::clrscr() {
    for (int i = 0; i < bufSize.Y; ++i) {
        for (int j = 0; j < bufSize.X; ++j)
            screenbuf[i * bufSize.X + j] = L' ';
    }
}

// Outputs the buffer to the console
void gameInfo::update() const {

    DWORD dwBytesWritten = 0;

    WriteConsoleOutputCharacter(
        gameHandle,
        screenbuf.c_str(),
        bufSize.X * bufSize.Y,
        { 0, 0 },
        &dwBytesWritten
    );
}



// Moves the cursor to the right cell
void gameInfo::movecursor(char input) {
    switch (input) {
    case 'w':
        if (mouseCoords.Y - 1 > -1)
            --mouseCoords.Y;
        break;

    case 'd':
        if (mouseCoords.X + 1 < boardSize.X)
            ++mouseCoords.X;
        break;

    case 's':
        if (mouseCoords.Y + 1 < boardSize.Y)
            ++mouseCoords.Y;
        break;

    case 'a':
        if (mouseCoords.X - 1 > -1)
            --mouseCoords.X;
    }

    SetConsoleCursorPosition(gameHandle, mouseCoords);
}

// Draws the correct symbol
void gameInfo::drawSymbol() {
    if (activePlayer == P1) {
        screenbuf[mouseCoords.Y * bufSize.X + mouseCoords.X] = L'x';
        activePlayer = P2;
    }

    else {
        screenbuf[mouseCoords.Y * bufSize.X + mouseCoords.X] = L'o';
        activePlayer = P1;
    }


}

// Checks if cell is clear
bool gameInfo::checkcell(COORD position) const {

    if (screenbuf[position.Y * bufSize.X + position.X] == L' ')
        return true;

    return false;
}

// Handles input
void gameInfo::processinput() {
    if (_kbhit()) {

        if (_getch() == 'x') {
            if (checkcell(mouseCoords))
                drawSymbol();
        }

        else movecursor(_getch());
    }
}

void gameInfo::checkGameOverState() {
    for (int i = 0; i < boardSize.Y; ++i) {

        int countx = 0;
        int counto = 0;
        for (int j = 0; j < boardSize.X; ++j) {
            if (screenbuf[i * bufSize.X + j] == L'x')
                ++countx;
            else if (screenbuf[i * bufSize.X + j] == L'o')
                ++counto;
        }

        if (countx == 3) {
            GOState = WIN;
            bGameOver = true;
        }
        else if (counto == 3) {
            GOState = LOSE;
            bGameOver = true;
        }
    }

    for (int i = 0; i < boardSize.Y; ++i) {

        int countx = 0;
        int counto = 0;
        for (int j = 0; j < boardSize.X; ++j) {
            if (screenbuf[j * bufSize.X + i] == L'x')
                ++countx;
            else if (screenbuf[j * bufSize.X + i] == L'o')
                ++counto;
        }

        if (countx == 3) {
            GOState = WIN;
            bGameOver = true;
        }
        else if (counto == 3) {
            GOState = LOSE;
            bGameOver = true;
        }
    }

    // This monstrosity checks diagonally
    if ((screenbuf[0 * bufSize.X + 0] == L'x' &&
         screenbuf[2 * bufSize.X + 2] == L'x' &&
         screenbuf[4 * bufSize.X + 4] == L'x') || 

        (screenbuf[0 * bufSize.X + 4] == L'x' &&
         screenbuf[2 * bufSize.X + 2] == L'x' &&
         screenbuf[4 * bufSize.X + 0] == L'x')) {

        GOState = WIN;
        bGameOver = true;
    }

    else if ((screenbuf[0 * bufSize.X + 0] == L'o' &&
              screenbuf[2 * bufSize.X + 2] == L'o' &&
              screenbuf[4 * bufSize.X + 4] == L'o') ||

             (screenbuf[0 * bufSize.X + 4] == L'o' &&
              screenbuf[2 * bufSize.X + 2] == L'o' &&
              screenbuf[4 * bufSize.X + 0] == L'o')) {

        GOState = LOSE;
        bGameOver = true;
    }

    // Checks for stalemates
    int count = 0;
    for (int i = 0; i < boardSize.Y; ++i) {
        for (int j = 0; j < boardSize.X; ++j)
            if (screenbuf[i * bufSize.X + j] != L' ')
                ++count;
    }

    if (count == boardSize.X * boardSize.Y) {
        GOState = STALEMATE;
        bGameOver = true;
    }
}

void gameInfo::drawInfo() {
    std::wstring controls = L"Press 'w, a, s, d' to move the cursor. Press x to mark a cell";
    for (int i = 0, n = controls.size(); i < n; ++i) {
        screenbuf[6 * bufSize.X + i] = controls[i];
    }
}

// Draws the map in the buffer
void gameInfo::drawMap() {
    for (int i = 0; i < boardSize.Y; ++i) {
        for (int j = 0; j < boardSize.X; ++j) {

            // Draws the right character for each position
            if (i % 2 != 0) screenbuf[i * bufSize.X + j] = L'-';

            else if (j % 2 != 0) screenbuf[i * bufSize.X + j] = L'|';

            else screenbuf[i * bufSize.X + j] = L' ';
        }
    }
}

// Starts the game and runs the game loop
void gameInfo::start() {

    drawMap();
    while (!bGameOver) {
        processinput();
        drawInfo();
        update();
        checkGameOverState();
    }
    clrscr();
}
\$\endgroup\$
  • \$\begingroup\$ I was able to run it on VS 2019, and i am on windows 10 x64, although it should be usable also on x86 machines \$\endgroup\$ – Nadpher May 6 at 19:48
  • 1
    \$\begingroup\$ Oh boy, don't use that ancient stuff: #include <conio.h> \$\endgroup\$ – πάντα ῥεῖ May 6 at 19:50
  • \$\begingroup\$ It's the most reliable way i know to get input without interrupting the main thread, really.. \$\endgroup\$ – Nadpher May 6 at 19:51
6
\$\begingroup\$

First, congratulations for writing a complete program. It's certainly the best way to learn, and the most gratifying. But I would also say that it's only the beginning: it's time to look back at your code, at the creative process, and think about what you could do to make your code more maintainable, and easier to improve.

I believe that your code should be more structured. If you look at it closely, you'll see that the screen buffer is way more than a screen buffer: it's a board representation that is also used to apply the game's rules. When you think of it, there's no obvious reason why a screen buffer should be the best tool to implement the game logic with; and when you speak of the monstrosity of the diagonal check, it probably means you have your doubts.

Now if you divide your program into two components: one about the display and other interactions with the user, and one about the game logic itself, you can choose different representations for different uses, provided that you enable communication between both through a well established interface.

So let's keep the screen buffer as a wide character string, but add a different board representation for the game logic part. You could make use of a mere integer to fit all necessary information: nine bits by player and one extra bit to indicate whose turn it is, so 19 bits. It's not about space though (although if you add AI to your game, you'll need to analyze a lot of boards to choose the next move), but about convenience:

constexpr std::array<int, 8> end_games = {
    //lines
    0b111000000,
    0b000111000,
    0b000000111,
    //columns
    0b100100100,
    0b010010010,
    0b001001001,
    //diagonals
    0b100010001,
    0b001010100,
};

constexpr int stalemate = 0b111111111;

bool check_end_game(int board) {
    return std::any_of(end_games.begin(), end_games.end(), [board](auto end_game) {
        return (end_game & extract_current_player_board(board)) == end_game;
    });
}

bool check_stalemate(int board) {
    return (extract_first_player_board(board) | extract_second_player_board(board)) == stalemate; 
}

Of course, many other representations can be considered. But that's the point: if you separate your game into several components, communicating through a stable minimal interface, you can then modify each component independently, depending only on what is best for the task at hand, be it display or computations.

Communication between both components can also take various forms. One of the most flexible way would be for the user interface part to communicate the intended move to the game logic part:

class Board {
public:
    bool intended_move(int row, int col);
//...
};

If the move is valid, the Board modifies the game data and returns true; on receiving true, the user interface modifies the screen buffer and refreshes the screen. If it isn't valid, it returns false and the user interface component acts accordingly, signaling the move isn't valid and requesting another.

\$\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.