My first C++ game (snake console game)

Question

I finally finished the code for a snake game I was working on. I would like it if you were to give me some advice as to things that can be improved.

#ifndef UNICODE
#define UNICODE
#endif

#include <iostream>
#include <Windows.h>
#include <conio.h>
#include <ctime>
#include <random>
#include <queue>
#include "Snake_segment.h"

typedef std::deque<Snake_segment> Snake_container;

const enum direction {
    UP = 0,
    RIGHT,
    DOWN,
    LEFT
};

// Constant variables
int nScreenWidth;
int nScreenHeight;
const int nFieldWidth = 40;
const int nFieldHeight = 15;

int score = 0;
bool bIsHit = true;
direction dir = direction::RIGHT;

void clear(wchar_t* buf);
void update(HANDLE hConsole, Snake_container& body, wchar_t* buf);
void directionCheck(char value);
void move(Snake_container& body, wchar_t* buf);
void genFood(wchar_t* buf);
void clearOnly(wchar_t* buf);

int main(void) {

    DWORD dwbyteswritten = 0;

    HANDLE stdH = GetStdHandle(STD_OUTPUT_HANDLE);
    CONSOLE_SCREEN_BUFFER_INFO csbi;
    GetConsoleScreenBufferInfo(stdH, &csbi);

    nScreenWidth = csbi.dwSize.X;
    nScreenHeight = csbi.dwSize.Y;

    wchar_t* temp = new wchar_t[nScreenWidth * nScreenHeight];
    clear(temp);

    bool bPlay = false;
    while (true) {
        int choice;
        std::wcout << L"1. Play" << std::endl;
        std::wcout << L"2. Quit" << std::endl;
        std::cin >> choice;

        if (choice == 1) {
            bIsHit = false;
            bPlay = true;
            break;
        }

        else if (choice == 2) {
            return 0;
        }

        else {
            std::wcout << L"Invalid input!";
            WriteConsoleOutputCharacter(stdH, temp, nScreenHeight * nScreenWidth, { 0, 0 }, &dwbyteswritten);
        }
    }

    const HANDLE hConsole = CreateConsoleScreenBuffer(
        GENERIC_READ | GENERIC_WRITE,
        0,
        NULL,
        CONSOLE_TEXTMODE_BUFFER,
        NULL
    );

    // Sets up the snake body
    Snake_container body;
    Snake_segment head;
    head.posx = nFieldWidth / 2; head.posy = nFieldHeight / 2;
    body.push_back(head);
    Snake_segment tail = head;
    --tail.posx;
    body.push_back(tail);

    // Builds the game buffer and clears it
    wchar_t* buffer = new wchar_t[nScreenWidth * nScreenHeight];
    SetConsoleActiveScreenBuffer(hConsole);
    clear(buffer);

    // Generates food and draws game
    update(hConsole, body, buffer);
    genFood(buffer);


    // Main game loop
    while (!bIsHit) {
        if (_kbhit())
            directionCheck(_getch());
        move(body, buffer);
        update(hConsole, body, buffer);
        clear(buffer);

        Sleep(200);
    }

    CloseHandle(hConsole);


    if (bPlay) {
        WriteConsoleOutputCharacter(stdH, temp, nScreenHeight * nScreenWidth, { 0, 0 }, &dwbyteswritten);
        std::wcout << L"Game over!" << std::endl;
        std::wcout << L"Score: " << score << std::endl;
        Sleep(1000);
    }

    CloseHandle(stdH);

    return 0;
}

void update(HANDLE hConsole, Snake_container& body, wchar_t* buf) {

    DWORD dwBytesWritten = 0;

    // Draws the screen
    for (int i = 0; i < nFieldHeight; ++i) {
        for (int j = 0; j < nFieldWidth; ++j) {

            // Draws top and bottom walls
            if (i == 0 || i == nFieldHeight - 1) buf[i * nScreenWidth + j] = L'#';

            // Draws left and right walls
            else if (j == 0 || j == nFieldWidth - 1) buf[i * nScreenWidth + j] = L'#';

            // Draws free space
            else if (buf[i * nScreenWidth + j] != L'*') buf[i * nScreenWidth + j] = L' ';

            // Prints snake
            for (int k = 0, n = body.size(); k < n; ++k) {

                // Prints snake

                if (buf[body[0].posx + body[0].posy * nScreenWidth] == L'#') bIsHit = true;

                else if (buf[body[0].posx + body[0].posy * nScreenWidth] == L'o') bIsHit = true;

                else if (body[k].posx == j && body[k].posy == i)
                    if (k)
                        buf[i * nScreenWidth + j] = L'o';
                    else buf[i * nScreenWidth + j] = L'@';
            }
        }
    }

    for (int i = 0; i < 37; ++i)
        buf[nFieldHeight * nScreenWidth + i] = L"Use 'w, a, s, d' to change directions"[i];

    WriteConsoleOutputCharacter(hConsole, buf, nScreenWidth * nScreenHeight, { 0, 0 }, &dwBytesWritten);
}

