The mystery of the appearing enemy fighter

Hello to all, recently I have been working on a fairly complex game similar to the widely popular 1980's game "Space Invaders". Of course, I will deviate from this path and make it into a game that is clearly my own. Due to lack of creativity, and for the simplicity of the implementation, the movements and some graphics will be somewhat similar for the time being. Enough of my pointless babbling though, I need to get to what my question was. I had the code worked out, and everything was great. Then, when I tried to load in 30 enemy images for use later in the game, I noticed that, mysteriously, an enemy fighter had appeared without me ever blitting it onto the screen. I think that this problem also has to do with the mysterious "empty space" mystery also. It seems to go black occasionally, even though all my background images are never completely black. Originally I thought it was some kind of illegal memory access in my code, but it would've crashed with signal SIGSEGV if it was. Which leads me to believe it's my implementation somewhere in my code. I have no clue as to how the enemy fighter started appearing every time the "empty space" mystery occurs. I was hoping you would help me figure out what I'm doing wrong.

Here is the full source code, I commented out the enemy fighter parts for obvious reasons.

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#include <math.h>
#include <process.h>


#include <SDL/SDL.h>


#define MAX_SPACE_IMAGES 3
#define SCREEN_WIDTH 920
#define SCREEN_HEIGHT 518
#define FRAMES_PER_SECOND 70
#define SPACE_UPDATE_FRAME 20
#define MAX_LASER_QUEUE 1


#define MAX_ENEMIES 15


#define TRUE 1
#define FALSE 0


typedef struct enemy_fighter
{
    SDL_Surface * image;


    int x;
    int y;


    unsigned is_dead:1;
    unsigned move_right:1;
    unsigned move_left:1;
    unsigned move_down:1;


} enemy_fighter;


typedef struct laser
{
    SDL_Rect shape;
    int node_id;
    int speed;


    struct laser * next;
} laser;


void SDL_errorexit( const char * error_message )
{
    char buffer[BUFSIZ];
    sprintf(buffer, "%s : %s", error_message, SDL_GetError());
    fprintf(stderr, "%s", buffer);


    exit(1);
}


void init_SDL( )
{
    if ( SDL_Init( SDL_INIT_EVERYTHING ) < 0 )
        SDL_errorexit("SDL_Init");


    return;
}


laser * create_list_node( )
{
    laser * temp = NULL;


    if ( !(temp = malloc(sizeof(*temp))) )
    {
        perror("Malloc");
        exit(1);
    }


    return temp;
}


laser * append_list( laser * lasers, int id, int speed, int x, int y, int w, int h )
{
    laser * temp = NULL;


    if ( !lasers )
    {
        lasers = create_list_node( );


        lasers->node_id = id;
        lasers->speed = speed;
        lasers->shape.h = h;
        lasers->shape.x = x;
        lasers->shape.y = y;
        lasers->shape.w = w;
        lasers->next = NULL;
    } else
    {
        for ( temp = lasers; temp->next; temp = temp->next );


        temp->next = create_list_node();


        temp->next->node_id = id;
        temp->next->speed = speed;
        temp->next->shape.h = h;
        temp->next->shape.x = x;
        temp->next->shape.y = y;
        temp->next->shape.w = w;
        temp->next->next = NULL;
    }


    return lasers;
}


laser * remove_first_node( laser * lasers )
{
    laser * temp = lasers;
    lasers = lasers->next;
    free(temp);


    return lasers;
}


void release_list( laser * lasers )
{
    laser * temp = NULL;


    while ( lasers )
    {
        temp = lasers;
        lasers = lasers->next;
        free(temp);
    }


    return;
}


SDL_Surface * load_image_opt( char * image_path, int is_transparent )
{
    SDL_Surface * image = NULL;
    SDL_Surface * temp_image = SDL_LoadBMP(image_path);


    if ( !temp_image )
        SDL_errorexit("SDL_LoadBMP");


    if ( is_transparent )
        SDL_SetColorKey( temp_image, SDL_SRCCOLORKEY, SDL_MapRGB(temp_image->format, 255, 255, 255) );


    image = SDL_DisplayFormat( temp_image );
    SDL_FreeSurface( temp_image );


    return image;
}


