Sprite-Object lost (Probably a scoping-problem) SOLVED

[Flyverse]

Hey!

I just wrote a little ResourceManager and a singleton to access these resource managers everyhwere (I know that Singletons are bad but no idea how to do it otherwise).
I also have some sort of an Entity, which uses those Managers to assign a Sprite* in the constructor and then uses the local reference in the render-function. When assigning in the constructor, everything is still "there": For example, I'm able to output the size of the texture the sprite is using. But in my render-function, suddenly, the pointer is null! I'm pretty sure it has something to do with the variable scope, but no idea what the exact problem is. Here some pseudo-code to visualize the problem:

Code: [Select]

class Entity {

public:
    Entity(){
        mySprite = Assets::instance()->spriteManager->request(spriteID); //If I debug fE texture size of mySprite, it outputs "good" results.
    }
    void render(){
        somewindow.draw(*mySprite); //ACCESS VIOLATION: Pointer is null
    }
private:
    sf::Sprite * mySprite;

}

Could anyone please help me ^^?

Kind regards

[eXpl0it3r]

Provide a minimal and complete example otherwise we can just make random guesses.

[Ixrec]

Since you're asking us to help find a bug in your program, there's really nothing we can do without a complete and minimal example.

The only thing I can do is respond to this comment:

I know that Singletons are bad but no idea how to do it otherwise

This is exactly why singletons/globals are bad: They let you "get away" with not figuring out the real solution to your problem.

Of course, without more details I can't tell you what the correct solution is, but most of the time it's fine to pass all of a class's dependencies to it in the constructor.  In this case, I would think that whoever is constructing Entities can talk to the resource manager itself, so the Entity simply receives a Sprite and deals with that.

[Flyverse]

Sorry, I thought it would be easier to understand the problem if only some simple pseudo-code was posted. Here is the whole code (Omitted unimportant parts like Header-Guards, includes, etc):

Assets.h

Code: [Select]

class Assets {
public:
static Assets& instance();
static const unsigned short int
SPRITE__BALL = 0;
ResourceManager<sf::Sprite> * spriteManager;
protected:
Assets();
static Assets * _instance;

};
Assets.cpp

Code: [Select]

Assets* Assets::_instance = NULL;
Assets::Assets(){
spriteManager = new ResourceManager<sf::Sprite>(new Loader<sf::Sprite>());
}
Assets& Assets::instance(){
if (_instance == NULL) _instance = new Assets();
return *_instance;
}
ResourceManager.h

Code: [Select]

template<class Res> class ResourceManager {
protected:
std::map<int, Res*> resourceList;
Loader<Res> * loader;
public:
~ResourceManager();
ResourceManager(Loader<Res> * loader);
void load(int key, std::string filePath);
Res* request(int key);
void unload(int key);
void dispose();
};
template<class Res> inline ResourceManager<Res>::~ResourceManager(){
this->dispose();
}

template<class Res> inline ResourceManager<Res>::ResourceManager(Loader<Res> * loader){
this->loader = loader;
}

template<class Res> inline void ResourceManager<Res>::load(int key, std::string filePath){
unload(key);
this->resourceList[key] = this->loader->loadFromFile(filePath);
}

template<class Res> inline Res* ResourceManager<Res>::request(int key){
assert(resourceList.find(key) != resourceList.end() && "Resource could not be found. Make sure to load it before requesting it.");
return this->resourceList[key];
}

template<class Res> inline void ResourceManager<Res>::unload(int key){
if (this->resourceList.find(key) != this->resourceList.end()){
delete this->resourceList[key];
this->resourceList.erase(key);
}
}

template<class Res> inline void ResourceManager<Res>::dispose(){
delete loader;
for (std::pair<int, Res*> kv : resourceList){
delete kv.second;
}
resourceList.clear();
}
Loader.h & Loader.cpp

Code: [Select]

template<class T> class Loader {
public:
T* loadFromFile(const std::string& filePath);
~Loader() {}
};
template<>
sf::Texture* Loader<sf::Texture>::loadFromFile(const std::string& path) {
sf::Texture* texture = new sf::Texture();
texture->loadFromFile(path);
return texture;
}

