I have decided to make a game of text-based Tetris in C. At first I had no idea how to start so I settled for threads. Each thread moves its own piece down the grid and all the main thread does is redraw it every 16ms.

However, now I see that multi-threading a game like Tetris isn't necessary at all. Now I can't figure out an efficient way to rotate my pieces. I have only made 3 of them for now and thought I would make the rest when I figure out rotation. At the moment I have made rotation work for the line piece, but that required a huge chunk of code and is just hideous.

#include <stdio.h>
#include "conio.h"
#include <stdlib.h>

//Function prototypes
int rand_range(int min, int max);
void drawField(void);
void drawLblock();
void drawLineBlock(void);
void spawnShape(int x);
void checkWin(void);

int a = 5, i, k = 0, j = 0, turn = 3, lock = 0, rotate = 0;
int field[21][10];
int speed = 400000000;

void *lineBlock(void *arg)
{
while(1)
{
if(rotate % 2 == 0)
{
field[k][a] = 1;
field[1 + k][a] = 1;
field[2 + k][a] = 1;
field[3 + k][a] = 1;
k++;
nanosleep((const struct timespec[]){{0, speed}}, NULL);
if(field[3 + k][a] != 0)
{
field[k - 1][a] = 2;
field[k][a] = 2;
field[k + 1][a] = 2;
field[k + 2][a] = 2;
break;
}
if(turn == 1 && a != 0 && field[3 + k][a - 1] == 0 &&         field[2 + k][a - 1] == 0 && field[1 + k][a - 1] == 0 && field[0 + k][a - 1] == 0)
{
a--;
if(field[k - 1][a + 1] != 2)
field[k - 1][a + 1] = 0;
if(field[k][a + 1] != 2)
field[k][a + 1] = 0;
if(field[k + 1][a + 1] != 2)
field[k + 1][a + 1] = 0;
if(field[k + 2][a + 1] != 2)
field[k + 2][a + 1] = 0;
turn = 0;
}
else if(turn == 2  && a !=9 && field[3 + k][a + 1] == 0 && field[2 + k][a + 1] == 0 && field[1 + k][a + 1] == 0 && field[0 + k][a + 1] == 0)
{
a++;
if(field[0 + k - 1][a - 1] != 2)
field[0 + k - 1][a - 1] = 0;
if(field[1 + k - 1][a - 1] != 2)
field[1 + k - 1][a - 1] = 0;
if(field[2 + k - 1][a - 1] != 2)
field[2 + k - 1][a - 1] = 0;
if(field[3 + k - 1][a - 1] != 2)
field[3 + k - 1][a - 1] = 0;
turn = 0;
}
if(field[0 + k - 1][a] != 2)
field[0 + k - 1][a] = 0;
if(field[1 + k - 1][a] != 2)
field[1 + k - 1][a] = 0;
if(field[2 + k - 1][a] != 2)
field[2 + k - 1][a] = 0;
if(field[3 + k - 1][a] != 2)
field[3 + k - 1][a] = 0;
}
else
{
field[k][a - 1] = 1;
field[k][a] = 1;
field[k][a + 1] = 1;
field[k][a + 2] = 1;
k++;
nanosleep((const struct timespec[]){{0, speed}}, NULL);
if(field[k][a] != 0 || field[k][a - 1] != 0 || field[k][a + 1] != 0 || field[k][a + 2] != 0)
{
field[k - 1][a - 1] = 2;
field[k - 1][a] = 2;
field[k - 1][a + 1] = 2;
field[k - 1][a + 2] = 2;
break;
}
if(turn == 1 && a - 1 != 0 && field[3 + k][a - 1] == 0 && field[2 + k][a - 1] == 0 && field[1 + k][a - 1] == 0 && field[0 + k][a - 1] == 0)
{
a--;
if(field[0 + k - 1][a + 1] != 2)
field[0 + k - 1][a + 1] = 0;
if(field[0 + k - 1][a + 2] != 2)
field[0 + k - 1][a + 2] = 0;
if(field[0 + k - 1][a  + 3] != 2)
field[0 + k - 1][a + 3] = 0;
if(field[0 + k - 1][a  + 4] != 2)
field[0 + k - 1][a + 4] = 0;
turn = 0;
}
else if(turn == 2  && a + 2 !=9 && field[3 + k][a + 1] == 0 && field[2 + k][a + 1] == 0 && field[1 + k][a + 1] == 0 && field[0 + k][a + 1] == 0)
{
a++;
if(field[0 + k - 1][a - 1] != 2)
field[0 + k - 1][a - 1] = 0;
if(field[0 + k - 1][a - 2] != 2)
field[0 + k - 1][a - 2] = 0;
if(field[0 + k - 1][a - 3] != 2)
field[0 + k - 1][a - 3] = 0;
if(field[0 + k - 1][a - 4] != 2)
field[0 + k - 1][a - 4] = 0;
turn = 0;
}
if(field[0 + k - 1][a - 1] != 2)
field[0 + k - 1][a - 1] = 0;
if(field[0 + k - 1][a] != 2)
field[0 + k - 1][a] = 0;
if(field[0 + k - 1][a + 1] != 2)
field[0 + k - 1][a + 1] = 0;
if(field[0 + k - 1][a + 2] != 2)
field[0 + k - 1][a + 2] = 0;

}
}
k = 0;
a = 5;
speed = 400000000;
checkWin();
rotate = 0;
spawnShape(rand_range(1, 3));
}