SDL_Surface * set_video_mode( const char * window_title )
{
    SDL_Surface * console = SDL_SetVideoMode( SCREEN_WIDTH, SCREEN_HEIGHT, 24, SDL_SWSURFACE | SDL_DOUBLEBUF );
    SDL_WM_SetCaption( window_title, "N/A" );


    if (!console)
        SDL_errorexit("SDL_SetVideoMode");


    return console;
}


void game_intro( SDL_Surface ** space, SDL_Surface * title, SDL_Surface * console )
{
    SDL_Rect title_rect;


    int offset = 0;
    int iteration;


    title_rect.x = (console->w - title->w) / 2;
    title_rect.y = (console->h - title->h) / 2;
    title_rect.h = title->h;
    title_rect.w = title->w;


    for ( iteration = 0; iteration < 9; iteration++ )
    {
        offset = (offset > MAX_SPACE_IMAGES - 1 ? 0 : offset + 1);


        SDL_FillRect(console, 0, SDL_MapRGB(console->format, 0, 0, 0)); /* Clear the screen */


        SDL_BlitSurface(space[offset], 0, console, NULL );
        SDL_BlitSurface(title, 0, console, &title_rect);
        SDL_Flip(console); /* Update the screen */


        SDL_Delay(250); /* Delay the loop */
    }


    return;
}


void handle_events( int * quit, int * laser_request, int * x )
{
    SDL_Event event;


    /* While-loop was from SDL documentation : http://www.libsdl.org/release/SDL-1.2.15/docs/html/guideinputkeyboard.html */


    while( SDL_PollEvent( &event ) )
    {
        switch( event.type )
        {
            case SDL_QUIT :
                *quit = 1;
            break;
            /* Keydown event */
            case SDL_KEYDOWN :
                switch( event.key.keysym.sym )
                {
                    case SDLK_LEFT :
                        *x = -5;
                    break;
                    case SDLK_RIGHT :
                        *x = 5;
                    break;
                    case SDLK_SPACE :
                        *laser_request = 1;
                    break;
                    case SDLK_ESCAPE :
                        *quit = 1;
                    break;
                    default : break;
                }
            break;
            /* End keydown event */
            /* Keyup event */
            case SDL_KEYUP:
                switch( event.key.keysym.sym )
                {
                    case SDLK_LEFT :
                        if( *x < 0 )
                            *x = 0;
                    break;
                    case SDLK_RIGHT :
                        if( *x > 0 )
                            *x = 0;
                    break;
                    case SDLK_SPACE :
                    if ( *laser_request > 0 )
                        *laser_request = 0;
                    break;
                    default : break;
                }
                /* End keyup event */
            break;
            default : break;
        }
    }


    return;
}


inline void move_fighter( SDL_Surface * console, SDL_Rect * fighter, int * x )
{
        if ( fighter->x >= console->w - 75 )
        {
            *x = 0;
            fighter->x = console->w - 76;
        }


        fighter->x += *x;


        return;
}


inline void update_space_array_offset( int * offset, int * frame )
{
    if ( *frame == SPACE_UPDATE_FRAME )
        *offset = (*offset > MAX_SPACE_IMAGES - 1 ? 0 : *offset + 1);
    return;
}


inline void reset_frame( int * frame )
{
    *frame = *frame == SPACE_UPDATE_FRAME ? 0 : *frame + 1;
    return;
}


inline void wait( unsigned time_start, unsigned time_end )
{
    if ( (time_end - time_start) < (1000 / FRAMES_PER_SECOND) )
            SDL_Delay( (( 1000 / FRAMES_PER_SECOND ) - ( time_end - time_start )) );
    return;
}


