7
\$\begingroup\$

I'm making a simple Tetris clone and would like to know how's my code for the map and pieces. I'm using x, y, x_before, y_before, t0, t1 to allow smooth movement.

colors.h

#ifndef TETRIS_COLORS_H
#define TETRIS_COLORS_H

typedef enum {
    DARK_CYAN, DARK_RED, DARK_BROWN, DARK_MAGENTA, 
    DARK_GRAY, DARK_GREEN, DARK_BLUE, WALL, EMPTY
} Color;

#endif

pieces.h

#ifndef TETRIS_PIECES_H
#define TETRIS_PIECES_H

#include "colors.h"
#include "map.h"
#include "definitions.h"

#define PIECE_COUNT 7
#define PIECE_ROWS 4
#define PIECE_COLUMNS 4
#define PIECE_POINTS (PIECE_ROWS * PIECE_COLUMNS)
#define PIECE_BLOCKS_SIZE 4

typedef struct {
    int n;
    int matrix[4][16];
    Color color;
    int x;
    int y;
    int x_before;
    int y_before;
    unsigned int t0;
    unsigned int t1;
} Piece;

void piece_rotate(Piece *piece);
void piece_rotate_backwards(Piece *piece);
void piece_random(Piece *dest);
void piece_new(Piece *piece);
void piece_draw(Map *map, Piece *piece);
int piece_valid_position(Map *map, Piece *piece);

#endif

pieces.c

#include "pieces.h"
#include <stdlib.h>
#include <string.h>

Piece i = {0, 

    0, 0, 0, 0,
    1, 1, 1, 1,
    0, 0, 0, 0,
    0, 0, 0, 0,

    0, 0, 1, 0,
    0, 0, 1, 0,
    0, 0, 1, 0,
    0, 0, 1, 0,

    0, 0, 0, 0,
    0, 0, 0, 0,
    1, 1, 1, 1,
    0, 0, 0, 0,

    0, 1, 0, 0,
    0, 1, 0, 0,
    0, 1, 0, 0,
    0, 1, 0, 0

, DARK_CYAN, 0, 0, 0, 0, 0, 0};



Piece j = {0,

    1, 0, 0, 0,
    1, 1, 1, 0,
    0, 0, 0, 0,
    0, 0, 0, 0,

    0, 1, 1, 0,
    0, 1, 0, 0,
    0, 1, 0, 0,
    0, 0, 0, 0,

    0, 0, 0, 0,
    1, 1, 1, 0,
    0, 0, 1, 0,
    0, 0, 0, 0,

    0, 1, 0, 0,
    0, 1, 0, 0,
    1, 1, 0, 0,
    0, 0, 0, 0
, DARK_RED, 0, 0, 0, 0, 0, 0};


Piece l = {0,

    0, 0, 1, 0,
    1, 1, 1, 0,
    0, 0, 0, 0,
    0, 0, 0, 0,

    0, 1, 0, 0,
    0, 1, 0, 0,
    0, 1, 1, 0,
    0, 0, 0, 0,

    0, 0, 0, 0,
    1, 1, 1, 0,
    1, 0, 0, 0,
    0, 0, 0, 0,

    1, 1, 0, 0,
    0, 1, 0, 0,
    0, 1, 0, 0,
    0, 0, 0, 0

, DARK_BROWN, 0, 0, 0, 0, 0, 0};

Piece o = {0,

    0, 1, 1, 0,
    0, 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, 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

, DARK_MAGENTA, 0, 0, 0, 0, 0, 0};

Piece s = {0,

    0, 1, 1, 0,
    1, 1, 0, 0,
    0, 0, 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, 1, 0,
    1, 1, 0, 0,
    0, 0, 0, 0,

    1, 0, 0, 0,
    1, 1, 0, 0,
    0, 1, 0, 0,
    0, 0, 0, 0

, DARK_GRAY, 0, 0, 0, 0, 0, 0};

Piece t = {0,

    0, 1, 0, 0,
    1, 1, 1, 0,
    0, 0, 0, 0,
    0, 0, 0, 0,

    0, 1, 0, 0,
    0, 1, 1, 0,
    0, 1, 0, 0,
    0, 0, 0, 0,

    0, 0, 0, 0,
    1, 1, 1, 0,
    0, 1, 0, 0,
    0, 0, 0, 0,

    0, 1, 0, 0,
    1, 1, 0, 0,
    0, 1, 0, 0,
    0, 0, 0, 0
, DARK_GREEN, 0, 0, 0, 0, 0, 0};

Piece z = {0,

    1, 1, 0, 0,
    0, 1, 1, 0,
    0, 0, 0, 0,
    0, 0, 0, 0,

    0, 0, 1, 0,
    0, 1, 1, 0,
    0, 1, 0, 0,
    0, 0, 0, 0,

    0, 0, 0, 0,
    1, 1, 0, 0,
    0, 1, 1, 0,
    0, 0, 0, 0,

    0, 1, 0, 0,
    1, 1, 0, 0,
    1, 0, 0, 0,
    0, 0, 0, 0

, DARK_BLUE, 0, 0, 0, 0, 0, 0};

