8
\$\begingroup\$

On an Internet forum a person came with their homework, to write Arkanoid in a console - it seemed they wanted someone to do their homework for them so I did not respond. However, I found the task interesting, especially when I broadened the scope of the assignment - so I wrote such arkanoid for my own entertainment. However, I don't really know C coding patterns so I'd be interested to know what more experienced devs think about my code.

#include <curses.h>
#include <stdlib.h>
#include <stdbool.h>
#include <signal.h>
#include <math.h>

const short brick_color = 1;
const short other_color = 2;

#define board_y_size 21
#define board_x_size 78

typedef enum { EMPTY, BRICK, BALL } content;
typedef struct
{
  unsigned lv;
  unsigned balls_left;
  bool running;
  content board[board_y_size][board_x_size];
  int ball_y, ball_x;
  int racket_pos;
  int delta_y, delta_x;
  unsigned score;
} game_data;

void init_new_level(game_data* gd)
{
  gd->lv++;
  if(gd->lv % 3 == 0)
    gd->balls_left++;
  gd->running = false;
  gd->racket_pos = 39;

  for(int y = 0; y < board_y_size; y++)
    for(int x = 0; x < board_x_size; x++)
      gd->board[y][x] = EMPTY;
  for(int y = 2; y < board_y_size - 4; y++)
    for(int x = 2; x < board_x_size - 2; x++)
      gd->board[y][x] = BRICK;
  gd->ball_y = board_y_size-1;
  gd->ball_x = gd->racket_pos;
  gd->board[gd->ball_y][gd->ball_x] = BALL;
}

void advance_ball(game_data* gd, int new_y, int new_x)
{
  gd->board[gd->ball_y][gd->ball_x] = EMPTY;
  gd->board[new_y][new_x] = BALL;
  gd->ball_y = new_y;
  gd->ball_x = new_x;
}

void recover_from_miss(game_data* gd)
{
  gd->racket_pos = 39;

ball_found:
  advance_ball(gd, board_y_size-1, gd->racket_pos);
}

int start_level(game_data* gd, int input)
{
  gd->delta_y = -1;
  if(input == KEY_LEFT)
    gd->delta_x = -1;
  else
    gd->delta_x = 1;

  gd->running = true;

  return (int)round(1000.0/pow(1.25, gd->lv-1));
}

void y_bounce(game_data* gd)
{
  gd->delta_y *= -1;
}

void x_bounce(game_data* gd)
{
  gd->delta_x *= -1;
}

void flip(game_data* gd)
{
  y_bounce(gd);
  x_bounce(gd);
}

void crush_bricks(game_data* gd, int y, int x)
{
  if(y >= 0 && y < board_y_size && x >= 0 && x < board_x_size &&
     gd->board[y][x] == BRICK)
  {
    gd->board[y][x] = EMPTY;
    gd->score++;
  }
}

bool is_out_of_board(int y)
{
  return y >= board_y_size;
}

bool is_blocked(game_data* gd, int y, int x)
{
  if(y < 0 || x < 0 || x >= board_x_size)
    return true;

  return gd->board[y][x] == BRICK;
}

bool normal_move(game_data* gd)
{
  int old_y = gd->ball_y, old_x = gd->ball_x;
  int new_y = old_y + gd->delta_y;
  int new_x = old_x + gd->delta_x;
  if(is_out_of_board(new_y))
  {
    gd->running = false;
    gd->balls_left--;
    return false;
  }

  else
  {
    int actual_new_x = new_x, actual_new_y = new_y;

    if(is_blocked(gd, old_y, new_x) && is_blocked(gd, new_y, old_x) ||
       is_blocked(gd, new_y, new_x) &&
         !is_blocked(gd, old_y, new_x) && !is_blocked(gd, new_y, old_x))
    {
      flip(gd);
      actual_new_x = old_x;
      actual_new_y = old_y;
    }

    else if(is_blocked(gd, old_y, new_x))
    {
      actual_new_x = old_x;
      x_bounce(gd);
    }
    else if(is_blocked(gd, new_y, old_x))
    {
      actual_new_y = old_y;
      y_bounce(gd);
    }

    advance_ball(gd, actual_new_y, actual_new_x);

    crush_bricks(gd, old_y, new_x);
    crush_bricks(gd, new_y, old_x);
    crush_bricks(gd, new_y, new_x);
  }

  return true;
}