// Clears the buffer
void clear(wchar_t* buf) {
    for (int i = 0; i < nScreenHeight; ++i) {
        for (int j = 0; j < nScreenWidth; ++j)
            if(buf[i * nScreenWidth + j] != L'*')
                buf[i * nScreenWidth + j] = L' ';
    }
}

// Changes the directions according to the value
void directionCheck(char value) {
    switch (value) {
    case 'a':
        if (dir != direction::RIGHT) dir = direction::LEFT;
        break;

    case 'w':
        if (dir != direction::DOWN) dir = direction::UP;
        break;

    case 'd':
        if (dir != direction::LEFT) dir = direction::RIGHT;
        break;

    case 's':
        if (dir != direction::UP) dir = direction::DOWN;
    }
}

// Moves the snake appropriately
void move(Snake_container& body, wchar_t* buf) {

    body[0].prevXpos = body[0].posx;
    body[0].prevYpos = body[0].posy;

    switch (dir) {
    case direction::RIGHT:
        ++body[0].posx;
        break;

    case direction::DOWN:
        ++body[0].posy;
        break;

    case direction::LEFT:
        --body[0].posx;
        break;

    case direction::UP:
        --body[0].posy;
    }

    for (int i = 1, n = body.size(); i < n; ++i) {
        body[i].prevXpos = body[i].posx;
        body[i].prevYpos = body[i].posy;
        body[i].posx = body[i - 1].prevXpos;
        body[i].posy = body[i - 1].prevYpos;
    }

    if (buf[body[0].posx + body[0].posy * nScreenWidth] == L'*') {
        Snake_segment tail_thing;
        tail_thing.posx = body[body.size() - 1].prevXpos;
        tail_thing.posy = body[body.size() - 1].prevYpos;
        body.push_back(tail_thing);
        clearOnly(buf);
        genFood(buf);
        score += 100;
    }
}

// Generates the food
void genFood(wchar_t* buf) {
    int fX;    int fY;

    do {
        time_t tim = time(NULL);
        srand(tim + rand());
        fX = rand() % (nFieldWidth - 2) + 1;
        fY = rand() % (nFieldHeight - 2) + 1;
    } while (buf[fX + fY * nScreenWidth] != L' ');

    buf[fX + fY * nScreenWidth] = L'*';
}

// Only clears * characters
void clearOnly(wchar_t* buf) {
    for (int i = 0; i < nScreenHeight; ++i) {
        for (int j = 0; j < nScreenWidth; ++j)
            if (buf[i * nScreenWidth + j] == L'*')
                buf[i * nScreenWidth + j] = L' ';
    }
}

File "Snake_segment.h" looks like this:

class Snake_segment {
public:
    int posx, posy, prevXpos, prevYpos;
};

Answer 1

First of all congratulations for this little entertaining console game.

It is simple but entertaining. I felt like I was back in the old mobile games era.

I don't have the time to rewrite all of the code but I still want to give some hints for improvements.

Here are some random observations:

Don't use global variables, they are a maintenance hazard. Consider using classes in C++ to share the data between functions (This is C++ not C).

Try to encapsulate concepts in several classes to make the maintenance of the program easier. You could have for example a Class Gameboard which describes the Gameboard and a class Snake which describes the Snake. A class for the Food. You already started doing a Snake_segment. Try to make some more. I suggest to read about C++ classes.

Also you should try to write smaller functions. A Function should ideally only do one thing not several things. This way functions are also easier to test.

