Sprite List Problem

[Silentstrike]

I'm having a problem implementing a sprite list for a game engine that I am making.  I have an abstract sprite class that holds a sf::Image and sf::Sprite object.  In my main file I initialize sprite objects as pointers to pass into the list in the engine class.  However, I get an access error when I try to load the  SFML image.  I know that the error comes from the sf::image going out of scope, but I was wondering how I could alter my code without sacrificing the use of the sprite handler using abstract Sprite objects.

Here is some code:

main.cpp

Sprite *sprite;
sprite->Load("plane.png");
sprite->Set(100,100);
g_engine->AddSprite(sprite);

sprite.h

class Sprite
    {
    protected:
sf::Image image;
sf::Sprite sprite;

public:
        Sprite();
        ~Sprite();
       
        bool Load(std::string filename);

        ...
}

sprite.cpp

bool Sprite::Load(std::string filename)
    {
        if(image.LoadFromFile(filename)) <- EXC_BAD_ACCESS
        {
            sprite.SetImage(image);
            SetWidth(image.GetWidth());
            SetHeight(image.GetHeight());
            return true;
        } else {
            return false;
        }
   }

engine.h

std::list<Sprite*> sprites;

[texus]

I know that the error comes from the sf::image going out of scope

I don't see a place where it goes out of scope, I don't think this is your problem.

I think the problem lies here:

Sprite *sprite

Just don't use a pointer to the sprite class (of course "->" in your code must change to ".")

Code: [Select]

Sprite sprite

If you do want to keep the pointer then change the line to this: (of course when you are done with it you must delete it)

Code: [Select]

Sprite *sprite = new Sprite()

[Nexus]

Just use std::list<sf::Sprite>, this is much simpler and safer. new and delete are not necessary.

[Silentstrike]

I forgot to paste that line for the sprite initialization.

sprite = new Sprite();

The main problem now is that I get a bad access error in the sprite handler list in the engine class.  I know that std::list<sf::Sprite> is safer, but as I said before I use my own sprite class in the list and it would also make sense to pass in pointers to sprites rather than the sprites themselves.

So here's what i've got,  I don't really know why this code is a problem as I copied it straight out of a book and don't see any problems with it unless it.  The bad access error occurs when i push_back the list and pass in the sprite.

sprite.h

class Sprite
    {
    protected:
        sf::Image image;
        sf::Sprite sprite;
 
    public:
        Sprite();
        ~Sprite();
       
        bool Load(std::string filename);
    };


main.cpp

    Sprite *sprite;
    sprite = new Sprite();
    sprite->Load("plane.png");
    sprite->Set(100,100);
    g_engine->AddSprite(sprite);

engine.h

std::list<Sprite*> sprites;

engine.cpp

void Engine::AddSprite(Sprite *sprite)
    {
        sprites.push_back(sprite); <- EXC_BAD_ACCESS
    }

[texus]

Try using references instead of pointers.

Code: [Select]


Sprite sprite;
sprite.Load("plane.png");
sprite.Set(100,100);
g_engine->AddSprite(sprite);


engine.cpp

void Engine::AddSprite(Sprite &sprite)
{
sprites.push_back(&sprite);
}
Do you still have the error with this code?

But Nexus is right, you shouldn't use a list of pointers.

[Silentstrike]

I retried references and it still gave me the same error.  Also, when you pass the sprite into the list dont you get rid of the ampersand?  

Anyway, I thought that if you have to pass objects between classes, especially into a data structure, that you have to use pointers since references are not assignable.  I have also been told by a lot of resources online and in a few books that a list of pointers to sprites on the heap is good way of making a sprite handler as long as you delete them from memory properly.

Any other advice would be appreciated.

[texus]

g_engine->AddSprite(sprite);

Is your g_engine created correctly? It is a pointer so you might receive the EXC_BAD_ACCESS if you also forgot "= new Engine()" here.
I see no reason why g_engine should be a pointer, use pointers as few as possible.

Don't keep g_engine as a pointer, maybe this solves your problem. If not then I don't think I can help you because I don't see anything wrong.

Anyway, I thought that if you have to pass objects between classes, especially into a data structure, that you have to use pointers since references are not assignable.

Unless you want to be able to change the pointer or have a NULL pointer you should always try to use references in functions.
You can always use references, the list will make a copy of it.

I have also been told by a lot of resources online and in a few books that a list of pointers to sprites on the heap is good way of making a sprite handler

I don't know about a std::list, but for e.g. std::vector this would be correct. It has to allocate enough space for every object. A pointer is only 4 bytes while the sprite itself is a lot bigger. So when using a pointer reallocation will be faster.

Basically my advice would be to use pointers as less as possible.
I don't even think you can get a segfault or EXC_BAD_ACCESS when not using pointers.

[Silentstrike]

Making the engine not a pointer solved the problem.  The reason I made it a pointer was because a book I am reading told me to.  I probably did not allocate memory correctly for it which gave me the error.  Anyway thanks for the help.