template<>
sf::Sprite* Loader<sf::Sprite>::loadFromFile(const std::string& path){
sf::Texture* texture = new sf::Texture;
texture->loadFromFile(path);
return new sf::Sprite(*texture);
}

And finally the problematic class, "BallEntity":

Code: [Select]

class BallEntity : public Entity {
public:
BallEntity(World * world, sf::RenderTarget* target, int radius);
        ... //Unnecessary functions omitted, fE getType(), update(),
void render();
private:
int radius;
sf::Sprite * ballSprite;
};
BallEntity::BallEntity(World * world, sf::RenderTarget* target, int radius) : Entity(world, target) {
this->radius = radius;
this->ballSprite = Assets::instance().spriteManager->request(Assets::SPRITE__BALL);
}

void BallEntity::render(){
ballSprite->setPosition(...);
ballSprite->setRotation(...);
target->draw(*ballSprite); // ACCESS VIOLATION!!!
}

I hope I did not forget any file, but I don't think so .

[Jesper Juhl]

For some examples of how to manage resources you could take a look at this stuff:

The Thor resources module (tutorial).
The Smart ResourceManager that can be found on the wiki.
You can also take a look at how my own Jukebox class manages its music resources.

[Ixrec]

Sorry, I thought it would be easier to understand the problem if only some simple pseudo-code was posted

That's why it has to be minimal.  See http://sscce.org/.  Reduce the code you actually have down to a single file that still reproduces the problem, but contains absolutely no code that isn't needed to reproduce it.  Then, we can actually help.

Usually trying to reduce the code helps you find the bug on your own, and anyone who claims they can't easily reduce it probably has such unmaintainable code that the current bug is the least of their worries.

Also, use a debugger.  If the null pointer thing happens inside draw(), but not in the dereferences you have directly above the draw(), the problem clearly is not what you think it is (which is why an SSCCE is so important).

Finally, resource managers are hard.  Jesper's right to suggest that you use an existing one instead.

[eXpl0it3r]

One of the main problem with your code are this function:

template<>
sf::Sprite* Loader<sf::Sprite>::loadFromFile(const std::string& path){
        sf::Texture* texture = new sf::Texture;
        texture->loadFromFile(path);
        return new sf::Sprite(*texture);
}

You're create a texture on the heap, but then throw away the pointer to the texture. Nobody now really owns the texture object and thus the lifetime is not very clear. Instead of manual memory management you should really use RAII with smart pointers (see this article).

Besides that you really need to be careful regarding copying of texture objects. A copy is an expensive operation for texture objects and the memory location will change, thus the references within the sprite breaks.

The problem I see here is less about the "scope", but more about lifetime and ownership. Who owns which objects and how is their lifetime being managed?

[Flyverse]

@Jesper Juhl
Going to take a look at it, thanks.

@Ixrec
Oh, okay. If the problem isn't solved (I think I have a hunch of how I might solve it right now), I'll post that version of my code here =)

@eXpl0it3r
Actually, I thought that the pointer WILL persist, since it IS a pointer and you have to manage its lifetime manually: Thats why I choose a pointer for the "Texture", and not only a Texture-Object itself. And I don't really think that the problem lies in that method, since the Texture/Sprite still exists when I request it from my BallEntity class (= Doesn't get destroyed after loading), but it just somehow gets destroyed before I can render it. It's weird, in my opinion. But after all, I'm new to C++... I'll take a look at RAII but I don't really like it since the syntax just looks so.. complicated, for something that should make your life easier. I dunno, it's just a feeling I get.

Besides...

May I ask where do I copy it? I mean, I thought that what I was doing was exactly the contrary, not copying it, since I work with pointers instead of the object itself?

Man, I really hope I'll understand this C++ stuff with pointers sometimes

[Flyverse]

Okay, so this is really weird.

When creating my BallEntity Object directly in the main-function and rendering it, everything works fine: So it isn't even a problem of my *ballSprite Variable or ResourceManager - But when it is created by my world class (Which just instantiates it and puts it in an array that is looped through to call the render function of every entity), I get the access violation error... Going to post the code soon, just have to do that minimal example.

