Text-based Tetris game

Question

How can I improve this game?

#include <iomanip>
#include <iostream>
#include <vector>
#include <random>
#include <conio.h>


struct Random
{
    Random(int min, int max)
        : mUniformDistribution(min, max)
    {}

    int operator()()
    {
        return mUniformDistribution(mEngine);
    }

    std::default_random_engine mEngine{ std::random_device()() };
    std::uniform_int_distribution<int> mUniformDistribution;
};

std::vector<std::vector<int>> stage(22, std::vector<int>(13, 0));
std::vector<std::vector<int>> block = 
{ 
    { 0, 0, 0, 0 },
    { 0, 0, 0, 0 },
    { 0, 0, 0, 0 },
    { 0, 0, 0, 0 } 
};

std::vector<std::vector<int>> field(22, std::vector<int>(13, 0));
// coordinate
int y = 0; 
int x = 4;
bool gameover = false;
size_t GAMESPEED = 20000; 

Random getRandom{ 0, 6 };

std::vector<std::vector<std::vector<int>>> block_list =
{ 
    { 
        { 0, 1, 0, 0 },
        { 0, 1, 0, 0 },
        { 0, 1, 0, 0 },
        { 0, 1, 0, 0 } 
    },
    { 
        { 0, 0, 0, 0 },
        { 0, 1, 1, 0 },
        { 0, 1, 0, 0 },
        { 0, 1, 0, 0 } 
    },
    { 
        { 0, 0, 1, 0 },
        { 0, 1, 1, 0 },
        { 0, 1, 0, 0 },
        { 0, 0, 0, 0 } 
    },
    { 
        { 0, 1, 0, 0 },
        { 0, 1, 1, 0 },
        { 0, 0, 1, 0 },
        { 0, 0, 0, 0 } 
    },
    { 
        { 0, 0, 0, 0 },
        { 0, 1, 0, 0 },
        { 1, 1, 1, 0 },
        { 0, 0, 0, 0 } 
    },
    { 
        { 0, 0, 0, 0 },
        { 0, 1, 1, 0 },
        { 0, 1, 1, 0 },
        { 0, 0, 0, 0 } 
    },
    { 
        { 0, 0, 0, 0 },
        { 0, 1, 1, 0 },
        { 0, 0, 1, 0 },
        { 0, 0, 1, 0 } 
    } 
};

int menu(); 
int gameOver(); 
void title(); 
void gameLoop(); 
void display(); 
bool makeBlocks(); 
void initGame(); 
void moveBlock(int, int); 
void collidable();
bool isCollide(int, int); 
void userInput(); 
bool rotateBolck(); 
void spwanBlock(); 

int main()
{
    switch (menu())
    {
    case 1:
        gameLoop();
        break;
    case 2:
        return 0;
    case 0:
        std::cerr << "Choose 1~2" << std::endl;
        return -1;
    }
    return 0;
}

int gameOver()
{
    using namespace std;

    char a;
    cout << " #####     #    #     # ####### ####### #     # ####### ######\n" ;
    cout << "#     #   # #   ##   ## #       #     # #     # #       #     #\n";
    cout << "#        #   #  # # # # #       #     # #     # #       #     #\n";
    cout << "#  #### #     # #  #  # #####   #     # #     # #####   ######\n";
    cout << "#     # ####### #     # #       #     #  #   #  #       #   #\n";
    cout << "#     # #     # #     # #       #     #   # #   #       #    #\n";
    cout << " #####  #     # #     # ####### #######    #    ####### #     #\n";
    cout << "\n\nPress any key and enter\n";
    cin >> a;
    return 0;
}

void gameLoop()
{
    size_t time = 0;
    initGame();

    while (!gameover) 
    { 
        if (kbhit()) 
        {
            userInput();
        }

        if (time < GAMESPEED)
        {
            time++;
        }
        else 
        {
            spwanBlock();
            time = 0;
        }
    }

}

int menu()
{
    title();

    int select_num = 0;

    std::cin >> select_num;

    switch (select_num)
    {
    case 1:
    case 2:
    case 3:
        break;
    default:
        select_num = 0;
        break;
    }

    return select_num;
}

void title()
{
    using namespace std;

    system("cls");

    cout << "#==============================================================================#\n";

    cout << "####### ####### ####### ######    ###    #####\n";
    cout << "   #    #          #    #     #    #    #     #\n";
    cout << "   #    #          #    #     #    #    #\n";
    cout << "   #    #####      #    ######     #     #####\n";
    cout << "   #    #          #    #   #      #          #\n";
    cout << "   #    #          #    #    #     #    #     #\n";
    cout << "   #    #######    #    #     #   ###    #####\t\tmade for fun \n";
    cout << "\n\n\n\n";

    cout << "\t<Menu>\n";
    cout << "\t1: Start Game\n\t2: Quit\n\n";

    cout << "#==============================================================================#\n";
    cout << "Choose >> ";
}