void game_start( SDL_Surface ** space, SDL_Surface * space_fighter, SDL_Surface * console )
{
    unsigned time_start = 0;
    unsigned time_end = 0;


    SDL_Rect fighter;


    int frame = 0;
    int offset = 0;
    int quit = 0;
    int x = 0;
    int laser_request = 0;
    int laser_queue = 0;
    int amount_of_lasers = 0;


    fighter.x = (console->w - space_fighter->w ) / 2; /* Set the fighter in the middle of the game */
    fighter.y = console->h - 100; /* Set it 100 pixels minus the screen size */
    fighter.h = 75;
    fighter.w = 75;


    laser * lasers = NULL;
    laser * temp = NULL;


    srand(time(NULL)); /* Randomize our node identifiers, may be needed in the future */


    while ( !quit )
    {
        time_start = SDL_GetTicks(); /* Start the timer here to cap our frame rate later. */


        update_space_array_offset( &offset, &frame ); /* Update the background image array if needed */
        handle_events( &quit, &laser_request, &x ); /* Handle all the events and store the results in the parameters */
        move_fighter( console, &fighter, &x ); /* Handle moving the fighter according to the result of the event handler */


        /*  Limit the lasers to a preset constant, can be any number,
        /  but obviously super high amounts will do nothing. */
        if ( amount_of_lasers < MAX_LASER_QUEUE )
            laser_queue += laser_request;


        for ( ; laser_queue; --laser_queue, ++amount_of_lasers )
        {
            lasers = append_list( lasers, rand(), 1, ((fighter.x + fighter.w / 2) - 2), fighter.y, 2, 16 ); /* Create a linked list to store all the valid requests */
            spawnl(P_NOWAIT, "sound.exe", "sound.exe", "laser.txt", NULL); /* This was simpler than trying to multithread to play it at the same time. */
        }


        if ( lasers )
        {
            for ( temp = lasers; temp; temp = temp->next )
            {
                if ( temp->shape.y > 0 )
                {
                    temp->shape.y -= 20;
                }
                else
                {
                    lasers = remove_first_node(lasers); /* Needs to be updated to have the ability to remove nodes in the middle of the list */
                    --amount_of_lasers; /* Decrease this, so it always represents the exact amount of lasers in existence */
                }
            }
        }


        /* Drawing starts here */


        SDL_FillRect(console, 0, SDL_MapRGB(console->format, 0, 0, 0)); /* Clear the screen */


        SDL_BlitSurface(space[offset], 0, console, NULL );
        SDL_BlitSurface(space_fighter, 0, console, &fighter);




        for ( temp = lasers; temp; temp = temp->next )
            SDL_FillRect(console, &temp->shape, SDL_MapRGB(console->format, 0, 255, 0)); /* Update each laser */


        SDL_Flip(console); /* Update the screen */


        /* End drawing */


        time_end = SDL_GetTicks();


        wait( time_start, time_end ); /* Wait until our FPS limit has been reached if executing faster than the limit */
        reset_frame( &frame );
    }


    if ( lasers ) /* There shouldn't be any lasers left, but we need to make sure */
        release_list(lasers);


    return;
}


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


    /*
    enemy_fighter enemies[MAX_ENEMIES] = {{NULL}};
    */
    SDL_Surface * space[MAX_SPACE_IMAGES] = {NULL};
    SDL_Surface * title = NULL;
    SDL_Surface * space_fighter = NULL;
    SDL_Surface * console = NULL;


    int offset;


    /* Initializing SDL... */


    init_SDL();


    if  (atexit(SDL_Quit) )
    {
        perror("atexit");
        exit(1);
    }


    /* Screen configurations */


    console = set_video_mode( "Space Fighter" );


    /* Image loading */


    /*
    for ( offset = 0; offset < MAX_ENEMIES; offset++ )
    {
        enemies[offset].image = load_image_opt( "enemy_fighter.bmp", 1 );
        enemies[offset].is_dead = FALSE;
        enemies[offset].move_down = FALSE;
        enemies[offset].move_left = TRUE;
        enemies[offset].move_right = FALSE;
        enemies[offset].x = 0;
        enemies[offset].y = 0;
    }
    */


    space[0] = load_image_opt( "space_dark.bmp", 0 );
    space[1] = load_image_opt( "space_lighter.bmp",  0 );
    space[2] = load_image_opt( "space_light.bmp", 0 );
    title = load_image_opt( "space_fighter_intro.bmp", 1 );
    space_fighter = load_image_opt( "space_fighter.bmp", 1 );


    /* Start the game introduction */


    game_intro( space, title, console );


    SDL_FreeSurface(title); /* We no longer need the introduction title */


    /* Start the game */


    game_start( space, space_fighter, console );


    /* Free allocated memory */


    for (offset = 0; offset < MAX_SPACE_IMAGES - 1; offset++ )
        SDL_FreeSurface(space[offset]);


    /*
    for ( offset = 0; offset < MAX_ENEMIES; offset++ )
        SDL_FreeSurface(enemies[offset].image);
    */


    SDL_FreeSurface(space_fighter);


    return 0;
}

