Separating a one file game into multiple files so it can be enhanced

Question

This is a modified sample file of André LaMothe ants.c

#define NUM_ANTS 250
#define NUM_ROCKS 400

enum direction {ANT_NORTH, ANT_EAST, ANT_SOUTH, ANT_WEST};
struct ant
{
    int x, y;
    enum direction state;
    unsigned char color;
    unsigned char back_color;
};

struct ant ANTS[NUM_ANTS];

void Draw_Ground(void)
{
    for (int i = 0; i < NUM_ROCKS; i++) put_pixel(rand() % 320, rand() % 200, 7);
}

void Initialize_Ants(void)
{
    for (int i = 0; i < NUM_ANTS; i++)
    {
        ANTS[i].x = rand() % 320;
        ANTS[i].y = rand() % 200;
        ANTS[i].state = rand() % 4;
        ANTS[i].color = 10;
        ANTS[i].back_color = get_pixel(ANTS[i].x, ANTS[i].y);
    }
}

void Erase_Ants(void)
{
    for (int i = 0; i < NUM_ANTS; i++) put_pixel(ANTS[i].x, ANTS[i].y, ANTS[i].back_color);
}

void Move_Ants(void)
{
    for (int i = 0; i < NUM_ANTS; i++)
    {
        switch (ANTS[i].state)
        {
            case ANT_NORTH:
                ANTS[i].y--;
                break;
            case ANT_SOUTH:
                ANTS[i].y++;
                break;
            case ANT_EAST:
                ANTS[i].x++;
                break;
            case ANT_WEST:
                ANTS[i].x--;
        }

        ANTS[i].x = ANTS[i].x < 0 ? 319 : ANTS[i].x;
        ANTS[i].x = ANTS[i].x > 319 ? 0 : ANTS[i].x;
        ANTS[i].y = ANTS[i].y < 0 ? 199 : ANTS[i].y;
        ANTS[i].y = ANTS[i].y > 199 ? 0 : ANTS[i].y;

        if (7 == get_pixel(ANTS[i].x, ANTS[i].y)) ANTS[i].state = rand() % 4;
    }
}

void Behind_Ants(void)
{
    for (int i = 0; i < NUM_ANTS; i++) ANTS[i].back_color = get_pixel(ANTS[i].x, ANTS[i].y);
}

void Draw_Ants(void)
{
    for (int i = 0; i < NUM_ANTS; i++) put_pixel(ANTS[i].x, ANTS[i].y, ANTS[i].color);
}

int main (int argc, char* argv[])
{
    Draw_Ground();
    Initialize_Ants();
    while (!key_press())
    {
        Erase_Ants();
        Move_Ants();
        Behind_Ants();
        Draw_Ants();
        delay(5);
    }
    return 0;
}

And this is how a broke that into multiple C files.

Firstly the basics.h file that is included everywhere.

#ifndef BODY_H
#define BODY_H

struct position
{
    int x, y;
};

struct kinetix
{
    int vx, vy;
};

struct sprite
{
    unsigned long color, back_color;
};

#endif

Later i can replace struct position with struct rect {int x, y; int width, height;}, so i will have the whole area of the ant to give it to the bit blitting rendering functions together with a change of struct sprite to point to the sprite data and background data instead of just the color of the dot.

I wanted only game.c to know of all the data types and other C files to be unaware of each other. The rendering (toSDL.c) should know only of position and sprite, the physics (world.c) to know only of position and velocity, the ant type to do itself the movement (that is the AI).