Did I say test? I recommend checking out how to write unit tests. By writing tests you will realize that your functions are too big or can get divided into smaller parts. You can use a framework like gtest or sth else.

Why do you use whchar_t* for the buffer? I recommend using std::wstring.

Instead of using a deque you should check out std::vector it is the default container you should use in C++.

Both containers handle memory allocation automatically for you. Only very rarely you should feel the need for using new

this:

wchar_t* temp = new wchar_t[nScreenWidth * nScreenHeight];

can become this:

std::wstring temp(nScreenWidth * nScreenHeight, ' ');

By replacing this you can also simplify your clearOnly function.

This:

 void clearOnly(wchar_t* buf) {
    for (int i = 0; i < nScreenHeight; ++i) {
        for (int j = 0; j < nScreenWidth; ++j)
            if (buf[i * nScreenWidth + j] == L'*')
                buf[i * nScreenWidth + j] = L' ';
    }
}

Can become this:

#include <algorithm>

....

void clearOnly(std::wstring& buf) 
{
    std::replace(buf.begin(), buf.end(), L'*', L' ');
}

Some Style observations

This:

    // Draws top and bottom walls
    if (i == 0 || i == nFieldHeight - 1) buf[i * nScreenWidth + j] = L'#';

Should be this:

    // Draws top and bottom walls
    if (i == 0 || i == nFieldHeight - 1) {
        buf[i * nScreenWidth + j] = L'#';
    }

Reason: Readability

this:

int main(void) {
    ...
    return 0;
}

should be this:

int main() {
    ...
}

Reason: In C++ unlike C it is not common to write explicit void if there are no function parameters. Also for the main function the compiler automatically generates the return 0

Feel free to rework the code and post it again. I'm pretty sure you can refactor a lot...

EDIT: Refactored Code:

I ended up having time and refactored all youre code here:

Snake console game in C++

I will edit here later when i find time what other suggestions for improvements i could find while i tryed to understand youre program.

EDIT

use namespaces: It is good practice in C++ wrapping youre programs into youre own namespace. This avoids name conflicts with existing functions from libraries.

Don't use std::endl: std::endl adds a newline and flushes the buffer. Most of the time you only want a simple newline. You get it by replacing std::endl with the newline sign '\n' (like in c). Why bother? https://stackoverflow.com/questions/213907/c-stdendl-vs-n

seperate different tasks from each other: That way parts of youre program can be even reused in other projects. A good example is youre not portable output to the console. The output is all over the place mixed with the other logic of of the program. This way you can't easy port the program to annother output source (for example a gui). When i rewrote the program i packed all the not portable output stuff in one place from the other tasks.

Also by writting everything connected with each other it is a big headache to understand whats going on in the code. Take this code. Forget it for a year and try to figure out what it does. Probaly its hard again to get into it.

It took me quite some time to untie all the knots to reveal what was really going on in youre program.

How could you organize the snake game better? I did the following:

Defining a struct Element:

struct Element {
    bool hasSnakeSegment{ false };
    bool hasSnakeHead{ false };
    bool hasWall{ false };
    bool hasFood{ false };
};

This Element can either have a snakeSegment, a snakeHead, a Wall or food. We can easily check with this whats going on on each field.

Then i defined a Point class for the Elements of the Snake and the SnakeSegment containing the previous and current poition of the segments:

struct Point {
    int x;
    int y;
};

struct SnakeSegment
{
    Point pos{ 0 , 0 };
    Point prev{ pos };
};

This SnakeSegments of course for the Snake:

class Snake
{
public:
    Snake(int boardWidth, int boardHeight);

    std::vector<SnakeSegment> getBody() const;

    void moveRight();
    void moveDown();
    void moveLeft();
    void moveUp();
    void grow();

private:
    void safeCurrentPosToLastOfFirstElement();
    void moveRemainingElements();

    std::vector<SnakeSegment> mBody;
};

std::vector<SnakeSegment> initSnake(int fieldWidth, int fieldHeight);

The Snake class defines were the Snake is on the Board and how to move it arround. Also we can grow the snake.

Then I defined the Board. This is were the game actions take place:

class Board
{
public:
    Board(int width, int height);

    void placeFood();
    void updateSnakePosition();
    bool snakeHitFood() const;
    void eatFood();
    void growSnake();
    bool snakeHitWall() const;
    bool snakeHitSnake() const;
    void moveSnake(SnakeDirection snakeDirection);