Piece *piece_list[PIECE_COUNT] = {&i, &j, &l, &o, &s, &t, &z};



void piece_rotate(Piece *piece)
{
    piece->n = ++piece->n % 4;
}

void piece_rotate_backwards(Piece *piece)
{
    piece->n = (--piece->n > -1) ? piece->n : 3;
}

void piece_random(Piece *dest)
{
    memcpy(dest, piece_list[rand() % PIECE_COUNT], sizeof(Piece)-sizeof(int) * 4);
}

void piece_new(Piece *piece)
{
    piece_random(piece);
    piece->x_before = piece->x = 1 + (GAME_COLUMNS - 2 - PIECE_BLOCKS_SIZE) / 2;
    piece->y_before = piece->y = 1;
    piece->t0 = piece->t1 = 0;
}

void piece_draw(Map *map, Piece *piece)
{
    for(int i = 0; i < PIECE_BLOCKS_SIZE * PIECE_BLOCKS_SIZE; ++i)
        if(piece->matrix[piece->n][i])
            map_draw_block( map, 
                            piece->y + i / PIECE_BLOCKS_SIZE, 
                            piece->x + i % PIECE_BLOCKS_SIZE, 
                            piece->color                    );
}

int piece_valid_position(Map *map, Piece *piece)
{
    for(int i = 0; i < PIECE_BLOCKS_SIZE * PIECE_BLOCKS_SIZE; ++i)
        if(piece->matrix[piece->n][i])
            if(map_block_at(    map, 
                                piece->y + i / PIECE_BLOCKS_SIZE, 
                                piece->x + i % PIECE_BLOCKS_SIZE    )
                            != EMPTY)
                return 0;

    return 1;
}

map.h

#ifndef TETRIS_MAP_H
#define TETRIS_MAP_H

#include "colors.h"

typedef struct Map {
    Color *blocks;
    int rows;
    int columns;
} Map;

Map *map_create(int rows, int columns);
void map_delete(Map *map);
void map_clear(Map *map);
void map_add_wall(Map *map);
Color map_block_at(Map *map, int row, int column);
void map_draw_block(Map *map, int row, int column, Color block);
void map_clear_row(Map *map, int row);
void move_rows_down(Map *map, int bottom_y);

#endif

map.c

#include "map.h"
#include <stdlib.h>

Map *map_create(int new_rows, int new_columns)
{
    Map *map = malloc(sizeof(Map) + sizeof(Color) * new_rows * new_columns);
    if(map == NULL)
        return NULL;

    map->blocks = (Color *)(map + 1);
    map->rows = new_rows;
    map->columns = new_columns;

    return map;
}

void map_delete(Map *map)
{
    free(map);
}

void map_clear(Map *map)
{
    for(int i = 0; i < map->rows * map->columns; ++i)
        map->blocks[i] = EMPTY;
}

void map_add_wall(Map *map)
{
    //Top
    for(int i = 0; i < map->columns; ++i)
        map->blocks[i] = WALL;

    //Bottom
    for(int i = 0; i < map->columns; ++i)
        map->blocks[(map->rows -1) * map->columns + i] = WALL;

    //Left and right
    for(int i = 1; i < map->rows; ++i){
        map->blocks[i * map->columns - 1] = WALL;
        map->blocks[i * map->columns] = WALL;
    }
}

Color map_block_at(Map *map, int row, int column)
{
    return map->blocks[row * map->columns + column];
}

void map_draw_block(Map *map, int row, int column, Color block)
{
    map->blocks[row * map->columns + column] = block;
}

void map_clear_row(Map *map, int row)
{
    for(int x = 1; x < map->columns - 1; ++x)
        map->blocks[row * map->columns + x] = EMPTY;
}

void move_rows_down(Map *map, int bottom_y)
{
    for(int y = bottom_y; y > 1; --y)
        for(int x = 1; x < map->columns - 1; ++x)
            map_draw_block(map, y, x, map_block_at(map, y - 1, x));

    map_clear_row(map, 1);
}
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Overall this looks very nice. It's easy to read, and is very refined. A few notes:

  • You can remove the Map from the first line of your structure.

    typedef struct
    {
        Color *blocks;
        int rows;
        int columns;
    } Map;
    
  • Your function map_delete() only performs one action, freeing the memory of map. Unless you plan to expand on the map structure where you will have to free more members within the structure itself, I would get rid of it and just call free(map). If you have compiler optimizations on, I guarantee that your compiler is already doing that for you anyways, so it should slightly decrease compilation time.

  • You can simpilfy your NULL tests.

    if(!map)
    
  • This is more of a style issue, but since this recently led to the Apple goto security flaw, I'm going to cover it. I don't think you are writing your brace-free single statement loops and test conditionals correctly. This is completely up to you to decide since this is a style issue, but I prefer to do it like this:

    if(!map) return NULL;
    

    Other's may be more strict and tell you to use braces, but it's up to you.

  • You need more comments. There are some lines in your code that I have trouble following. And if I have trouble following it, chances are that you will have trouble following it as well when you re-visit this project in a few months. Save yourself (and possibly others) the trouble and document your code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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