[Flyverse]

Okay, it's 100% sure now that the problem isn't my Loader/ResourceManager - But I still can't locate the origin of it. I tried making a minimal version of my code - And suddenly, it works. Weird is, that I just removed everything that hasn't to do with the rendering; And I don't really believe that those removed functions and members could influence that ballSprite Pointer. I just don't get it.

Here is my minimal example:

Code: [Select]

class Entity{
    public:
        Entity(sf::RenderTarget*target);
        virtual ~Entity() = 0;
        virtual void render()=0;
    protected:
        sf::RenderTarget*target;
};
Entity::Entity(sf::RenderTarget*target){
    this->target = target;
}
inline Entity::~Entity() { }

class BEntity : public Entity {
public:
BEntity(sf::RenderTarget* target);
~BEntity();
void render();
private:
sf::Sprite * ballSprite;
};
BEntity::BEntity(sf::RenderTarget* target) : Entity(target) {
this->ballSprite = Assets::instance().spriteManager->request(Assets::SPRITE__BALL);
this->target = target;
}
BEntity::~BEntity(){}
void BEntity::render(){
ballSprite->setPosition(200, 200);
ballSprite->setRotation(40);
target->draw(*ballSprite);
}
class World{
private:
std::vector<BEntity*> entities;
sf::RenderTarget * target;
public:
World(sf::RenderTarget * target);
void createBall();
void render();
};
World::World(sf::RenderTarget * target){
this->target = target;
}
void World::render(){
for (BEntity* e : entities){
e->render();
}
}
void World::createBall(){
entities.push_back(new BEntity(this->target));
}

int main()
{
Assets::instance().spriteManager->load(Assets::SPRITE__BALL, "ball.png");
World world(&window);
world.createBall();
sf::Clock clock;
while (window.isOpen()){
sf::Event event;
while (window.pollEvent(event)){
if (event.type == sf::Event::Closed)
window.close();
}
sf::Time delta = clock.restart();
window.clear();
world.render();
window.display();
}
return 0;
}