    void debugPrintSnakeCoordinates();
private:
    std::vector<std::vector<Element>> initFieldWithWalls(int width, int height);
    void removeOldSnakePosition(const std::vector<SnakeSegment>& body);
    void addNewSnakePosition(const std::vector<SnakeSegment>& body);

    Snake mSnake;
    std::vector<std::vector<Element>> mField;

    std::random_device mRandomDevice;
    std::default_random_engine mGenerator;
    std::uniform_int_distribution<int> mWidthDistribution;
    std::uniform_int_distribution<int> mHeightDistribution;

    friend std::wostream& operator<<(std::wostream& os, const Board& obj);
};

std::wostream& operator<<(std::wostream& os, const Board& obj);

Then i defined functions how to display the game in the console. If needed they can be replaced with other functions if we want to dsiplay on annother thing than a console.

The board and the output functions get used by the runGame function. So the main becomes only this:

#include "Game.h"

#include <iostream>

int main() 
try {
    snakeGame::runGame();
    return 0;
}
catch (...) {
    std::wcerr << "unknown error " << "\n";
    std::wcin.get();
}

So the main logic of the programm can be read in the runGame function:

void runGame()
{
    for (;;) {

        if (askUserToEndGame()) {
            return;
        }

        constexpr auto fieldWidth = 40;
        constexpr auto fieldHeight = 15;

        Board board{ fieldWidth, fieldHeight };
        board.updateSnakePosition();
        board.placeFood();
        SnakeDirection snakeDirection = SnakeDirection::right;

        long long score{ 0 };
        long long points{ 100 };
        auto delay(300);

        bool wasPausedInLastLoop{ false };
        for (;;) {
            putCursorToStartOfConsole();
            printBoardWithStats(board, score, delay);

            if (wasPausedInLastLoop) {
                // If we don't do this and print pause to the console by 
                // pressing p during the game the pause statement will 
                // still be printed because  during the game the pause 
                // statement will still be printed because during the game 
                // the pause statement will still be printed because 
                // during the game the pause statement will still be 
                // printed because we start printing from the beginning of
                // the console and now the total string printed to the 
                // console would be one row lower.
                std::wcout << L"                                     \n";
                wasPausedInLastLoop = false;
            }

            if (keyWasPressed()) {
                auto key = getKey();

                if (key == 'p') {
                    wasPausedInLastLoop = true;
                    std::wcout << L"#####PAUSED#####\n";
                    pauseUntilPauseKeyPressedAgain();
                }
                else {
                    snakeDirection = updateDirection(key, snakeDirection);
                }
            }

            board.moveSnake(snakeDirection);

            if (board.snakeHitFood()) {
                board.eatFood();
                board.growSnake();
                board.placeFood();
                score += points;
                points *= 2;
                delay -= 5;
            }
            else if (board.snakeHitWall() || board.snakeHitSnake()) {
                break;
            }
            board.updateSnakePosition();

            std::this_thread::sleep_for(std::chrono::milliseconds{ delay });
        }

        printGameOverWithScore(score);
    }
}

}

Notice here how the low level stuff doesn't show up because it is encapsulated in other functions the main calls. I don't say my implementation is perfect but i hope it gives some insight how to seperate tasks.

For the full code see this: Snake console game in C++ and feel free to also discuss my solution.

Answer 2

You include the modern random.

#include <random>

But in your code you use the old srand() and rand() functions. Also your usage of these functions is not correct.

        time_t tim = time(NULL);
        srand(tim + rand());
        fX = rand() % (nFieldWidth - 2) + 1;
        fY = rand() % (nFieldHeight - 2) + 1;

