Text-based Tetris game
up vote
8
down vote
favorite
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 << "nnPress any key and entern";
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 << " # ####### # # # ### #####ttmade for fun n";
cout << "nnnn";
cout << "t<Menu>n";
cout << "t1: Start Gament2: Quitnn";
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 << "ntA: lefttS: downtD: 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();
}
}
c++ c++11 game tetris
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
add a comment |
up vote
8
down vote
favorite
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 << "nnPress any key and entern";
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 << " # ####### # # # ### #####ttmade for fun n";
cout << "nnnn";
cout << "t<Menu>n";
cout << "t1: Start Gament2: Quitnn";
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 << "ntA: lefttS: downtD: 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();
}
}
c++ c++11 game tetris
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
add a comment |
up vote
8
down vote
favorite
up vote
8
down vote
favorite
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 << "nnPress any key and entern";
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 << " # ####### # # # ### #####ttmade for fun n";
cout << "nnnn";
cout << "t<Menu>n";
cout << "t1: Start Gament2: Quitnn";
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 << "ntA: lefttS: downtD: 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();
}
}
c++ c++11 game tetris
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
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 << "nnPress any key and entern";
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 << " # ####### # # # ### #####ttmade for fun n";
cout << "nnnn";
cout << "t<Menu>n";
cout << "t1: Start Gament2: Quitnn";
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 << "ntA: lefttS: downtD: 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();
}
}
c++ c++11 game tetris
c++ c++11 game tetris
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
edited Dec 18 '16 at 18:34
Jamal♦
30.2k11115226
30.2k11115226
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
asked Dec 20 '14 at 15:46
MORTAL
1,67531529
1,67531529
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
up vote
10
down vote
accepted
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"
"nnPress any key and entern";
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.
answered Dec 20 '14 at 20:53
Edward
45.5k377207
1
Nice answer, +1. One minor detail: Anonymous namespaces are preferred overstaticfor file-scoped variables.
– glampert
Dec 21 '14 at 20:15
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
1
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
|
show 1 more comment
up vote
2
down vote
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.
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
2
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:std::cout << toBoardChar(field[i][j]) << std::flush;.
– Edward
Dec 21 '14 at 21:08
add a comment |
protected by Community♦ 2 days ago
Thank you for your interest in this question.
Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).
Would you like to answer one of these unanswered questions instead?
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
10
down vote
accepted
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"
"nnPress any key and entern";
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.
answered Dec 20 '14 at 20:53
Edward
45.5k377207
1
Nice answer, +1. One minor detail: Anonymous namespaces are preferred overstaticfor file-scoped variables.
– glampert
Dec 21 '14 at 20:15
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
1
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
|
show 1 more comment
up vote
10
down vote
accepted
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"
"nnPress any key and entern";
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.
answered Dec 20 '14 at 20:53
Edward
45.5k377207
1
Nice answer, +1. One minor detail: Anonymous namespaces are preferred overstaticfor file-scoped variables.
– glampert
Dec 21 '14 at 20:15
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
1
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
|
show 1 more comment
up vote
10
down vote
accepted
up vote
10
down vote
accepted
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"
"nnPress any key and entern";
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.
answered Dec 20 '14 at 20:53
Edward
45.5k377207
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"
"nnPress any key and entern";
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.
answered Dec 20 '14 at 20:53
Edward
45.5k377207
edited Dec 21 '14 at 21:02
answered Dec 20 '14 at 20:53
Edward
45.5k377207
answered Dec 20 '14 at 20:53
Edward
45.5k377207
answered Dec 20 '14 at 20:53
Edward
45.5k377207
45.5k377207
1
Nice answer, +1. One minor detail: Anonymous namespaces are preferred overstaticfor file-scoped variables.
– glampert
Dec 21 '14 at 20:15
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
1
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
|
show 1 more comment
1
Nice answer, +1. One minor detail: Anonymous namespaces are preferred overstaticfor file-scoped variables.
– glampert
Dec 21 '14 at 20:15
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
1
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
1
1
Nice answer, +1. One minor detail: Anonymous namespaces are preferred over
static for file-scoped variables.– glampert
Dec 21 '14 at 20:15
Nice answer, +1. One minor detail: Anonymous namespaces are preferred over
static for file-scoped variables.– glampert
Dec 21 '14 at 20:15
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert: Excellent point -- thanks! I've updated my answer accordingly.
– Edward
Dec 21 '14 at 21:02
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@glampert i just knew that static storage duration objects are zero initialized. is Anonymous namespace behave like static for initializing or not
– MORTAL
Dec 22 '14 at 23:14
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
@MORTAL, that's actually a very good question. I normally don't rely on the default zero initialization, and also prefer the explicit initialization for the clarity, so I wasn't sure if the namespace variables would be zero filled. According to this and this questions, they also qualify as static-storage, so are zero filled by default.
– glampert
Dec 22 '14 at 23:57
1
1
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
@MORTAL, yep, did the same test here with Clang and it zeroed the variables. Though like I said, it is best to explicitly initialize the data to leave no margin for doubt.
– glampert
Dec 23 '14 at 1:46
|
show 1 more comment
up vote
2
down vote
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.
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
2
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:std::cout << toBoardChar(field[i][j]) << std::flush;.
– Edward
Dec 21 '14 at 21:08
add a comment |
up vote
2
down vote
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.
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
2
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:std::cout << toBoardChar(field[i][j]) << std::flush;.
– Edward
Dec 21 '14 at 21:08
add a comment |
up vote
2
down vote
up vote
2
down vote
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.
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
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.
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
answered Dec 20 '14 at 20:59
Malachi♦
25.5k772175
25.5k772175
2
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:std::cout << toBoardChar(field[i][j]) << std::flush;.
– Edward
Dec 21 '14 at 21:08
add a comment |
2
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:std::cout << toBoardChar(field[i][j]) << std::flush;.
– Edward
Dec 21 '14 at 21:08
2
2
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:
std::cout << toBoardChar(field[i][j]) << std::flush;.– Edward
Dec 21 '14 at 21:08
I agree with that formatting change. Also, I think that a better way to address that particular code would be to introduce a function which returns the appropriate character and call it once:
std::cout << toBoardChar(field[i][j]) << std::flush;.– Edward
Dec 21 '14 at 21:08
add a comment |
protected by Community♦ 2 days ago
Thank you for your interest in this question.
Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).
Would you like to answer one of these unanswered questions instead?
