8
\$\begingroup\$

What sticks out and what would you have done better in this termina-tetris implementation? I do not intend to use namespaces or split it up into multiple files.

#include <ncurses.h>
#include <array>
#include <cstdlib>

// The tetris board
const int board_size_x = 15;
const int board_size_y = 20;

// In-game stats
int block_x = 0; // Position of the moving block inside the board
int block_y = 0;
int tick_force_down; // Ticks down. Force down block when < 0. Resets to 'level' every time the block moves down.
int level;  // Max ticks before block is forced down. Decreases during the game as score increases.
int score;  // Current score (number of lines taken)


// Where to print stuff
const int board_x = 10; // Tetris board
const int board_y = 3;
const int score_y = 1; // Scoreboard
const int score_x = 5;
const int next_block_x = 30; // Next block
const int next_block_y = 5;

const int blocksize = 3;
using block = std::array<std::array<int, blocksize>, blocksize>;

// Tetris board. 0 = empty
std::array<std::array<int, board_size_x>, board_size_y> board {};
block current_block {};
block next_block{};


// Draw a colored square
void drawsquare(int y, int x, int color){
    move(y,x);
    attron(COLOR_PAIR(color));
    addch(' ');
    attroff(COLOR_PAIR(color));
}

// Draw block
void drawblock(int row, int col, block & b){
    for(int y=0; y < blocksize; ++y)
        for(int x=0; x < blocksize; ++x)
            if(b[y][x])
                drawsquare(row + y, col + x, b[y][x]);
}

// Draw moving block
void drawmoving(){
    drawblock(board_y+1+block_y, board_x+1+block_x, current_block);
}


// Draw next block
void drawnext(){
    mvprintw(next_block_y, next_block_x, "Next: ");
    drawblock(next_block_y+1, next_block_x+1, next_block);
}


// Randomize next block
void newnext(){
    int c = 1 + rand()%7;   // Color. 1-7 as initialized for ncurses.
    switch(rand()%7){
        case 0:
            next_block = {0,c,0,  0,c,0,  c,c,c};
            break;
        case 1:
            next_block = {c,c,c,  c,c,c,  c,c,c};
            break;
        case 2:
            next_block = {c,c,0,  0,c,c,  0,0,0};
            break;
        case 3:
            next_block = {0,c,0,  0,c,0,  0,c,c};
            break;
        case 4:
            next_block = {0,c,0,  0,c,0,  0,c,0};
            break;
        case 5:
            next_block = {0,c,c,  c,c,0,  0,0,0};
            break;
        case 6:
            next_block = {0,c,0,  0,c,0,  c,c,0};           
            break;
    }

}


// Crystalizes moving block into the tetris board
void raster(){
    for(int y=0; y < blocksize; ++y)
    for(int x=0; x < blocksize; ++x){
        if(!  current_block[y][x])
            continue;
        board[block_y+y][block_x+x] = current_block[y][x];
    }   
}

// block is inside another rasterized block or outside the board?
bool collide(int row, int col, const block & b){
    for(int y=0; y < blocksize; ++y)
    for(int x=0; x < blocksize; ++x){
        if(! b[y][x] )
            continue;

        int y_on_board = row + y;
        int x_on_board = col + x;

        if(x_on_board < 0 || x_on_board >= board_size_x || y_on_board >= board_size_y)
            return true;

        if(board[y_on_board][x_on_board])
            return true;
    }

    return false;
}

// Drops the next block, makes a new next. False on collide.
bool drop(){
    block_y = 1;
    block_x = board_size_x/2 - 1;
    current_block = next_block;
    newnext();

    return !collide(block_y, block_x, current_block);
}

// Rotated right if possible
void rotright(){
    block rot;
    for(int ny=0; ny < blocksize; ++ny)
        for(int nx=0; nx < blocksize; ++nx)
            rot[ny][nx] = current_block[blocksize-1-nx][ny];
    if(collide(block_y, block_x, rot))
        return;
    current_block = rot;
}

// Rotated left if possible
void rotleft(){
    block rot;
    for(int ny=0; ny < blocksize; ++ny)
        for(int nx=0; nx < blocksize; ++nx)
            rot[ny][nx] = current_block[nx][blocksize-1-ny];
    if(collide(block_y, block_x, rot))
        return;
    current_block = rot;
}


// false and refuse on collide
bool movedown(){
    if(collide(block_y+1, block_x, current_block))
        return false;
    ++block_y;
    return true;
}

void moveleft(){
    if(collide(block_y, block_x-1, current_block))
        return;
    --block_x;  
}

void moveright(){
    if(collide(block_y, block_x+1, current_block))
        return;
    ++block_x;
}



void textout(int y, int x, const char* str){
    mvprintw(y, x, str);
}

// Returns number of cleared lines
int clearlines(){
    int cleared = 0;
    for(int y=0; y < board_size_y; ++ y){
        int squares = 0;
        for(int x=0; x < board_size_x; ++ x){
            if(board[y][x])
                ++squares;
        }


        // Drop down all the above lines        
        if(squares == board_size_x){
            ++cleared;
            for(int xc=0; xc < board_size_x; ++xc) // Clear line. Important for row 0.
                board[y][xc]=0;

            for(int y2 = y; y2 > 0; --y2) // The line we're moving to
            for(int x2 = 0; x2 < board_size_x; ++x2)
                board[y2][x2] = board[y2-1][x2]; // Move above line to this line
        }


    }
    return cleared;
}