---

Here's exhibit A, the game screen after the title screen with no "empty space" :

Attachment 12926

Here's exhibit B, the game screen after the title screen with the mysterious enemy_fighter in the top left corner.

Attachment 12923

Here are the background images, and as you can see, none of them should be completely black.

This is space_dark in .png format :
Attachment 12924
This is space_lighter in .png format :
Attachment 12925
This is space_light in .png format :
Attachment 12925

Any help is appreciated.

Your program is too long for me to dig through, but I imagine it's nothing more than a simple logical error. Have you tried debugging it? Putting breakpoints at the point something may go awry? Putting in asserts for things that should never happen?

And:

Quote:

---

"Originally I thought it was some kind of illegal memory access in my code, but it would've crashed with signal SIGSEGV if it was"

---

is absolutely 100% wrong. SIGSEGV only occurs if the OS detects a massively illegal memory access (i.e. outside of the memory segments allocated to the program - hence the SEG and the word segfault), it doesn't stop you corrupting memory in any way and won't detect most things at all if they are small logical errors (like being one-off in the particular size, etc.). Don't think it does, and don't assume that because you don't get a SIGSEGV everything is alright.

You've written a program - now go debug it. What have you tried in terms of actual debugging?

After a quick look through, which is by no means accurate or conclusive, I just wanted to make sure your program even compiled properly...

Just from the screenshots it looks like the background image is actually changed to the "fighter" image. This is why you get a black frame with no stars, and the enemy fighter appears at 0,0 - your code is drawing what it thinks is the background but it has been handed the fighter image.

That would be where I would start to look - and probably that "linked list" implementation you have there. That would be my most likely culprit. But, as I say, I haven't time to debug other people's programs for them if they haven't even thought to debug / stick a printf in there somewhere.

Personally, I'd be sticking a "printf("Just printed the background, size %i, %i\n", surface->w, surface->h);" in there somewhere when I draw my background. I bet when the problem occurs that the surface drawn changes size because you're actually printing the fighter, not the background. That whole "offset" thing looks likely to be the cause of the breakage to me.

Quote:

---

Originally Posted by ledow

Your program is too long for me to dig through, but I imagine it's nothing more than a simple logical error. Have you tried debugging it? Putting breakpoints at the point something may go awry? Putting in asserts for things that should never happen?

And:



is absolutely 100% wrong. SIGSEGV only occurs if the OS detects a massively illegal memory access (i.e. outside of the memory segments allocated to the program - hence the SEG and the word segfault), it doesn't stop you corrupting memory in any way and won't detect most things at all if they are small logical errors (like being one-off in the particular size, etc.). Don't think it does, and don't assume that because you don't get a SIGSEGV everything is alright.

You've written a program - now go debug it. What have you tried in terms of actual debugging?

---

I didn't do much in terms of debugging, but after you posted I went through and looked at my code. One of the reasons I didn't think there was a SIGSEGV was because I thought SDL would handle it, since it usually does. Apparently, I shouldn't rely on it, I ran it through my Windows debugger when I was looking through my code and immediately discovered a segfault on line 327. At first I didn't understand why it would happen because the y coordinate was a proper variable and wasn't a trash value. Then I thought that maybe the node was removed and then my code tried to access that value again. I added a fprintf() to see if the node was removed or not. It was removed when it reached under 0 and then it was tested a second time causing the segfault. I added a break to the loop believing that would fix it :

Code:

---

if ( lasers )
        {
            for ( temp = NULL, temp = lasers; temp; temp = temp->next )
            {
                if ( temp->shape.y > 0 )
                    temp->shape.y -= 20;
                else
                {
                    lasers = remove_first_node(lasers); /* Needs to be updated to have the ability to remove nodes in the middle of the list */
                    --amount_of_lasers; /* We need to decrease this, so it always represents the exact amount of lasers in existence */
                    break;
                }
            }
        }

---

That fixed that issue, although I'm not entirely sure why the loop didn't catch it. As for my space going black problem, that was a logical error.

Code:

---