bool collide_with_racket(game_data* gd)
{
  int old_y = gd->ball_y, old_x = gd->ball_x;
  int new_y = old_y + gd->delta_y;
  int new_x = old_x + gd->delta_x;

  if(old_y != board_y_size-1 || new_y != board_y_size)
    return false;

  if(new_x == gd->racket_pos-1 || new_x == gd->racket_pos-2 && gd->delta_x < 0)
  {
    y_bounce(gd);
    new_x = gd->racket_pos+1;
    gd->delta_x = 1;
    new_y = old_y-1;
    advance_ball(gd, new_y, new_x);
    return true;
  }
  else if(new_x == gd->racket_pos+1 || new_x == gd->racket_pos + 2 && gd->delta_x > 0)
  {
    y_bounce(gd);
    new_x = gd->racket_pos-1;
    gd->delta_x = -1;
    new_y = old_y-1;
    advance_ball(gd, new_y, new_x);
    return true;
  }
  else if(new_x == gd->racket_pos)
  {
    y_bounce(gd);
    new_x += gd->delta_x;
    new_y = old_y-1;
    advance_ball(gd, new_y, new_x);
    return true;
  }

  return false;
}

bool no_bricks_left(game_data* gd)
{
  for(int y = 0; y < board_y_size; y++)
    for(int x = 0; x < board_x_size; x++)
      if(gd->board[y][x] == BRICK)
        return false;
  gd->running = false;
  return true;
}

bool process_turn(game_data* gd)
{
  if(!collide_with_racket(gd))
  {
    if(!normal_move(gd))
      return false;
    return no_bricks_left(gd);
  }
  return false;
}

void move_racket(game_data* gd, int racket_move)
{
  if(gd->racket_pos + racket_move >= 1 &&
     gd->racket_pos + racket_move < board_x_size-1)
  {
    gd->racket_pos+=racket_move;
  }
}

void print_game_data(game_data const* gd)
{
  for(int x = 0; x < 80; x++) 
  {
    mvaddch(0, x, ' ');
    mvaddch(23, x, ' ');
  }

  mvprintw(0, 3, "Lv: %u", gd->lv);
  mvprintw(0, 65, "Balls: %u", gd->balls_left);
  mvprintw(0, 52, "Score: %u", gd->score);
  if(!gd->running)
  {
    attron(A_BLINK);
    if(gd->balls_left)
      mvprintw(0, 16, "PRESS LEFT OR RIGHT TO START");
    else
      mvprintw(0, 25, "GAME OVER");
    attroff(A_BLINK);
  }                 

  mvaddch(1, 0, ACS_ULCORNER);
  mvaddch(1, 79, ACS_URCORNER);
  for(int y = 2; y < 24; y++)
  {
    mvaddch(y, 0, ACS_VLINE);
    mvaddch(y, 79, ACS_VLINE);
  }
  for(int x = 1; x < 79; x++)
    mvaddch(1, x, ACS_HLINE);

  const int y_offset = 2;
  const int x_offset = 1;

  for(int y = 0; y < board_y_size; y++)
    for(int x = 0; x < board_x_size; x++)
    {
      chtype char_to_print;
      switch(gd->board[y][x])
      {
      case EMPTY:
        char_to_print = ' ';
        break;
      case BRICK:
        char_to_print = ACS_CKBOARD | COLOR_PAIR(brick_color);
        break;
      case BALL:
        char_to_print = 'O';
        break;
      }
      mvaddch(y+y_offset, x+x_offset, char_to_print);
    }

  const int racket_print_pos = gd->racket_pos+x_offset;
  mvaddch(23, racket_print_pos-1, ACS_HLINE);
  mvaddch(23, racket_print_pos, ACS_TTEE);
  mvaddch(23, racket_print_pos+1, ACS_HLINE);

  refresh();
}

void game_loop(game_data* gd)
{
  int sleep_time;
  bool advance_lv = true;

  while(true)
  { 
    if(!gd->running)
    {
      nodelay(stdscr, FALSE);
      if(gd->balls_left)
        if(advance_lv)
          init_new_level(gd);
        else
          recover_from_miss(gd);
      print_game_data(gd);
      int input;
      do input = getch();
      while(!(gd->balls_left && (input == KEY_LEFT || input == KEY_RIGHT)));
      sleep_time = start_level(gd, input);
      nodelay(stdscr, TRUE);
      process_turn(gd);
    }
    else
    {
      print_game_data(gd);
      napms(sleep_time);
      int racket_move = 0;
      int input;
      while((input = getch()) != ERR)
        if(input == KEY_LEFT)
          racket_move = -1;
        else if(input == KEY_RIGHT)
          racket_move = 1;
        else if(input == KEY_DOWN)
          racket_move = 0;
      move_racket(gd, racket_move);
      advance_lv = process_turn(gd);
    }
  }
}

void cleanup(int signal)
{
  endwin();
  exit(0);
}