void *squareBlock(void *arg)
{
while(1)
{
field[0 + k][a] = 1;
field[1 + k][a] = 1;
field[0 + k][a + 1] = 1;
field[1 + k][a + 1] = 1;
k++;
nanosleep((const struct timespec[]){{0, speed}}, NULL);
if(field[1 + k][a] != 0 || field[1 + k][a + 1] != 0)
{
field[0 + k - 1][a] = 2;
field[1 + k - 1][a] = 2;
field[0 + k - 1][a + 1] = 2;
field[1 + k - 1][a + 1] = 2;
break;
}
if(turn == 1 && a != 0 && field[1 + k][a - 1] == 0 && field[0 + k][a - 1] == 0)
{
a--;
if(field[0 + k - 1][a + 1] != 2)
field[0 + k - 1][a + 1] = 0;
if(field[1 + k - 1][a + 1] != 2)
field[1 + k - 1][a + 1] = 0;
if(field[0 + k - 1][a + 2] != 2)
field[0 + k - 1][a + 2] = 0;
if(field[1 + k - 1][a + 2] != 2)
field[1 + k - 1][a + 2] = 0;
turn = 0;
}
else if(turn == 2 && a + 1 != 9 && field[1 + k][a + 2] == 0 && field[0 + k][a + 2] == 0)
{
a++;
if(field[0 + k - 1][a - 1] != 2)
field[0 + k - 1][a - 1] = 0;
if(field[1 + k - 1][a - 1] != 2)
field[1 + k - 1][a - 1] = 0;
if(field[0 + k - 1][a - 2] != 2)
field[0 + k - 1][a - 2] = 0;
if(field[1 + k - 1][a - 2] != 2)
field[1 + k - 1][a - 2] = 0;
turn = 0;
}
if(field[0 + k - 1][a] != 2)
field[0 + k - 1][a] = 0;
if(field[1 + k - 1][a ] != 2)
field[1 + k - 1][a] = 0;
if(field[0 + k - 1][a + 1] != 2)
field[0 + k - 1][a + 1] = 0;
if(field[1 + k - 1][a + 1] != 2)
field[1 + k - 1][a + 1] = 0;
}
k = 0;
a = 5;
speed = 400000000;
checkWin();
spawnShape(rand_range(1, 3));
}
void *LBlock(void *arg)
{
while(1)
{
field[0 + k][a] = 1;
field[1 + k][a] = 1;
field[2 + k][a] = 1;
field[2 + k][a + 1] = 1;
k++;
nanosleep((const struct timespec[]){{0, speed}}, NULL);
if(field[2 + k][a] != 0 || field[2 + k][a + 1] != 0)
{
field[0 + k - 1][a] = 2;
field[1 + k - 1][a] = 2;
field[2 + k - 1][a] = 2;
field[2 + k - 1][a + 1] = 2;
break;
}
if(turn == 1 && a != 0)
{
if(field[2 + k][a - 1] == 0 && field[1 + k][a - 1] == 0 && field[0 + k][a - 1] == 0)
{
a--;
if(field[0 + k - 1][a + 1] != 2)
field[0 + k - 1][a + 1] = 0;
if(field[1 + k - 1][a + 1] != 2)
field[1 + k - 1][a + 1] = 0;
if(field[2 + k - 1][a + 1] != 2)
field[2 + k - 1][a + 1] = 0;
if(field[2 + k - 1][a + 2] != 2)
field[2 + k - 1][a + 2] = 0;
turn = 0;
}
}
else if(turn == 2 && a + 1 != 9)
{
if(field[0 + k][a + 1] == 0 /*&& field[1 + k][a + 1] == 0*/ && field[2 + k][a + 2] == 0)
{
a++;
if(field[0 + k - 1][a - 1] != 2)
field[0 + k - 1][a - 1] = 0;
if(field[1 + k - 1][a - 1] != 2)
field[1 + k - 1][a - 1] = 0;
if(field[2 + k - 1][a - 1] != 2)
field[2 + k - 1][a - 1] = 0;
if(field[2 + k - 1][a - 2] != 2)
field[2 + k - 1][a - 2] = 0;
turn = 0;
}
}
if(field[0 + k - 1][a] != 2)
field[0 + k - 1][a] = 0;
if(field[1 + k - 1][a] != 2)
field[1 + k - 1][a] = 0;
if(field[2 + k - 1][a] != 2)
field[2 + k - 1][a] = 0;
if(field[2 + k - 1][a + 1] != 2)
field[2 + k - 1][a + 1] = 0;
}
k = 0;
a = 5;
speed = 400000000;
checkWin();
spawnShape(rand_range(1, 3));
}