And then there is my longer example (Didn't omit anything but the header guards)

Entity .h&.cpp

Code: [Select]

class World;
class Entity {
public:
Entity(World * world, sf::RenderTarget* target);
virtual ~Entity() = 0;
Body* getPhysicsBody();
World* getWorld();
virtual Body* createBody() = 0;
virtual void render() = 0;
virtual void update(float delta) = 0;
virtual unsigned short int getType() = 0;
protected:
sf::RenderTarget* target;
Body * body;
World * parent;
};
inline Entity::~Entity() { }

Entity::Entity(World * world, sf::RenderTarget* target){
this->target = target;
this->parent = world;
}
Body * Entity::getPhysicsBody(){
return body;
}
World * Entity::getWorld(){
return parent;
}
BallEntity .h&.cpp

Code: [Select]

class BallEntity : public Entity {
public:
BallEntity(World * world, sf::RenderTarget* target, int radius);
~BallEntity();
Body* createBody();
void render();
void update(float delta);
unsigned short int getType();
private:
int radius;
sf::Sprite * ballSprite;
};

BallEntity::BallEntity(World * world, sf::RenderTarget* target, int radius) : Entity(world, target) {
this->radius = radius;
this->ballSprite = Assets::instance().spriteManager->request(Assets::SPRITE__BALL);
this->body = createBody();
}
BallEntity::~BallEntity(){}
void BallEntity::update(float delta){}
void BallEntity::render(){
ballSprite->setPosition(200, 200);
ballSprite->setRotation(40);
target->draw(*ballSprite); //VIOLATION ERROR
}
Body * BallEntity::createBody(){
b2BodyDef bodyDef;
bodyDef.type = b2_dynamicBody;
bodyDef.position.Set(0, 0);
b2Body* b2Body = getWorld()->physicsWorld.CreateBody(&bodyDef);
b2CircleShape dynamicBox;
dynamicBox.m_radius = 0.5f;
dynamicBox.m_p.Set(0, 0);
b2FixtureDef fixtureDef;
fixtureDef.shape = &dynamicBox;
fixtureDef.density = 1.0f;
fixtureDef.friction = 0.3f;
b2Body->CreateFixture(&fixtureDef);
Body * body = new Body_Box2d(this, b2Body, &getWorld()->physicsWorld);
return body;
}
unsigned short int BallEntity::getType(){
return 0;
}

World .h&.cpp

Code: [Select]

class Entity;
class World : public b2ContactListener {
private:
std::vector<Entity*> entities;
sf::RenderTarget * target;
public:
b2World physicsWorld;
World();
void deleteEntity(Entity * entityToRemove);
void createBall();
void moveEntity(float delta, sf::Vector2f velocity, Entity * entity);
void update(sf::Time delta);
void render();
void BeginContact(b2Contact* contact);
void EndContact(b2Contact* contact);
};

World::World() : physicsWorld(b2Vec2(0, 10)){
physicsWorld.SetContactListener(this);
this->entities.push_back(new WallEntity(this, 2, Assets::RESOLUTION_H*Assets::PIXELS_TO_METERS, 0, 0));
this->entities.push_back(new WallEntity(this, 2, Assets::RESOLUTION_H*Assets::PIXELS_TO_METERS, Assets::RESOLUTION_W*Assets::PIXELS_TO_METERS, 0));
}
void World::deleteEntity(Entity * entityToRemove){
entityToRemove->getPhysicsBody()->remove();
this->entities.erase(std::remove(entities.begin(), entities.end(), entityToRemove));
}
void World::createBall(){
BallEntity * be = new BallEntity(this, this->target, 0.5f);
this->entities.push_back(be);
be->getPhysicsBody()->setPositionY(0)->setPositionX(fmod(rand(), Assets::RESOLUTION_W*Assets::PIXELS_TO_METERS));
//TODO: APPLY FORCE
}
void World::moveEntity(float delta, sf::Vector2f velocity, Entity * entity){}
void World::render(){
for(Entity* e : entities){
e->render();
}
}
void World::update(sf::Time delta){
this->physicsWorld.Step(delta.asSeconds(), 8, 6);
std::vector<Entity*>::iterator iterator = entities.begin();
while (iterator != entities.end()){
if ((*iterator)->getPhysicsBody()->getPositionY() < Assets::RESOLUTION_H*Assets::PIXELS_TO_METERS){
delete *iterator;
iterator = entities.erase(iterator);
//TODO: SEND ENTITY DESTROY EVENT
(*iterator)->getPhysicsBody()->remove();
}else{
++iterator;
}
}

}
void World::BeginContact(b2Contact * contact){
Entity* e1 = (Entity*)contact->GetFixtureA()->GetBody()->GetUserData();
Entity* e2 = (Entity*)contact->GetFixtureB()->GetBody()->GetUserData();
if(e1->getType() == 0 && e2->getType() == 0){
//TODO: SEND BALL COLLIDE EVENT
}
if((e1->getType() == 0 && e2->getType() == 2) || e1->getType() == 2 && e2->getType() == 0){
//TODO:: SEND BALL COLLIDE PLATFORM EVENT
}
}
void World::EndContact(b2Contact * contact){

}

main()

Code: [Select]

World gameWorld;
int main()
{
Assets::instance().spriteManager->load(Assets::SPRITE__BALL, "ball.png");
gameWorld.createBall();

sf::Clock clock;
while (window.isOpen()){
sf::Event event;
while (window.pollEvent(event)){
if (event.type == sf::Event::Closed)
window.close();
}

sf::Time delta = clock.restart();

gameWorld.update(delta);

window.clear();
gameWorld.render();
window.display();
}
return 0;
}
Minimal Example: Everything works fine.
"Real code": Violation Error.

I don't get it

[Flyverse]

Okay, solved it. I'm so stupid. I thought the whole time that ballSprite was the problem, never even thinking about me forgetting to pass an actual sf::RenderTarget to the Entity. Great.

Thanks for all of your help =)