Multithreaded text-based Tetris game

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
#include
#include "conio.h"
#include

//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];
pthread_t pth0;
pthread_t pth1;
pthread_t pth2;
pthread_t pth3;
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

I see some things that may help you improve your code.

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 . 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 rotatio

StackExchange Code Review Q#133830, answer score: 5