{
char input;
while(1)
{
fflush(stdin);
input = getch();
fflush(stdin);
if(input == 'a')
turn = 1;
else if(input == 'd')
turn = 2;
else if(input == 's')
speed = 150000000;
else if(input == 'w')
rotate++;
}
}

int main(void)
{
int gameOver = 0; //Variable to keep track of game state

srand(time(NULL)); //seed randomizer with time

for(i = 0; i < 10; i++) //Set last (invisible) row of matrix to 1's.
field[19][i] = 1;

spawnShape(rand_range(1,3)); // Spawn first tetromino

while(gameOver == 0) // Main loop that draws the grid at around 60 fps
{
system("clear");
drawField();
nanosleep((const struct timespec[]){{0, 160000000L}}, NULL);
}
return 0;
}

void drawField(void)
{
int i, a;
printf("________________\n");
for(i = 0; i < 19; i++)
{
printf("|*|");
for(a = 0; a < 10; a++)
{
if(field[i][a] == 1 || field[i][a] == 2)
printf("O");
else
printf(" ");
}
printf("|*|\n");
}
printf("----------------\n");
}

int rand_range(int min, int max)//Returns a random integer in the specified range
{
return rand() % (max - min + 1) + min;
}

void spawnShape(int x) //Function that starts threads to draw tetrominos
{
if(x == 1)
else if(x == 2)
else if(x == 3)
}

void checkWin(void)
{
int fieldtemp[20][10];
int i, a, count = 0, count2 = 0;

for(i = 0; i < 19; i++)
{
count = 0;
for(a = 0; a < 10; a++)
if(field[i][a] == 2)
count++;
if(count == 10)
{
count2++;
for(a = 0; a < 10; a++)
field[i][a] = 0;
}
}
if(count2 > 0)
for(i = 0; i < 20; i++)
{
for(a = 0 ;a < 10; a++)
{
fieldtemp[i][a] = field[i][a];
field[i][a] = 0;
field[i][a] = fieldtemp[i][a];
}
}
}


Any advice on what to do with this hunk of ugly code or how to make this work using a single thread is greatly appreciated. The conio.h header is some code I found that provides getch() for Linux. If you want to run this code you could substitute with getchar().