void display()
{
    system("cls");

    for (size_t i = 0; i < 21; i++) 
    {
        for (size_t j = 0; j < 12; j++) 
        {
            switch (field[i][j]) 
            {
            case 0:
                std::cout << " " << std::flush;
                break;
            case 9:
                std::cout << "@" << std::flush;
                break;
            default:
                std::cout << "#" << std::flush;
                break;
            }
        }
        std::cout << std::endl;
    }

    std::cout << "\n\tA: left\tS: down\tD: right \t Rotation[Space]";

    if (gameover)
    {
        system("cls");
        gameOver();
    }
}

void initGame()
{
    for (size_t i = 0; i <= 20; i++)
    {
        for (size_t j = 0; j <= 11; j++)
        {
            if ((j == 0) || (j == 11) || (i == 20)) 
            {
                field[i][j] = stage[i][j] = 9;
            }
            else
            {
                field[i][j] = stage[i][j] = 0;
            }
        }
    }

    makeBlocks();

    display();
}

bool makeBlocks()
{
    x = 4;
    y = 0;

    int blockType = getRandom();

    for (size_t i = 0; i < 4; i++)
    {
        for (size_t j = 0; j < 4; j++)
        {
            block[i][j] = 0;
            block[i][j] = block_list[blockType][i][j];
        }
    }

    for (size_t i = 0; i < 4; i++)
    {
        for (size_t j = 0; j < 4; j++)
        {
            field[i][j + 4] = stage[i][j + 4] + block[i][j];

            if (field[i][j + 4] > 1)
            {
                gameover = true;
                return true;
            }
        }
    }
    return false;
}

void moveBlock(int x2, int y2)
{

    //Remove block
    for (size_t i = 0; i < 4; i++)
    {
        for (size_t j = 0; j < 4; j++)
        {
            field[y + i][x + j] -= block[i][j];
        }
    }
    //Update coordinates
    x = x2;
    y = y2;

    // assign a block with the updated value
    for (size_t i = 0; i < 4; i++)
    {
        for (size_t j = 0; j < 4; j++)
        {
            field[y + i][x + j] += block[i][j];
        }
    }

    display();
}

void collidable()
{
    for (size_t i = 0; i<21; i++)
    {
        for (size_t j = 0; j<12; j++)
        {
            stage[i][j] = field[i][j];
        }
    }
}

bool isCollide(int x2, int y2)
{
    for (size_t i = 0; i < 4; i++)
    {
        for (size_t j = 0; j < 4; j++)
        {
            if (block[i][j] && stage[y2 + i][x2 + j] != 0)
            {
                return true;
            }
        }
    }
    return false;
}

void userInput()
{
    char key;

    key = getch();

    switch (key)
    {
    case 'd':
        if (!isCollide(x + 1, y))
        {
            moveBlock(x + 1, y);
        }
        break;
    case 'a':
        if (!isCollide(x - 1, y))
        {
            moveBlock(x - 1, y);
        }
        break;
    case 's':
        if (!isCollide(x, y + 1))
        {
            moveBlock(x, y + 1);
        }
        break;
    case ' ':
        rotateBolck();
    }
}

bool rotateBolck()
{
    std::vector<std::vector<int>> tmp(4, std::vector<int>(4, 0));

    for (size_t i = 0; i < 4; i++)
    { //Save temporarily block
        for (size_t j = 0; j < 4; j++)
        {
            tmp[i][j] = block[i][j];
        }
    }

    for (size_t i = 0; i < 4; i++)
    { //Rotate
        for (size_t j = 0; j < 4; j++)
        {
            block[i][j] = tmp[3 - j][i];
        }
    }

    if (isCollide(x, y))
    { // And stop if it overlaps not be rotated
        for (size_t i = 0; i < 4; i++)
        {
            for (size_t j = 0; j < 4; j++)
            {
                block[i][j] = tmp[i][j];
            }
        }
        return true;
    }

    for (size_t i = 0; i < 4; i++)
    {
        for (size_t j = 0; j < 4; j++)
        {
            field[y + i][x + j] -= tmp[i][j];
            field[y + i][x + j] += block[i][j];
        }
    }

    display();

    return false;
}

void spwanBlock()
{
    if (!isCollide(x, y + 1))
    {
        moveBlock(x, y + 1);
    }
    else
    {
        collidable();
        makeBlocks();
        display();
    }
}