Also i supposed that the order of things to happen is that the AI should move first, than the physics should make corrections on the object moving (have the ability to change it's: position, velocity, acceleration...), then draw the object.

Any ideas about game programming and good C coding style are mostly welcomed. Others about the speed of code are not so important at this moment.

The world is 2x sized up, the ants, the screen (640x480), ant moves 2 pixel per frame. In order to compile the source you will need the SDL Development Libraries.

game.c

#include "toSDL.h"
#include "world.h"
#include "ant.h"

#include <stdlib.h>
#include <SDL_main.h>

#define FPS 25
#define NUM_ANTS 400
#define NUM_ROCKS 800
#define COL_GRAY1 0xb0b0b0
#define COL_GRAY2 0x808080

struct ant ANTS[NUM_ANTS];

void Draw_Ground(void);
void Initialize_Ants(void);
void Erase_Ants(void);
void Move_Ants(void);
void Behind_Ants(void);
void Draw_Ants(void);

int main (int argc, char* argv[])
{

    set_video_mode();
    Draw_Ground();
    Initialize_Ants();

    while (!kbhit())
    {
        unsigned int then = now();
        Erase_Ants();
        Move_Ants();
        Behind_Ants();
        Draw_Ants();
        delay_fps(FPS, then);
    }

    return 0;
}

void Draw_Ground(void)
{
    int i, x, y;

    for (i = 0; i < NUM_ROCKS; i++)
    {
        x = (rand() % 320) * 2;
        y = (rand() % 240) * 2;
        put_pixel(x, y, (rand() % 2 ? COL_GRAY1 : COL_GRAY2));
    }
}

void Initialize_Ants(void)
{
    int i;

    for (i = 0; i < NUM_ANTS; i++)
    {
        ANTS[i] = Ant_Init();
        ANTS[i].s.back_color = get_pixel_RGB(ANTS[i].p.x, ANTS[i].p.y);
    }
}

void Erase_Ants(void)
{
    int i;

    for (i = 0; i < NUM_ANTS; i++)
    {
        put_pixel(ANTS[i].p.x, ANTS[i].p.y, ANTS[i].s.back_color);
    }
}

void Move_Ants(void)
{
    int i;

    for (i = 0; i < NUM_ANTS; i++)
    {
        ANTS[i] = Ant_Move(ANTS[i]);
        ANTS[i].p = react_box(ANTS[i].p);
        ANTS[i].k = react_colision(ANTS[i].k, get_pixel_RGB(ANTS[i].p.x, ANTS[i].p.y));
    }
}

void Behind_Ants(void)
{
    int i;

    for (i = 0; i < NUM_ANTS; i++)
    {
        ANTS[i].s.back_color = get_pixel_RGB(ANTS[i].p.x, ANTS[i].p.y);
    }
}

void Draw_Ants(void)
{
    int i;

    for (i = 0; i < NUM_ANTS; i++)
    {
        put_pixel(ANTS[i].p.x, ANTS[i].p.y, ANTS[i].s.color);
    }
}

toSDL.h

void set_video_mode(void);

unsigned long get_pixel_RGB(int x, int y);
void put_pixel(int x, int y, unsigned long color);

int kbhit(void);
unsigned long now(void);
void delay_fps(unsigned int fps, unsigned int then);

toSDL.c

#pragma comment (lib, "C:\lang\BCC\Lib\SDL.lib")
#pragma comment (lib, "C:\lang\BCC\Lib\SDLmain.lib")

#include "toSDL.h"
#include <SDL.h>

static SDL_Event event;
static SDL_Surface* screen = 0;

void set_video_mode(void)
{
    SDL_Init(SDL_INIT_EVERYTHING);
    screen = SDL_SetVideoMode(640, 480, 0, SDL_ANYFORMAT);
}

unsigned long get_pixel(int x, int y)
{
    int bpp = screen->format->BytesPerPixel;
    unsigned char *p = (unsigned char *)screen->pixels + y * screen->pitch + x * bpp;
    unsigned long r = 0;

    switch (bpp)
    {
        case 1:
            r = *p;
            break;
        case 2:
            r = *(unsigned short *)p;
            break;
        case 3:
            if (SDL_BYTEORDER == SDL_BIG_ENDIAN)
                r = p[0] << 16 | p[1] << 8 | p[2];
            else
                r = p[0] | p[1] << 8 | p[2] << 16;
            break;
        case 4:
            r = *(unsigned long *)p;
            break;
    }
    return r;
}

unsigned long get_pixel_RGB(int x, int y)
{
    unsigned char R, G, B;

    SDL_GetRGB(get_pixel(x, y), screen->format, &R, &G, &B);
    return SDL_MapRGB(screen->format, R, G, B);
}

void put_pixel(int x, int y, unsigned long color)
{
    SDL_Rect t = {0, 0, 2, 2};

    t.x = x;
    t.y = y;
    color = SDL_MapRGB(screen->format, (color >> 16) & 0xff, (color >> 8) & 0xff, color & 0xff);
    SDL_FillRect(screen, &t, color);
}

int kbhit(void)
{
        if (SDL_PollEvent(&event))
        {
            if (SDL_QUIT == event.type)
                return 1;
            else if (SDLK_ESCAPE == event.key.keysym.sym)
                return 1;
            else
                return 0;   
        }
        else return 0;
}

unsigned long now(void)
{
    return SDL_GetTicks();
}

void delay_fps(unsigned int fps, unsigned int then)
{
    if (1000/fps > (SDL_GetTicks() - then))
        SDL_Delay(1000/fps - (SDL_GetTicks() - then));
    SDL_Flip(screen);
}

ant.h

#include "basics.h"

#define ANT_WIDTH 2
#define ANT_HEIGHT 2

#define COL_GREEN 0x00ff00
#define COL_RED 0xff0000

struct ant
{
    struct position p;
    struct kinetix k;
    struct sprite s;
};

struct ant Ant_Init(void);
struct ant Ant_Move(struct ant a);

ant.c

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

struct ant Ant_Init(void)
{
    struct ant a = {0};

    a.p.x = (rand() % 320) * 2;
    a.p.y = (rand() % 240) * 2;

    {
        int t = rand() % 4;
        switch (t)
        {
            case 0:
                a.k.vx = -2;
                a.k.vy = 0;
                break;
            case 1:
                a.k.vx = 2;
                a.k.vy = 0;
                break;
            case 2:
                a.k.vx = 0;
                a.k.vy = -2;
                break;
            case 3:
                a.k.vx = 0;
                a.k.vy = 2;
                break;
        }
    }

    a.s.color = rand() % 2 ? COL_RED : COL_GREEN;
    return a;
}

struct ant Ant_Move(struct ant a)
{
    a.p.x += a.k.vx;
    a.p.y += a.k.vy;

    return a;
}

world.h

#include "basics.h"

struct position react_box(struct position p);
struct kinetix react_colision(struct kinetix k, unsigned long back_color);

world.c

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

struct position react_box(struct position p)
{
    p.x = p.x > 638 ? 0 : p.x;
    p.x = p.x < 0 ? 638 : p.x;

    p.y = p.y > 478 ? 0 : p.y;
    p.y = p.y < 0 ? 478 : p.y;

    return p;
}

struct kinetix react_colision(struct kinetix k, unsigned long back_color)
{
    if (back_color)
    {
        struct kinetix w = {0};
        do
        {
            int t = rand() % 4;
            switch (t)
            {
                case 0:
                    w.vx = -2;
                    w.vy = 0;
                    break;
                case 1:
                    w.vx = 2;
                    w.vy = 0;
                    break;
                case 2:
                    w.vx = 0;
                    w.vy = -2;
                    break;
                case 3:
                    w.vx = 0;
                    w.vy = 2;
                    break;
            }
        } while ((k.vx == w.vx) && (k.vy == w.vy));
        return w;
    }
    return k;
}

Answer 1

First of all you've done a good job, the functions are atomic and perform only one action. The code is well indented and quite readable. The file names seem to be a good choice as well.

So just a few suggestions:

Follow a C Coding Standard When Programming in C

The C programming language and the C++ programming language are distinct languages. Here on CodeReview C++ is defined as

C++ is a statically typed, free-form, multi-paradigm, compiled, general-purpose programming language.

and C is defined as

C is a general-purpose computer programming language used for operating systems, games, and other high performance work and is clearly distinct from C++. It was developed in 1972 by Dennis Ritchie for use with the Unix operating system.

There are two coding standards that might be applicable, either C99 or C11. C11 would probably be the proper choice for new programs.

Many compilers today will allow the programmer to select a standard, in the gcc online documents:

-std= The compiler can accept several base standards, such as ‘c90’ or ‘c++98’, and GNU dialects of those standards, such as ‘gnu90’ or ‘gnu++98’. When a base standard is specified, the compiler accepts all programs following that standard plus those using GNU extensions that do not contradict it. For example, -std=c90 turns off certain features of GCC that are incompatible with ISO C90, such as the asm and typeof keywords, but not other GNU extensions that do not have a meaning in ISO C90, such as omitting the middle term of a ?: expression. On the other hand, when a GNU dialect of a standard is specified, all features supported by the compiler are enabled, even when those features change the meaning of the base standard. As a result, some strict-conforming programs may be rejected. The particular standard is used by -Wpedantic to identify which features are GNU extensions given that version of the standard. For example -std=gnu90 -Wpedantic warns about C++ style ‘//’ comments, while -std=gnu99 -Wpedantic does not.

While this C coding standard is for a specific class at a university it gives some good tips.

The Use of typedef Might Help

The typedef declaration provides a way to declare an identifier as a type alias, to be used to replace a possibly complex type name. By using typedef in the struct declarations the structs becomes types similar in some ways to int, double and char.

An Example Using typedef

typedef struct
{
    int x;
    int y;
} Position;

The use of typedefs in the code would reduce the amount of typing needed everywhere a type is used in the program as shown above.

In world.h:

Position react_box(Position p);

Variable Names

While the original code had the variable name ANTS for the array of ants, the C coding standard indicates that constants should be defined using all capitals and variable names should be a combination of capitals and lower case, but mostly lower case.

In the ant.c file the variable a might be better named ant, and the struct ant might be better name Ant. this might improve the readability of the code. The field p in the ant structure could be name position if the struct position was renamed to Position or the variable could be named either location. or pos.

It's not clear in the code what vx and vy mean in the kinetix struct. Are they vectors or velocities? In physics or math the shortest form of change in location might be dx and dy for delta x and delta y

In Ant_Init(void) what exactly is t?

The second page of the PDF coding standard in the cited in the first section provides some good tips on variable naming.

MAGIC Numbers

There are already symbolic constants in use in parts of the program, however, their use could be expanded. It is unclear what the numbers 2, -2, 320 and 240 mean in the functions Ant_Init(void) and void Draw_Ground(void). It is also unclear what the numbers -2 and 2 mean in the function react_colision(struct kinetix k, unsigned long back_color).

Symbolic constants help programmers in a number of ways:

• They make the code more readable
• They decrease the number of lines that need to be changed in edit, this makes changing the code easier in the future.

If 320 and 240 are screen size, it would definitely be easier to modify the code in the future if symbolic constants are used, then the edit would only need to modify 2 lines in a header file.

There are 2 constants defined in stdlib.h and stdlib, these are EXIT_SUCCESS and EXIT_FAILURE. These make main() more readable rather than return 0, return EXIT_SUCCESS or EXIT_FAILURE from main, this will also allow error handling to be added at some point in the future.

Global Variables

While the global variable ANTS seems to be only in game.c it could be modified in any file because of the way it is declared. If it should only be modified in game.c then it should be declared as static which would limit the scope to only game.c. It would be even better if ANTS was declared within main and passed by reference or value to the functions that need it.

Using global variables can make it very difficult to write correct code and to debug. The ANTS variable can be changed anywhere in the program and as more lines of code and files are added it becomes very difficult to find where it is modified.

Because the scope of ANTS is program wide NUM_ANTS and probably NUM_ROCKS should be moved to the basics.h header file.