Wow... As you want to get rid of multithreading (I can certainly agree with that, it's huge overkill for tetris game), I would suggest to rewrite it completely.

I'm not going to write it here at the moment, just some ideas:

Have a solid world state, the field will do as is. Have falling shape shape as some enum or int. Next_shape can be the same. Score, level and duration/speed of game. And finally position x/y of falling shape and it's rotation.

If I didn't overlook anything, that should be complete info needed to render the world to the screen (from scratch, any time you wish).

To have a "nice" Tetris you should implement also some "effects" for lines removing, etc, which are just another data on top of the basic world, modifying final screen output.

Now do a main loop of the game, ticking at vertical refresh rate of display speed:

1. wait for vertical retrace of screen (to have smooth animation) (in ideal world here you can predict how long it will take, and do sleep for shorter time to easy CPU burden).
2. draw the current state of world (be quick enough to avoid tearing, ie finish within retrace period, or update screen from top to bottom ahead of display ray)
3. if you have any "effects", here is probably a good place to progress their states by single frame (to be ready for next world draw), or you may want to do that ahead of world draw, if you know it's fast enough.
4. read user input (handle exit, handle key repeating as you wish), produce clean "rotate/counter-rotate, move left/right, fast down" values for next step (so you can add later here also reading of gamepads/joystick, resulting into the same "user input" values for rest of code).
5. try to move the moving shape by user input: rotation/left or right/fast down. I would probably honour that very order of input handling. Action resulting in invalid result (moving shape on already occupied field) is ignored. Valid action will adjust current shape/rotation/position.
6. additional hint: Fast drop action is a cycle trying to move the piece down by one line, till invalid position is hit. Resetting drop timer to something what feels good. (zero would remove ability to rotate/move piece after fast drop, so I would recommend some short while).
7. decrease time till next line-drop of shape, if not zero yet, go to first step.
8. When drop timer reaches zero, try to move the piece by one line down. If the new position would be valid, move it, reset drop timer and go to first step.
9. Make moving piece solid at the (valid) place it was: fill up the field, then scan field for full lines and cut them out. (add score, "effects", reset drop timer, set new moving shape (shape, rotation, position, next_shape). Go to first step.

About pieces handling. I would use 4x4 pre-set fields, so for example shape "4" would be:

0 1 0 0
1 1 1 0
0 0 0 0
0 0 0 0


shape "9"

0 1 0 0
1 1 0 0
0 1 0 0
0 0 0 0


and I would have transition table telling me that counter-rotation of shape "4" will produce shape "9". It's hard-coded (as the rotation can be easily programmed in code with only basic shapes defined), but it's probably easy to grasp for beginner, and not that hard to read in source (just bulky long).

Then a valid position of moving shape is, when every 0 in shape definition is ignored, and every 1 has a valid field position (so moving left from position 0 will make some 1 to be at -1 coordinate -> invalid), and that valid field position is unoccupied. You can write general purpose validity check routine taking as input the shape definition (already rotated), position and it will just do:

for (int y: {0, 1, 2, 3}) for (int x: {0, 1, 2, 3}) {
if (0 == shape[y,x]) continue;
int fieldX = x + posX, fieldY = y + posY;
if (fieldX < FIELD_POS_X_MIN || FIELD_POS_X_MAX < fieldX) return invalid;
if (fieldY < FIELD_POS_Y_MIN || FIELD_POS_Y_MAX < fieldY) return invalid;
if (0 != field[fieldY, fieldX]) return invalid;
}
return valid;


Making the moving shape "solid" in the field will look very similar (assuming the current shape position+rotation is valid!):

for (int y: {0, 1, 2, 3}) for (int x: {0, 1, 2, 3}) {
if (0 == shape[y,x]) continue;
int fieldX = x + posX, fieldY = y + posY;
field[fieldY, fieldX] = shape[y,x]; // you can post-process the cube value here
// Or add some independent "effect" data, so they will not mix with raw game state, but add the effect visuals on top of it.
}


This is classic 8bit era main loop of game, syncing to the display ray, to make the graphics movements as smooth as possible. In modern era this has one huge catch. The display rates are no more stable across the platform. So a game with fixed waiting timers will then run twice as fast on 120Hz display, as on 60Hz one. For the beginning I would write it for your display refresh rate first.

Then I would add extra logic to timer values, adjusting for different display rates. But that extra logic may be not trivial, if you want to have the timing perfect (so for example level 7 on 60Hz would take 6 frames delay between single line drop, while on 85Hz it will take 8.5 of frames to have the same delay, so you would need to alternate between 8 and 9 frames delay).

But I would still go for the single-thread ticking by display refresh rate (for such simple game as tetris).

I would enter multi-thread waters only for games where single frame requires too much of computing (complex physics, lot of 3D world updating, etc..) and where parts of the processing can be done in parallel, but having the main loop with parallel threads usually make the code at least by one order of magnitude more complex (you have to think about all possible race conditions and who can modify which data at what time, locking/copying them accordingly).

Unless you want to exercise your multithread programming skills, I don't see any reason to use it for Tetris. Even as exercise I would wrote first the single loop version, to have something to relate to and compare with.

• Thank you. You have really given me an idea on how to do things. I will start writing from scratch.
– Max
Commented Jul 4, 2016 at 20:23

## Use consistent formatting

The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently.

## Use the required #includes

The code uses nanosleep which means that it should #include <time.h>. It was not difficult to infer, but it helps reviewers if the code is complete.

## Eliminate unused variables

Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code, global variables j and lock are set but never actually used. My compiler also tells me that. Your compiler is probably also smart enough to tell you that, if you ask it to do so.

## Where practical, eliminate unused parameters

I understand the desire for a consistent interface, which is good, but when the warning are cranked up on the compiler (as they should normally be), the arg parameter is not used in lineBlock(), squareBlock(), etc. Depending on the particulars of the compiler, one way to address the warning and also to make it clear to readers of the code that you are intentionally not using that parameter, might be to simply omit the name in the definition and implementation. The other obvious alternative is to omit that parameter.

## Make sure you return what you claim

The lineBlock(), squareBlock(), etc. function claim to return void * but don't actually return anything. The way to signify that is to write the function like this:

void squareBlock()


Alternatively, one could return NULL;.

## Don't use system("clear")

There are two reasons not to use system("clear") 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 clear and in the path, your program will execute that program instead of what you intend, and that other program could be anything. First, isolate this into a seperate functions cls() and then modify your code to call that function instead of system. For example:

void cls()
{
printf("\x1b[2J");
}


## Use a switch where appropriate

Within the inputThread() routine, we have a long compound if like this:

if (input == 'a')
turn = 1;
else if (input == 'd')
turn = 2;
// etc.


This would probably be better expressed as a switch statement instead:

switch(input) {
case 'a':
turn = 1;
break;
case 'd':
turn = 2;
break;
// etc.


## Provide a way to quit the program

It would be convenient to be able to type q to quit the program. We can easily do this by passing a pointer to gameOver from within main like this:

pthread_create(&pth0, NULL, inputThread, &gameOver);    //Start input thread


Now within inputThread(), we can do this:

int *gameOver = arg;


Finally simply add this to the switch statement mentioned above:

case 'q':
*gameOver = 1;
break;


In this case, we pass in a pointer to a single variable, but what is often done is to create a struct to contain all relevant data and then pass in a pointer to that. This is described in the next suggestion.

## Avoid the use of global variables

I see that turn and rotate, etc. are declared as global variables rather than as local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. This can be done with threads by wrapping the relevant variables up into a single struct like so:

typedef struct {
int a;
int k;
int turn;
int rotate;
int speed;
int gameOver;
int field[21][10];
} GameState;


Now instead of a global variable, we can use a GameState variable that is local within main. The rewritten inputThread now looks like this:

void *inputThread(void *arg)
{
GameState *gs = arg;
char input;
while (1) {
fflush(stdin);
input = getch();
fflush(stdin);
switch(input) {
case 'a':
gs->turn = 1;
break;
case 'd':
gs->turn = 2;
break;
case 's':
gs->speed = 150000000;
break;
case 'w':
gs->rotate++;
break;
case 'q':
gs->gameOver = 1;
break;
}
}
return NULL;
}


## Don't Repeat Yourself (DRY)

The rotation operations all include large portions of repeated code. Instead of repeating code, it's generally better to make common code into a function. Consider instead that every block in every orientation fits within a 4x4 square. Write a function to rotate a 4x4 square (whatever it might contain) and now you have a single function to do the rotation.

## Eliminate the bug

When you complete a horizontal row in the traditional Tetris game, that row disappears and anything above slides down one row. This code only makes the row disappear and doesn't slide everything down after.

## Think of the user

The game as implemented in current code doesn't end. It would be nice to both fix up the checkWin code so it actually causes the game to end if the field fills up vertically and, as mentioned before, provide a way for the user to gracefully quit the game on request.

I think this code could use threads more gracefully by having only two threads: one to handle input (as you've done) and a single one to handle the falling block and the update of the screen. That second thread might be either a spawned thread or the main routine (which is also a thread.)

## Eliminate "magic numbers"

There are a few numbers in the code, such as 2 and 19 that have a specific meaning in their particular context. By using named constants instead (either with #define or const as needed), the program becomes easier to read and maintain.

## Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

• Thank you. I will apply your advice to my code. Honestly I do things like cleaning up unused variables and tiding up the code after finishing the entire program.
– Max
Commented Jul 4, 2016 at 20:26
• @МаксимАтанасов: don't do "refactoring" phases after entire program is finished. Do them whenever you reach somewhat "stable" phase, when you can easily test current version of program, and tell whether it works as expected. First commit to git. At that point look especially for DRY (your original thread-per-piece smells of copy/paste duplicity immediately, after a bit of sniffing you should realise that code can be written in general way - configured by data of shape). Also formatting/unused variables are usually just 1-3min clean-up task with modern IDE, and all compiler warnings ON. Commented Jul 7, 2016 at 9:35