4
\$\begingroup\$

Please help me improve this code. Move snake using : a w s d.

(My compiler doesn't support initialization of index variables inside loops, so please ignore this part)

#include <stdio.h>
#include <string.h>
#include <termios.h>
#include <unistd.h>
#include <fcntl.h>
#include <ncurses.h>

#define vertical 40
#define horizontal 200
#define down 115
#define up 119
#define left 97
#define right 100



typedef struct Snake
{
    char symbol;
    int size;
    char direction;
    char prev_direction;
    int tail_X;
    int tail_Y;
    int head_X;
    int head_Y;

}snake;


typedef struct snake_pos
{
    int Y[vertical*horizontal];
    int X[vertical*horizontal];

}snake_pos;



typedef struct food
{
    int X;
    int Y;
    char symbol;

}food;



void snake_init(snake *snake1);
void pos_init(snake_pos *pos1);
void food_init(food *food1);
void gotoxy(int,int);
void snake_place(snake *snake1, snake_pos *pos1);
void snake_move(snake *snake1, snake_pos *pos1, food *food1, int*);
void move_tail(snake *snake1, snake_pos *pos1);
void move_head(snake *snake1, snake_pos *pos1);
void food_print(food *food1);
int game_over(snake *snake1, snake_pos *pos1);
void set_borders();
void print_score(int*);



int main()
{
      int snake_speed=100000;
      int score=0;

      /* struct init */

      snake snake1;
      snake_pos pos1;
      food food1;
      snake_init(&snake1);
      pos_init(&pos1);
      food_init(&food1);


      /* set initial field */

      system("clear");
      system("stty -echo");
      curs_set(0);                    // doesn't work for some reason
      snake_place(&snake1,&pos1);
      set_borders();
      food_print(&food1);


      /* use system call to make terminal send all keystrokes directly to stdin */

      system ("/bin/stty raw");


      /* while snake not got collided into itself */

      while(!(game_over(&snake1,&pos1)))
      {

          /* while key not pressed */

          while (!kbhit())
          {
                 usleep(snake_speed);
                 snake_move(&snake1,&pos1,&food1,&score);
                 if (game_over(&snake1,&pos1))
                 {
                     break;
                 }

          }
          /* store previous direction and fetch a new one */

          snake1.prev_direction=snake1.direction;
          snake1.direction=getchar();



     }
      /* use system call to set terminal behaviour to more normal behaviour */
      system ("/bin/stty cooked");
      system("stty echo");
      system("clear");
      printf("\n\n Final score: %d \n\n", score);



      return 0;

}




void snake_init(snake *snake1)
{
    snake1->symbol='*';
    snake1->size=10;
    snake1->direction=right;
    snake1->prev_direction=down;
    snake1->tail_X=5;
    snake1->tail_Y=5;
    snake1->head_X=snake1->tail_X+snake1->size-1;
    snake1->head_Y=5;
}


void snake_place(snake *snake1, snake_pos *pos1)
{
    int i;
    for (i=0; i<snake1->size; ++i)
    {
        gotoxy(snake1->tail_X,snake1->tail_Y);
        printf("%c",snake1->symbol);
        pos1->X[i]=snake1->tail_X;
        pos1->Y[i]=snake1->tail_Y;
        snake1->tail_X+=1;
    }

}

void set_borders()
{
    int i;
    for (i=0; i<vertical; ++i)
    {
        gotoxy(0,i);
        printf("X");
        gotoxy(horizontal,i);
        printf("X");
    }

    for (i=0; i<horizontal; ++i)
        {
            gotoxy(i,0);
            printf("X");
            gotoxy(i,vertical);
            printf("X");
        }
}



void snake_move(snake *snake1, snake_pos *pos1, food *food1, int *score)
{
    move_head(snake1,pos1);

    if (!((snake1->head_X==food1->X) && (snake1->head_Y==food1->Y)))
    {
        move_tail(snake1,pos1);
    }
    else
    {
        snake1->size++;
        *score=*score+1;
        food1->X=rand()%(horizontal-5);
        food1->Y=rand()%(vertical-5);
        food_print(food1);
    }
}



void move_tail(snake *snake1, snake_pos *pos1)
{
    int i;

    // remove last cell of tail
    gotoxy(pos1->X[0],pos1->Y[0]);
    printf(" ");


    // update new tail position
    for (i=0; i<snake1->size; ++i)
    {
        pos1->X[i]=pos1->X[i+1];
        pos1->Y[i]=pos1->Y[i+1];
    }
}



void move_head(snake *snake1, snake_pos *pos1)
{
    switch (snake1->direction)
        {
            case right:
                if (snake1->prev_direction==left)
                {
                    snake1->head_X--;
                    break;
                }
                    snake1->head_X++;
                    break;

            case left:
                if (snake1->prev_direction==right)
                {
                    snake1->head_X++;
                    break;
                }
                    snake1->head_X--;
                    break;


            case up:
                if (snake1->prev_direction==down)
                {
                    snake1->head_Y++;
                    break;
                }
                    snake1->head_Y--;
                    break;


            case down:
                if (snake1->prev_direction==up)
                {
                    snake1->head_Y--;
                    break;
                }
                    snake1->head_Y++;
                    break;


            default:
                 break;
        }


        // update tail position
        pos1->X[snake1->size]=snake1->head_X;
        pos1->Y[snake1->size]=snake1->head_Y;

        gotoxy(pos1->X[snake1->size],pos1->Y[snake1->size]);
        printf("%c",snake1->symbol);
}



void food_init(food *food1)
{
    food1->X=(rand()%(horizontal-5))+1;
    food1->Y=(rand()%(vertical-5))+1;
    food1->symbol='F';
}


void food_print(food *food1)
{
    gotoxy(food1->X,food1->Y);
    printf("%c",food1->symbol);

}


void gotoxy(int x,int y)
{
    printf("%c[%d;%df",0x1B,y,x);
}



void pos_init(snake_pos *pos1)
{
    memset(pos1, 0, sizeof(*pos1));
}


int game_over(snake *snake1, snake_pos *pos1)
{
    int i;

    for (i=0; i<snake1->size-1; ++i)
    {
        if ((pos1->X[i]==snake1->head_X) && (pos1->Y[i]==snake1->head_Y))
        {
            return 1;
        }
    }


    if ((snake1->head_X==horizontal) || (snake1->head_X==1) || (snake1->head_Y==vertical) || (snake1->head_Y==1))
        {
            return 1;
        }


    return 0;
}



int kbhit(void)
{
  struct termios oldt, newt;
  int ch;
  int oldf;

  tcgetattr(STDIN_FILENO, &oldt);
  newt = oldt;
  newt.c_lflag &= ~(ICANON | ECHO);
  tcsetattr(STDIN_FILENO, TCSANOW, &newt);
  oldf = fcntl(STDIN_FILENO, F_GETFL, 0);
  fcntl(STDIN_FILENO, F_SETFL, oldf | O_NONBLOCK);

  ch = getchar();

  tcsetattr(STDIN_FILENO, TCSANOW, &oldt);
  fcntl(STDIN_FILENO, F_SETFL, oldf);

  if(ch != EOF)
  {
    ungetc(ch, stdin);
    return 1;
  }

  return 0;
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ You're using an old compiler. Out of curiosity, what is it and where did you get it? I've been meaning to play with old-fashioned C for a while. \$\endgroup\$
    – anon
    Commented Jun 21, 2016 at 14:37
  • \$\begingroup\$ gcc version 4.8.5 20150623 (Red Hat 4.8.5-4) (GCC) - hope this answered your question. \$\endgroup\$
    – manish ma
    Commented Jun 21, 2016 at 14:50
  • 5
    \$\begingroup\$ That version of the compiler supports lots more than you're using. You just need to specify the dialect using something like -std=c11. \$\endgroup\$
    – Edward
    Commented Jun 21, 2016 at 14:57

3 Answers 3

6
\$\begingroup\$

Minor

Only small things to add to the other 2 posts

  1. Incomplete declaration. A declaration without parameters conveys no information about the parameters. Thus a call to set_borders(1,2,3) before the definition of set_borders() will not flag an error. Use void.

    // void set_borders();
    void set_borders(void);
    
  2. Formatting: Why indent the last 2 lines?

        case right:
            if (snake1->prev_direction==left)
            {
                snake1->head_X--;
                break;
            }
                snake1->head_X++;
                break;
    
  3. Formatting: Excessive vertical spacing. Many places in code. Certainly a style issues, but this, IMO, beyond a useful amount.

    void food_print(food *food1)
    {
        gotoxy(food1->X,food1->Y);
        printf("%c",food1->symbol);
        // why blank line here?
    }
    
    
          system ("/bin/stty cooked");
          system("stty echo");
          system("clear");
          printf("\n\n Final score: %d \n\n", score);
    
        // why blank line here?
        // why blank line here?
          return 0;
        // why blank line here?
     }
    
  4. Pedantic: Careful with computing sizes. Both vertical and horizontal are int constants. As array sizes are best characterized with type size_t, on coding using much larger values, int * int may overflow int math, but not size_t math. Not an issue will this small program, but as a guide for large task, insure math is done with size_t math

    // int Y[vertical*horizontal];
    int Y[(size_t)1 * vertical*horizontal];
    
\$\endgroup\$
5
\$\begingroup\$

kbhit and direction changes

You're using your kbhit function to detect when a key has been pressed, however you're not actually checking that the key hit was one that is valid for your snake movement. Consequently, if another key on the keyboard is pressed you set direction to an unknown value which results in game over. You should be validating that the input is an expected value before setting it to direction.

snake_pos

This looks very counter-intuitive. Why would X need to be more than horizontal? Why would Y need to be more than vertical? Maybe you're looking for a two dimensional array int position[vertical][horizontal]?

typedef struct snake_pos
{
    int Y[vertical*horizontal];
    int X[vertical*horizontal];

}snake_pos;

Terminal Size / Wall collision detection

Collision detection didn't work as expected when I ran your program, currently you can run straight off the right side of the screen, turn around by going down and left and come back onto the screen.

It turns out this is because although my terminal is only 80 characters, horizontal is set to 200, so it keeps going off the side. The terminal is capping the right hand printing, so the walls are printed correctly. Ideally you would either detect, or ask the user for their terminal size.

You don't often see two breaks in the same case statement, maybe it should be:

case right:
    if (snake1->prev_direction==left)
        snake1->head_X--;
    else
        snake1->head_X++;
    break;

magic numbers

You use some defines at the top for magic numbers, however you also use 5 in several places like this:

food1->X=rand()%(horizontal-5)

It's not immediately clear where this number comes from, so it could benefit from being a defined constant.

\$\endgroup\$
5
\$\begingroup\$
typedef struct snake_pos
{
    int Y[vertical*horizontal];
    int X[vertical*horizontal];

}snake_pos;

This would have been better written as follows:

typedef struct snake_pos
{
    int X;
    int Y;

}snake_pos;

snake_pos pos1[vertical*horizontal];

Fetching X and Y coordinates from different vectors is somewhat weird. You should try to group data which is accessed in the same context, rather than attempting to sort it by type.

E.g.

// update new tail position
for (i=0; i<snake1->size; ++i)
{
    pos1->X[i]=pos1->X[i+1];
    pos1->Y[i]=pos1->Y[i+1];
}

then simplifies into

// update new tail position
for (i=0; i<snake1->size; ++i)
{
    pos1[i] = pos1[i+1];
}

I also couldn't help but notice that you defined X/Y pairs multiple times, which always resulted in copying the X and Y components individually between different structs.

Define something such as a "position" once, and then just reuse / nest the struct definition.

E.g.

typedef struct Snake
{
    char symbol;
    int size;
    char direction;
    char prev_direction;
    snake_pos tail;
    snake_pos head;
}snake;

Properly reusing common structures helps reducing the size of your code significantly.


And a bug: You are placing the food randomly without checking for intersections with the snake's body. That can result in the food colliding with the snake.

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