How do I correctly use SDL_FreeSurface when dealing with a vector of surfaces.

Question

I have setup a small shooter game as a tutorial for myself in SDL. I have a struct of a projectile

struct projectile
{
    SDL_Surface* surface;
    int x;
    int y;
};

And I put that into a vector.

vector<projectile> shot;
projectile one_shot;

And when I press space I create a new projectile and add it to the vector and then they're blitted when they're rendered.

This works fine, but I'm in seemingly random cases getting a "program has stopped working" error.

So I'm wondering what is the proper way to free the surfaces.

• Do I free them all afterwards?
• Do I free each individual shot when it exits the screen?
• Or some other choice?

UPDATE:

I have found where it crashes when I quit, when I have fired a few shots and they have all exited the screen. I have tried replacing the code that adds the surface to the vector with the "proper way to duplicate" as seen in this example, and it still behaves in the same way.

This is how I free the surface.

if(shot.at(i).y < 0 - shot.at(i).surface->h)
{
 SDL_FreeSurface(shot.at(i).surface);
 shot.erase(shot.begin() + i);
}

Anyone have an idea or some sample code I can look at to figure this out.

Answer 1

You should free the surface when you destroy the projectile. When you destroy the projectile is a game design decision; probably when it leaves the screen at the latest, but also of course when (if) it hits a target.

Answer 2

Do you use the same surface also elsewhere? Because if so, you cannot free it as long as it is used somewhere else.

In case you don't do that: Make the constructing/loading of your surface and the freeing in the constructor / destructor of projectile. I.e.:

struct projectile {
    SDL_Surface* surface;

    projectile() : surface(NULL) {
        surface = LoadImage(...);
    }

    ~projectile() {
        if(surface) {
             SDL_FreeSurface(surface);
             surface = NULL;
        }
    }

};

Answer 3

If several projectiles use the same sprite (as in almost all sprite-based games), it's probably better to use an image cache containing all the images used by your games and do memory management only there. Fill it at start or on demand and flush it when exiting. Then projectiles just need to ask to this cache a pointer to "arrow.png", the cache loads it (if needed) and returns the surface pointer.

Such cache can be a simple std::map< string, SDL___Surface* > with just functions like get_surface(string) and flush().

EDIT: an implementation of this idea:

class image_cache{
 map<string, SDL_Surface*> cache_;
 public:
 SDL_Surface* get_image(string file){
  map<string, SDL_Surface*>::iterator i = cache_.find(file);
  if(i == cache_.end()) {
   SDL_Surface* surf = SDL_LoadBMP(file.c_str());
   i = cache_.insert(i, make_pair(file, surf));
  }
  return i->second;
 } 
 void flush(){
  map<string, SDL_Surface*>::iterator i = cache_.begin();
  for(;i != cache_.end();++i)
   SDL_FreeSurface(i->second);
  cache_.clear();
 }
 ~image_cache() {flush();}
};

image_cache images;
// you can also use specialized caches instead of a global one
image_cache projectiles_images;

int main()
{
 ...
 SDL_Surface* surf = images.get_image("sprite.png");
 ...
}