int main()
{
  struct sigaction cleanup_action = { .sa_handler = cleanup, .sa_flags = 0 };
  sigfillset(&cleanup_action.sa_mask);
  sigaction(SIGINT, &cleanup_action, NULL);
  initscr();
  cbreak();
  keypad(stdscr, TRUE);
  noecho();
  nonl();
  curs_set(0);
  start_color();
  nodelay(stdscr, TRUE);

  init_pair(brick_color, COLOR_RED, COLOR_WHITE);
  init_pair(other_color, COLOR_BLACK, COLOR_WHITE);

  attrset(COLOR_PAIR(other_color));

  game_data gd = {.lv = 0, .balls_left = 3, .score = 0};
  game_loop(&gd);

  return 0;
}

I did spot two flaws in my code (DRY violations) and hopefully corrected them before posting it here - so I did take time to make it (reasonably) clean. Still, I'm sure the code is not ideal - what are the flaws I did not spot? (the game seems to be working)

There is still one flaw I'm aware of but that I consciously chose not to fix - namely, I'm not checking if the functions I call do not return errors. However, I think that error-checking in C is so tedious and the probability that these calls return errors is so low on any 'reasonable' platform that I thought it was simply not practical to do error-checking: the benefits would not justify cluttering the code.

\$\endgroup\$
7
  • \$\begingroup\$ Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Could you add some context or description about your code? \$\endgroup\$ – dfhwze Jun 13 '19 at 14:27
  • 2
    \$\begingroup\$ @dfhwze This is the context: On an Internet forum a person came with their homework, to write Arkanoid in a console - it seemed they wanted someone to do their homework for them so I did not respond. However, I found the task interesting, especially when I broadened the scope of the assignment - so I wrote such arkanoid for my own entertainment. However, I don't really know C coding patterns so I'd be interested to know what more exprienced devs think about my code. This is kind of the whole context \$\endgroup\$ – gaazkam Jun 13 '19 at 14:31
  • \$\begingroup\$ @dfhwze edited Q.... \$\endgroup\$ – gaazkam Jun 13 '19 at 14:33
  • \$\begingroup\$ You might want to fix your SIGINT handler to not call exit but instead raise SIGINT anew. This improves the behaviour when your program is called from a script or similar and is just good programming style. \$\endgroup\$ – FUZxxl Aug 27 '20 at 16:56
  • \$\begingroup\$ For those of us who don't know the game, can you summarise it for us? I see enum { EMPTY, BRICK, BALL } and wonder if it's a Breakout-style game? \$\endgroup\$ – Toby Speight Feb 6 at 16:16
1
\$\begingroup\$

Enable more compiler warnings. With gcc -std=c17 -Wall -Wextra -Wwrite-strings -Wno-parentheses -Wpedantic -Warray-bounds -Wstrict-prototypes -Wconversion I see, amongst others:

222224.c: In function ‘recover_from_miss’:
222224.c:57:1: warning: label ‘ball_found’ defined but not used [-Wunused-label]
 ball_found:
 ^~~~~~~~~~
222224.c: In function ‘cleanup’:
222224.c:331:18: warning: unused parameter ‘signal’ [-Wunused-parameter]
 void cleanup(int signal)
              ~~~~^~~~~~
222224.c: At top level:
222224.c:337:5: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
 int main()
     ^~~~
222224.c: In function ‘main’:
222224.c:339:10: error: variable ‘cleanup_action’ has initializer but incomplete type
   struct sigaction cleanup_action = { .sa_handler = cleanup, .sa_flags = 0 };
          ^~~~~~~~~

Fixing these would immediately improve the program. (That last one requires us to set _POSIX_C_SOURCE to an appropriate value before including <signal.h>; that's easiest done as a command-line argument such as -D_POSIX_C_SOURCE or -D_GNU_SOURCE).

Please use ALL_CAPS for macros. Macros need to be immediately recognisable so that we can treat them with extra caution, as they don't act like variables.

There's a lot of naked constants sprinkled over the code. For example,

gd->racket_pos = 39;

Is it coincidental that this is board_x_size / 2? Or is it intrinsically related?

And:

  if(gd->lv % 3 == 0)
    gd->balls_left++;

That's baking the game's parameters into its code; a more flexible game would allow such things to be set up as gameplay configuration.

We could do with some targeted comments to help readers (including future-you) to understand what values mean. For example, we have:

int start_level(game_data* gd, int input)
{
    /* ... */
    return (int)round(1000.0/pow(1.25, gd->lv-1));
}

It's not obvious why start_level() should even return a value, and completely opaque as to what the value might mean. I had to find the calling code to discover that it's a time delay, and then look up the documentation of napms() to discover the units. I'd argue that would be better obtained by a separate function, e.g.

int millis_per_frame(const game_data *game);

That immediately documents what it does, rather than hiding in a return value from an unrelated function.

\$\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.