Answer 1

I see a number of things that can help you improve this code.

Don't use system("cls")

There are two reasons not to use system("cls") or system("pause"). The first is that it is not portable to other operating systems which you may or may not care about now. The second is that it's a security hole, which you absolutely must care about. Specifically, if some program is defined and named cls or pause, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate these into a separate functions cls() and pause() and then modify your code to call those functions instead of system. Then rewrite the contents of those functions to do what you want using C++. For example, if your terminal supports ANSI Escape sequences, you could use this:

void cls()
{
    std::cout << "\x1b[2J";
}

Isolate platform-specific code

In this code, there are several things that are DOS/Windows only including #include <conio.h> and the getch() and kbhit() functions within that, and also system("cls"); that I've already mentioned. Your code runs successfully on Linux if I supply those missing functions, but it would be nice if there were an #ifdef WINDOWS already in the code so that one could recompile without having to alter the source code.

Fix spelling errors

The code has spwanBlock() instead of spawnBlock() and rotateBolck() instead of rotateBlock(). These kinds of typos don't bother the compiler at all, but they will bother human readers of the code and make it a little more difficult to understand and maintain.

Use more objects

The game is written much more in the procedural style of C rather than in the object-oriented style of C++. The game itself could be an object, with most of the procedures as functions of that object. This would reduce coupling and make the program easier to understand. Also, each of the blocks could quite obviously be an object. It would also eliminate the global variables that now occupy the code, such as x, y, and gameover.

Reduce variable scope as much as possible

Almost all of the global variables can be eliminated by using objects, but if any remain, they should be static to limit them to file scope, or even better, as noted by @glampert in a comment, use an anonymous namespace.

Use string concatenation

The gameOver() and title() functions both have many repeated lines where the ostream operator<< is used multiple times with std::cout and a fixed string. Those multiple calls don't need to happen. You could simply rely on the fact that C++ merges separate constant strings automatically. For example, here is a recoded gameOver():

void gameOver()
{
    std::cout << "\n"
            " #####     #    #     # ####### ####### #     # ####### ######\n" 
            "#     #   # #   ##   ## #       #     # #     # #       #     #\n"
            "#        #   #  # # # # #       #     # #     # #       #     #\n"
            "#  #### #     # #  #  # #####   #     # #     # #####   ######\n"
            "#     # ####### #     # #       #     #  #   #  #       #   #\n"
            "#     # #     # #     # #       #     #   # #   #       #    #\n"
            " #####  #     # #     # ####### #######    #    ####### #     #\n"
            "\n\nPress any key and enter\n";
    char a;
    std::cin >> a;
}

Note a few simple things here. First, there's only a single input and a single output call, so the using namespace std; didn't seem worthwhile and was removed. Second, the dummy variable a was declared just before use instead of at the beginning of the function. Third, because the return variable was neither useful nor used, it has been omitted.

Eliminate "magic numbers"

The code, including the implementation of initGame() is full of "magic numbers" -- that is raw numbers in the text that don't have obvious meaning. For example:

void initGame()
{
    for (size_t i = 0; i <= 20; i++)
    {
        for (size_t j = 0; j <= 11; j++)
        {
            if ((j == 0) || (j == 11) || (i == 20)) 
            {
                field[i][j] = stage[i][j] = 9;
            }

It's not at all apparent what the meaning is of 20 or 11 or 9 in this code. Meaningfully named constants would be a better way to do this.

Use const where practical

Variables such as GAMESPEED are never altered by the program and should therefore be declared const.

Use better timekeeping

The current gameLoop() routine uses a very crude increment loop to do timing. This would be much better implemented using something from std::chrono or better, the whole thing could be rewritten using an asynchronous programming model instead of a synchronous scheme.

Enhance the gameplay

The original Tetris game would delete a row once it became completely filled in, but this code doesn't do that. It also kept score and the original game was in color. You could add all of these things, the first two by making functional changes to the code, and the last item by means of the previously mentioned ANSI Escape sequences. It would make for a more interesting game.

Answer 2

The formatting on your switch statements look weird to me.

        switch (field[i][j]) 
        {
        case 0:
            std::cout << " " << std::flush;
            break;
        case 9:
            std::cout << "@" << std::flush;
            break;
        default:
            std::cout << "#" << std::flush;
            break;
        }

I would rather have the case statements indented,

        switch (field[i][j]) 
        {
            case 0:
                std::cout << " " << std::flush;
                break;
            case 9:
                std::cout << "@" << std::flush;
                break;
            default:
                std::cout << "#" << std::flush;
                break;
        }

that way it looks like the Case statements are children of the Switch, as they should be.