void drawboard(){
    // Draw a box around the tetris board
    mvaddch(board_y, board_x, ACS_ULCORNER);
    mvaddch(board_y, board_x + board_size_x + 1, ACS_URCORNER);
    mvaddch(board_y + board_size_y + 1, board_x, ACS_LLCORNER);
    mvaddch(board_y + board_size_y + 1, board_x + board_size_x + 1, ACS_LRCORNER);

    for(int i = 1; i <= board_size_x; ++i){
        mvaddch(board_y, board_x + i , ACS_HLINE);
        mvaddch(board_y + board_size_y + 1, board_x + i, ACS_HLINE);
    }

    for(int i = 1; i <= board_size_y; ++i){
        mvaddch(board_y + i, board_x, ACS_VLINE);
        mvaddch(board_y + i, board_x + board_size_x + 1, ACS_VLINE);
    }

    // Draw the filled board squares
    for(int y=0; y < board_size_y; ++y)
        for(int x=0; x < board_size_x; ++x)
            drawsquare(board_y + y + 1, board_x + x + 1, board[y][x]);
}

// Init a new game
void newgame(){
    newnext();
    drop();
    level = 300;
    tick_force_down = level;
    score = 0;
}

bool lost = false;
bool ingame_loop(){
    int c=getch();

    if(c == 'q' || c == 'Q')
        return false;

    if(lost){
        mvprintw(0,0,"You lost. Press q to quit.");
        refresh();
        return true;
    }

    bool down = false;

    if(--tick_force_down < 0){
        tick_force_down = level;
        down = true;
    }

    switch(c){
        case ' ':
            down = true;
            while(movedown())
                ;
            break;
        case 'z':
        case 'Z':
            rotleft();
            break;

        case 'x':
        case 'X':
        case KEY_UP:
            rotright();
            break;

        case KEY_LEFT:
            moveleft();
            break;

        case KEY_RIGHT:
            moveright();
            break;

        case KEY_DOWN:
            down = true;
            break;
    }

    if(down){
        tick_force_down = level;
        if(!movedown()){
            raster();
            if(!drop())
                lost = true;
            else{
                int lines = clearlines();
                level -= lines;
                score += lines;
            }
        }
    }

    // Update the screen
    clear();
    drawboard();
    drawmoving();
    drawnext();
    mvprintw(score_y, score_x, "Score: %d", score);
    refresh();

    return true;
}

int main()
{   
    // Init ncurses
    initscr();
    start_color();
    curs_set(0);
    cbreak();
    noecho();
    keypad(stdscr,TRUE);
    for(int i=1; i <= 7; ++i) // man init_pair
        init_pair(i, COLOR_BLACK, i);
    timeout(1);

    newgame();
    while(ingame_loop())
        ;

    endwin();
    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Addition to my question: the goal was also to be minimalistic. \$\endgroup\$ – tetriscoder7654876543 Sep 7 '17 at 2:29
  • \$\begingroup\$ Minimalistic in what way, and why? Limited time for implementation? Ease of comprehension for beginners? Suitability for low-power devices? \$\endgroup\$ – 200_success Sep 7 '17 at 18:53
  • \$\begingroup\$ Time and effort I guess, but also in codesize by not "overengineering" it. Specifically, I wanted to do what was needed to make a tetris game and no more. C++ was my choice of language. \$\endgroup\$ – tetriscoder7654876543 Sep 7 '17 at 18:57
3
\$\begingroup\$

I'll steer clear of the high-level design questions, and just critique the code itself within its own context.

Curly braces:

for(int y=0; y < blocksize; ++y)
  for(int x=0; x < blocksize; ++x)
    if(b[y][x])
      drawsquare(row + y, col + x, b[y][x]);

While omitting the curly braces is valid, you should try to avoid doing that if the full statement doesn't fit on one line. That's because the following compiles fine, but doesn't do what you want, and is an easy mistake to make:

for(int y=0; y < blocksize; ++y)
  for(int x=0; x < blocksize; ++x)
    if(b[y][x])
      drawsquare(row + y, col + x, b[y][x]);
      std::cout << "I drew a square\n"; 

Constness

void drawblock(int row, int col, block & b){

If a function accepts a non-const reference, it's generally seen as a contract that the function could possibly modify the object. That's not the case here, so the reference should be const.

Comments:

// Draw block
void drawblock(int row, int col, block & b){

These types of comments are just useless. Comments should provide additional information, not just redundantly state what the code obviously says already.

Variable types:

const int board_size_x = 15;

Size variables should use size_t.

Excessive copies

Your block type is big enough that I would personally create global instances and pass pointers instead of copying them around.

Limit scoping of lost

lost is only ever used inside of ingame_loop(), it has no business being a global variable. I would simply move the while() loop inside of ingame_loop().

Uninitialized rand()

You need to call srand(), otherwise your program won't actually be random. Better yet, you should use the stl random library instead.

Minor stuff

Spacing:

Be consistent:

block current_block {};
block next_block{};
                ^
for(int y=0; y < blocksize; ++y)
         ^

if(!  current_block[y][x])
    ^

Visual language:

You have a nice missed opportunity to make your block initializations visually readable:

next_block = {0,c,0,  0,c,0,  c,c,c};
//vs
next_block = {0, c, 0,  
              0, c, 0,  
              c, c, c};
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.