inline void update_space_array_offset( int * offset, int * frame )
{
    if ( *frame == SPACE_UPDATE_FRAME )
        *offset = (*offset == MAX_SPACE_IMAGES - 1 ? 0 : *offset + 1);
    return;
}

---

I changed the greater than to an equality comparison operator and that fixed that problem. The enemy fighter no longer appears when I added that code back in also.

I guess I could have fixed it by myself when I posted this if I would have looked a bit harder. At least I know now that I can't just keep relying on SDL's parachute to track segfaults for me.

I am just a beginner in C, so I don't understand the code. But this must take days to debug :|:|....

Quote:

---

Originally Posted by bos1234

I am just a beginner in C, so I don't understand the code. But this must take days to debug :|:|....

---

Now extend that to 1000s of lines of code for an OS kernel, where a debugger is almost impossible to be used, and all goes to hell because you failed to connect inline assembly correctly with everything else, and why? Because you forgot to declare a single register as used...
I spent a whole day( 24 hours ) in trying to solve this problem, and I was banging my head against the wall for making that foolish mistake. :p

Quote:

---

I am just a beginner in C, so I don't understand the code. But this must take days to debug :|:|....

---

500 lines? Sorry, but this is really nothing. A quick single-step through with a debugger will find the problem for you in less than ten minutes. Fixing it might take a little longer. If you don't have such advanced tools available (yeah, right, debuggers are ten-a-penny nowadays and you're compiling with something, I guarantee it has a debugger too), then just single-stepping through the code by means of printing statements as you go so you can trace through what's it done.

In it's simplest form, this is just a "printf" of various variables at various stages, and thus showing what values variables hold as you jump between functions. It'll generate a ton of output but you can just skim most of it until you see where the problem hits. But with any of the decent debuggers, you'll find that problem quite quickly if you understand the code and can single-step it. When I was a kid, "debugging" meant running through things on paper using your head as the compiler/computer, and still you'd find this problem in under a day. Way under a day.

But this is also why, when we build larger programs, we modularise them. You really don't want to worry about whether "append_list" etc. in the above are correct when you're relying on them so much. So you would modularise them, put them into a source file of their own, and test and debug them as a separate program before you started using them in your own code. That way you don't have to worry if what you've done is cocked up the list implementation - you'll know which part of the program must be wrong and can concentrate your efforts on the module you know is wrong. Here, there's just a bunch of code in one long file, which makes things harder. You remember all that stuff you learned about header files and .c files etc.? This is why we use them. Personally I'd have "list.h" and "list.c" which contained all the things I needed to do with lists, and the main program would just include it. Bam. Dozens or hundreds of lines of code out of your main file that you know do nothing but lists and which you can test separately.

And then when everything is in nice little blocks, your usual debugging will be much simpler too. Oh, the list is giving the wrong item back? Okay. Probably can ignore EVERYTHING else in the whole program and just concentrate on debugging my list.

If you think this is bad, I'm writing a 2D isometric game that is nowhere near complete. It's over 150,000 lines of C, all written by me. There's so much code in there, that I don't even RECOGNISE it as my own code when I come back to it years later. And though I might have individual C files with several thousand lines of code, it's modularised so they are self-contained, and I don't attempt to dig into problems without a decent debugger.

Code:

---

for ( iteration = 0; iteration < 9; iteration++ )   
{       
  offset = (offset > MAX_SPACE_IMAGES - 1 ? 0 : offset + 1);         
  SDL_FillRect(console, 0, SDL_MapRGB(console->format, 0, 0, 0)); /* Clear the screen */         
  SDL_BlitSurface(space[offset], 0, console, NULL );       
  SDL_BlitSurface(title, 0, console, &title_rect);       
  SDL_Flip(console); /* Update the screen */         
  SDL_Delay(250); /* Delay the loop */   
}

---

I do not see 30 images. I see code that loads 3 images. Instead of using that ternary just do:

Code:

---

  ...
  ++offset;
  if (offset >= MAX_SPACE_IMAGES)
  {
      offset = 0;
  }
  ...
  ...

---

Also if you are running in debug mode you can certainly blow the bounds of an array. Memory has tons of extra 'space' in it between objects while in debug mode. I suspect if you run this in release mode it will seg fault.