Here you are abusing the seeding of rand. The point about seeding is to have a starting point. Once you have established a starting point the following sequence of number should have an even distribution and be somewhat randomish (Lets not get into the argument that rand is not good at either that's what it was supposed to be). By re-seeding before each call to rand you are throwing away any chance at even distribution.

The standard argument is that you should use srand() once in the application (just after startup is good). Then simply call rand() when you need a new value.

int main()
{
    srand(time());
    ...
    // CODE that uses rand()
}

Now coming back to the problem with rand() family. We have all know that rand has been pretty broken for a while (its fine for simple problems (like games like this)). But as a result the modern <random> library was introduced that has a much better random library and it is simply just a much better idea to use this new library (even in small games like this).

int main()
{
    std::default_random_engine generator;
    std::uniform_int_distribution<int>   widthDistribution(1,nFieldWidth-1);
    std::uniform_int_distribution<int>   heightDistribution(1,nFieldHeight-1);

    // Some stuff

    fX = widthDistribution(generator);
    fY = heightDistribution(generator);

---

Sure:

typedef std::deque<Snake_segment> Snake_container;

The modern way of doing this is:

using Snake_container = std::deque<Snake_segment>;

Personally not a fan of "Snake Case"

---

These are not const!!!

// Constant variables
int nScreenWidth;
int nScreenHeight;

---

OK. So this is a C application (that happens to use some C++ features).

void clear(wchar_t* buf);
void update(HANDLE hConsole, Snake_container& body, wchar_t* buf);
void directionCheck(char value);
void move(Snake_container& body, wchar_t* buf);
void genFood(wchar_t* buf);
void clearOnly(wchar_t* buf);

If we created some class types we can group these function somewhat more logically and potentially isolate the variables so you don't accidently cause tight coupling between them.

I can see:

• Screen Object
• Snake Object (That can be drawn on a screen)

  * There seems to be a wchar_t buffer being passed around.

Manual memory management:

    wchar_t* temp = new wchar_t[nScreenWidth * nScreenHeight];

This is a bad idea. If there is an exception it leaks (OK in this context maybe not) but it is a bad habit. Get used to using containers (or smart pointers) when you need dynamic allocation. This simply looks like a buffer. So use std::vector

    std::vector<wchar_t>  temp(nScreenWidth * nScreenHeight);

All memory management handeled.

In modern C++ it is very rare to see naked new/delete.

---

Always check that the read worked.

        std::cin >> choice;

        // Should be:

        if ((std::cin >> choice) && (choice == 1 || choice ==2)) {
           // user question worked.
        }
        else {
           // user input failed.

}

Looks like a snake constructor:

    // Sets up the snake body
    Snake_container body;
    Snake_segment head;
    head.posx = nFieldWidth / 2; head.posy = nFieldHeight / 2;
    body.push_back(head);
    Snake_segment tail = head;

You should isolate this code in its own class.

---

Answer 3

First on my Windows 10 computer in both Visual Studio 2015 and Visual Studio 2017 the console is killed by this line in the update function.

    WriteConsoleOutputCharacter(hConsole, buf, nScreenWidth * nScreenHeight, { 0, 0 }, &dwBytesWritten);

This may have to do with settings on my computer.

Second I get this warning message in both VS 2015 and 2017:

warning C4244: 'argument': conversion from 'time_t' to 'unsigned int', possible loss of data

on this line in the genFood() function.

        srand(tim + rand());

It is generally not a good practice to ignore warning messages or disable warning messages.

Is there only one food item expected? That is all genFood() is placing in the buffer if food is represented by * (asterisk)?

The function srand() only needs to be called once per game after that rand() has been seeded and will generate different numbers each time. The call to srand() can probably be moved to main().

Class Versus Struct
C++ has other object types besides classes. One such object type is struct. In a struct by default all fields are public. A struct can also contain methods.

There is no reason to make Snake_segment a class, it has no methods, no constructor and no destructor.

Constants
Having global constants such as nFieldWidth and nFieldHeight are good, however, to the person reviewing the code they look like variables. It might be better to make their names all CAPITALS to show that they are global constants.

Complexity
This has been discussed in another answer, but there are clearly multiple functions in main() that should be in their own function. The code to get the user input including the while(true) loop should be in it's own function.

Another possible function is the initialization of the board.

The main game loop is also another good function.

As programs grow larger the main function becomes responsible for processing, each action of main should probably be encapsulated in a function. The primary job of main is:
- process any command line arguements
- set up for the main processing
- execute the main processing
- clean up after the program has finished
- handle any exceptions that are thrown (this might be handled any multiple levels in the program).

Style
As mentioned in another answer, it might be better to have the then clause of an if statement on a second line and to wrap it in braces. This allows for additional code to be added at a later time without changing the structure of the program.