Text-based Tetris game - follow-up

Question

Previous question:

Text-based Tetris game

I don't have a Linux terminal to be sure whether or not I have implemented the Linux version correctly, but hopefully it's OK.

Summary of improvements:

• More OOP
• Improved the names of the classes and their functions and I hope they're OK now
• More portable, isolated platform-specific code
• Eliminated "magic numbers"

I would like to know if there is anything I should take care of before processing the code.

#include <iomanip>
#include <iostream>
#include <vector>
#include <random>
#ifdef __linux__ 
/*****************************************************************************
kbhit() and getch() for Linux/UNIX
Chris Giese <[email protected]> http://my.execpc.com/~geezer
Release date: ?
This code is public domain (no copyright).
You can do whatever you want with it.
****************************************************************************/
#include <sys/time.h> /* struct timeval, select() */
/* ICANON, ECHO, TCSANOW, struct termios */
#include <termios.h> /* tcgetattr(), tcsetattr() */
#include <stdlib.h> /* atexit(), exit() */
#include <unistd.h> /* read() */
#include <stdio.h> /* printf() */
static struct termios g_old_kbd_mode;
/*****************************************************************************
*****************************************************************************/
static void cooked(void)
{
 tcsetattr(0, TCSANOW, &g_old_kbd_mode);
}
/*****************************************************************************
*****************************************************************************/
static void raw(void)
{
 static char init;
 /**/
 struct termios new_kbd_mode;
 if (init)
 return;
 /* put keyboard (stdin, actually) in raw, unbuffered mode */
 tcgetattr(0, &g_old_kbd_mode);
 memcpy(&new_kbd_mode, &g_old_kbd_mode, sizeof(struct termios));
 new_kbd_mode.c_lflag &= ~(ICANON | ECHO);
 new_kbd_mode.c_cc[VTIME] = 0;
 new_kbd_mode.c_cc[VMIN] = 1;
 tcsetattr(0, TCSANOW, &new_kbd_mode);
 /* when we exit, go back to normal, "cooked" mode */
 atexit(cooked);
 init = 1;
}
/*****************************************************************************
*****************************************************************************/
static int _kbhit(void)
{
 struct timeval timeout;
 fd_set read_handles;
 int status;
 raw();
 /* check stdin (fd 0) for activity */
 FD_ZERO(&read_handles);
 FD_SET(0, &read_handles);
 timeout.tv_sec = timeout.tv_usec = 0;
 status = select(0 + 1, &read_handles, NULL, NULL, &timeout);
 if (status < 0)
 {
 printf("select() failed in kbhit()\n");
 exit(1);
 }
 return status;
}
/*****************************************************************************
*****************************************************************************/
static int _getch(void)
{
 unsigned char temp;
 raw();
 /* stdin = fd 0 */
 if (read(0, &temp, 1) != 1)
 return 0;
 return temp;
}
struct COORD { short X; short Y; };
bool gotoxy(unsigned short x = 1, unsigned short y = 1) {
 if ((x == 0) || (y == 0))
 return false;
 std::cout << "\x1B[" << y << ";" << x << "H";
}
void clearScreen(bool moveToStart = true) {
 std::cout << "\x1B[2J";
 if (moveToStart)
 gotoxy(1, 1);
}
inline
void print(std::string& str, COORD& coord)
{
 gotoxy(coord.X, coord.Y);
 std::cout << str << std::flush;;
}
inline
void print(TCHAR* str, COORD& coord){
 gotoxy(coord.X, coord.Y);
 std::cout << str << std::flush;;
}
inline
void print(TCHAR& c, COORD& coord){
 gotoxy(coord.X, coord.Y);
 std::cout << c << std::flush;
}
#elif _WIN32
#include <conio.h> /* kbhit(), getch() */
#include <Windows.h>
#include <tchar.h>
void clearScreen()
{
 HANDLE hStdOut;
 CONSOLE_SCREEN_BUFFER_INFO csbi;
 DWORD count;
 DWORD cellCount;
 COORD homeCoords = { 0, 0 };
 hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
 if (hStdOut == INVALID_HANDLE_VALUE) return;
 if (!GetConsoleScreenBufferInfo(hStdOut, &csbi)) return;
 cellCount = csbi.dwSize.X *csbi.dwSize.Y;
 if (!FillConsoleOutputCharacter(
 hStdOut,
 (TCHAR) ' ',
 cellCount,
 homeCoords,
 &count
 )) return;
 if (!FillConsoleOutputAttribute(
 hStdOut,
 csbi.wAttributes,
 cellCount,
 homeCoords,
 &count
 )) return;
 /* Move the cursor home */
 SetConsoleCursorPosition(hStdOut, homeCoords);
}
void gotoxy(int x, int y)
{
 COORD coord;
 coord.X = x;
 coord.Y = y;
 SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), coord);
}
static CONSOLE_SCREEN_BUFFER_INFO csbi;
COORD getXY()
{
 GetConsoleScreenBufferInfo(GetStdHandle(STD_OUTPUT_HANDLE), &csbi);
 COORD position;
 position.X = csbi.dwCursorPosition.X;
 position.Y = csbi.dwCursorPosition.Y;
 return position;
}
static DWORD cCharsWritten = 0;
inline
void print(std::string& str, COORD& coord)
{
 WriteConsoleOutputCharacter(GetStdHandle(STD_OUTPUT_HANDLE), str.c_str(), str.length(), coord, &cCharsWritten);
}
inline
void print(TCHAR* str, COORD& coord)
{
 WriteConsoleOutputCharacter(GetStdHandle(STD_OUTPUT_HANDLE), str, _tcslen(str), coord, &cCharsWritten);
}
inline
void print(TCHAR& c, COORD& coord)
{
 FillConsoleOutputCharacter(GetStdHandle(STD_OUTPUT_HANDLE), c, 1, coord, &cCharsWritten);
}
#else
#error "OS not supported!"
#endif
static 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 }
 }
};
struct NonCopyable
{
 NonCopyable() = default;
 NonCopyable(const NonCopyable &) = delete;
 NonCopyable(const NonCopyable &&) = delete;
 NonCopyable& operator = (const NonCopyable&) = delete;
};
struct Random : public NonCopyable
{
 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;
};
struct Block : public NonCopyable
{
 static const int ROTATIONS_IN_CIRCLE = 4;
 int rotation_count = 0;
 COORD coord;
 std::vector<std::vector<int>> block;
 Block()
 {
 block.resize(4, std::vector<int>(4, 0));
 coord.X = 0;
 coord.Y = 0;
 }
 void rotate()
 {
 ++rotation_count;
 while (rotation_count > ROTATIONS_IN_CIRCLE)
 {
 rotation_count -= ROTATIONS_IN_CIRCLE;
 }
 }
 int& getDim(int row, int column)
 {
 switch (rotation_count % ROTATIONS_IN_CIRCLE)
 {
 default:
 return block[row][column];
 case 1:
 return block[block.size() - column - 1][row];
 case 2:
 return block[block.size() - row - 1][block.size() - column - 1];
 case 3:
 return block[column][block.size() - row - 1];
 }
 }
 size_t size()
 {
 return block.size();
 }
};
struct Board : public NonCopyable
{
 Board()
 {
 field.resize(22, std::vector<int>(13, 0));
 coord.X = 0;
 coord.Y = 0;
 }
 COORD coord;
 std::vector<std::vector<int>> field;
 int& getDim(int row, int column)
 {
 return field[row][column];
 }
 size_t size() const
 {
 return field.size();
 }
 size_t rowSize() const
 {
 return field[0].size();
 }
};
struct Collidable : public NonCopyable
{
 Collidable()
 {
 stage.resize(22, std::vector<int>(13, 0));
 coord.X = 0;
 coord.Y = 0;
 }
 COORD coord;
 int& getDim(int row, int column)
 {
 return stage[row][column];
 }
 size_t size() const
 {
 return stage.size();
 }
 size_t rowSize() const
 {
 return stage[0].size();
 }
 std::vector<std::vector<int>> stage;
};
class Tetris : public NonCopyable
{
public:
 Tetris() 
 : board(), block(), stage()
 {};
 bool makeBlocks();
 void initField();
 void moveBlock(int, int);
 void collidable();
 bool isCollide(int, int);
 void userInput();
 bool rotateBlock();
 void spawnBlock();
 virtual void display(){};
 virtual void GameOverScreen() {};
protected:
 int y = 0;
 int x = 4;
 bool gameOver = false;
 Board board;
 Block block;
 Collidable stage;
 Random getRandom{ 0, 6 };
};
void Tetris::initField()
{
 for (size_t i = 0; i <= board.size() - 2; ++i)
 {
 for (size_t j = 0; j <= board.rowSize() - 2; ++j)
 {
 if ((j == 0) || (j == 11) || (i == 20))
 {
 board.getDim(i, j) = stage.getDim(i, j) = 9;
 }
 else
 {
 board.getDim(i, j) = stage.getDim(i, j) = 0;
 }
 }
 }
 makeBlocks();
 display();
}
bool Tetris::makeBlocks()
{
 x = 4;
 y = 0;
 int blockType = getRandom();
 for (size_t i = 0; i < block.size(); ++i)
 {
 for (size_t j = 0; j < block.size(); ++j)
 {
 block.getDim(i, j) = 0;
 block.getDim(i, j) = block_list[blockType][i][j];
 }
 }
 for (size_t i = 0; i < block.size(); i++)
 {
 for (size_t j = 0; j < block.size(); j++)
 {
 board.getDim(i, j + block.size()) = stage.getDim(i, j + block.size()) + block.getDim(i, j);
 if (board.getDim(i, j + block.size()) > 1)
 {
 gameOver = true;
 return true;
 }
 }
 }
 return false;
}
void Tetris::moveBlock(int x2, int y2)
{
 //Remove block
 for (size_t i = 0; i < block.size(); ++i)
 {
 for (size_t j = 0; j < block.size(); ++j)
 {
 board.getDim(y + i, x + j) -= block.getDim(i, j);
 }
 }
 //Update coordinates
 x = x2;
 y = y2;
 // assign a block with the updated value
 for (size_t i = 0; i < block.size(); ++i)
 {
 for (size_t j = 0; j < block.size(); ++j)
 {
 board.getDim(y + i, x + j) += block.getDim(i, j);
 }
 }
 display();
}
void Tetris::collidable()
{
 for (size_t i = 0; i < stage.size(); ++i)
 {
 for (size_t j = 0; j < stage.rowSize(); ++j)
 {
 stage.getDim(i, j) = board.getDim(i, j);
 }
 }
}
bool Tetris::isCollide(int x2, int y2)
{
 for (size_t i = 0; i < block.size(); ++i)
 {
 for (size_t j = 0; j < block.size(); ++j)
 {
 if (block.getDim(i, j) && stage.getDim(y2 + i, x2 + j) != 0)
 {
 return true;
 }
 }
 }
 return false;
}
void Tetris::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 ' ':
 rotateBlock();
 }
}
bool Tetris::rotateBlock()
{
 std::vector<std::vector<int>> tmp(block.size(), std::vector<int>(block.size(), 0));
 for (size_t i = 0; i < tmp.size(); ++i)
 {
 for (size_t j = 0; j < tmp.size(); ++j)
 {
 tmp[i][j] = block.getDim(i, j);
 }
 }
 for (size_t i = 0; i < block.size(); ++i)
 { //Rotate
 for (size_t j = 0; j < block.size(); ++j)
 {
 block.getDim(i, j) = tmp[block.size() - 1 - j][i];
 }
 }
 if (isCollide(x, y))
 { // And stop if it overlaps not be rotated
 for (size_t i = 0; i < block.size(); ++i)
 {
 for (size_t j = 0; j < block.size(); ++j)
 {
 block.getDim(i, j) = tmp[i][j];
 }
 }
 return true;
 }
 for (size_t i = 0; i < block.size(); ++i)
 {
 for (size_t j = 0; j < block.size(); ++j)
 {
 board.getDim(y + i, x + j) -= tmp[i][j];
 board.getDim(y + i, x + j) += block.getDim(i, j);
 }
 }
 display();
 return false;
}
void Tetris::spawnBlock()
{
 if (!isCollide(x, y + 1))
 {
 moveBlock(x, y + 1);
 }
 else
 {
 collidable();
 makeBlocks();
 display();
 }
}
class Game : public Tetris
{
public:
 Game() = default;
 int menu();
 virtual void gameOverScreen();
 void gameLoop();
 virtual void display();
 void introScreen();
private:
 size_t GAMESPEED = 20000;
};
void Game::gameOverScreen()
{
 COORD coord = { 0, 0 };
 coord.Y++;
 print(" ##### # # # ####### ####### # # ####### ######", coord);
 coord.Y++;
 print("# # # # ## ## # # # # # # # #", coord);
 coord.Y++;
 print("# # # # # # # # # # # # # # #", coord);
 coord.Y++;
 print("# #### # # # # # ##### # # # # ##### ######", coord);
 coord.Y++;
 print("# # ####### # # # # # # # # # #", coord);
 coord.Y++;
 print("# # # # # # # # # # # # # #", coord);
 coord.Y++;
 print(" ##### # # # # ####### ####### # ####### # #", coord);
 coord.Y += 2;
 print("Press any key and enter", coord);
 char a;
 std::cin >> a;
}
void Game::gameLoop()
{
 size_t time = 0;
 initField();
 while (!gameOver)
 {
 if (_kbhit())
 {
 userInput();
 }
 if (time < GAMESPEED)
 {
 time++;
 }
 else
 {
 spawnBlock();
 time = 0;
 }
 }
}
int Game::menu()
{
 introScreen();
 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 Game::introScreen()
{
 clearScreen();
 COORD coord = { 0, 0 };
 print("#==============================================================================#", coord);
 coord.Y++;
 print("####### ####### ####### ###### ### #####", coord);
 coord.Y++;
 print(" # # # # # # # #", coord);
 coord.Y++;
 print(" # # # # # # #", coord);
 coord.Y++;
 print(" # ##### # ###### # #####", coord);
 coord.Y++;
 print(" # # # # # # #", coord);
 coord.Y++;
 print(" # # # # # # # #", coord);
 coord.Y++;
 print(" # ####### # # # ### ##### made for fun ", coord);
 coord.Y += 4;
 coord.Y++;
 print(" <Menu>", coord);
 coord.Y++;
 print(" 1: Start Game", coord);
 coord.Y++;
 print(" 2: Quit", coord);
 coord.Y += 2;
 print("#==============================================================================#", coord);
 coord.Y++;
 print("Choose >> ", coord);
 coord.X = strlen("Choose >> ");
 gotoxy(coord.X, coord.Y);
}
void Game::display()
{
 clearScreen();
 for (size_t i = 0; i < board.size(); ++i)
 {
 for (size_t j = 0; j < board.rowSize(); ++j)
 {
 switch (board.getDim(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;
 }
 COORD coord = { 0, board.size() };
 print(" A: left S: down D: right Rotation[Space]", coord);
 if (gameOver)
 {
 clearScreen();
 gameOverScreen();
 }
}
int main()
{
 Game game;
 switch (game.menu())
 {
 case 1:
 game.gameLoop();
 break;
 case 2:
 return 0;
 case 0:
 COORD coord = { 20, 20 };
 print("Choose 1~2", coord);
 return -1;
 }
 return 0;
}

• MORTAL: updaded code gist.github.com/MORTAL2000/f4701a1f70e7e1e038fc

Answer 1

Just a few quick comments on things I saw when scanning through:

struct Block : public NonCopyable

By defining this as a struct, you make members public (unless overridden). As a general rule, it is preferred for members to be private as a default. If you are marking a member as public then you should have a specific reason for that member. That pushes you to write code that does not rely on the members being public. You do not seem to be publicly using your public members, so you might as well make this a class. Note: this is not to say that there is never a reason to use a struct or public member, just that I see no sign of such a reason here.

 block.resize(4, std::vector<int>(4, 0));
 field.resize(22, std::vector<int>(13, 0));
 stage.resize(22, std::vector<int>(13, 0));
int x = 4;
Random getRandom{ 0, 6 };
for (size_t i = 0; i <= board.size() - 2; ++i)
 for (size_t j = 0; j <= board.rowSize() - 2; ++j)
 if ((j == 0) || (j == 11) || (i == 20))
 board.getDim(i, j) = stage.getDim(i, j) = 9;
x = 4;
 COORD coord = { 20, 20 };

There still seem to be a lot of magic numbers in the code. Some of them are repeated.

Random getRandom{ 0, 6 };

could be something like

Random getRandom{ 0, blocklist.size() };

I don't understand when the following triggers:

 if (board.getDim(i, j + block.size()) > 1)
 {
 gameOver = true;

It's probably in the code somewhere, but I don't feel like tracking it down now. A comment of why we are comparing to 1 would be helpful.

• MORTAL: thanks for answer. i agree about if (board.getDim(i, j + block.size()) > 1) statement and its double for-loop actually it is not suppose to be in makeBlock function and why comparing to 1 because the board.getDim content is all zero. so when it i become 1 due to block it means the board.getDim get full. its not accurate checking but it's work somehow. i hope i made it clear. it will be clear when i apply deleting complete line if formed
• Loki Astari: Don't think I agree with: As a general rule, it is preferred for members to be private as a default. That is too generic a statement. Sometimes its useful to default to all public; sometimes it is not. Come up with something more specific or get rid of it.

Answer 2

Ok linus only is fine.
But:

#ifdef __linux__ 
..... Several pages of stuff.
#else
#error "OS not supported!"
#endif

Seems a good way to hide stuff.
Make it all up front.

#ifndef __linux__ 
#error "OS not supported!"
#endif
..... Several pages of stuff.

OK. We are good programmers and know that static storage duration objects are zero initialized. But not everybody else does.

static char init; // Default initialization of static storage duration
 // is by zero initialization so this is guaranteed to be zero.
if (init) // So we know that the first time this is executed.
 return; // It will fall through and run the following code.

But it would be nicer to the less experienced to make this explicit.

static char init = 0;
if (init) {
 return;
}

Even so this is still is a very C way of doing stuff. A constructor would be nicer.

Prefix underscore is never a good idea.

static int _kbhit(void)

This is actually a reserved name. Don't use prefix underscore. Even if you know the rules (but you don't) not everybody does. So don't do it even when you do know the rules. See What are the rules about using an underscore in a C++ identifier?. In this case: Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace

Platform dependent code:

bool gotoxy(unsigned short x = 1, unsigned short y = 1) {
 if ((x == 0) || (y == 0))
 return false;
 std::cout << "\x1B[" << y << ";" << x << "H";
}

There are whole libraries that hide terminal dependencies that do this more generically. Look up ncurses.

OK. Just found the.

#elif _WIN32

So you do have two implementations. In this case it is easier. to do.

// Interface declaration here.
... STUFF
#ifdef __linux__ 
#include "<linux File.h>"
#elif _WIN33
#include "<Windowx File.h>"
#else
#error "OS not supported!"
#endif

Static three dimensional read only list:

static std::vector<std::vector<std::vector<int>>> block_list = { /* STUFF */ };

Why not an std::array. Or just a static const C array?

• MORTAL: the original source code was using C array. i did it by vector for practicing in C++/C++11. btw cool hat