Assigning Texture via For Loop Not Working

[BlazeTide]

For the past week I've been trying to assign specific textures to sprites in a vector array via a for loop, with both elements being called from a XML file using PugiXML. With my current code, every sprite except for the last one within the vector array appears to have a white texture incorrectly assigned to it, with only the last one within the vector array having the correct texture assigned to it.

So far, I don't see any flaws within my code, which is confusing me. If anyone happens to see an error that I don't, a reply would be greatly appreciated.

initCollisionObject Function:

void CollisionObject::initCollisionObject(std::string fileName, b2World &_world, float objectX, float objectY, float objectRotation)
{
        collisionObjectString = fileName;
        collisionObjectImage.loadFromFile(collisionObjectString);
        collisionObjectTexture.loadFromImage(collisionObjectImage);
        collisionObjectSprite.setTexture(collisionObjectTexture);
        width = collisionObjectSprite.getGlobalBounds().width;
        height = collisionObjectSprite.getGlobalBounds().height;
        x = objectX;
        y = objectY;
        rotation = objectRotation;
        collisionObjectSprite.setOrigin(collisionObjectSprite.getGlobalBounds().width / 2, collisionObjectSprite.getGlobalBounds().height / 2);
        collisionObjectSprite.setPosition(x, y);
        collisionObjectSprite.setRotation(rotation);

        createStaticCollision(_world, x, y, width, height, rotation);

        return;
}

for Loop:

for (xml_node collisions = doc.child("level").child("collisions").child("object"); collisions; collisions = collisions.next_sibling("object"))
{
        //cout << "Iterated" << endl;
        xml_attribute textureAttribute = collisions.attribute("spritesheet");
        xml_attribute xPosAttribute = collisions.attribute("x");
        xml_attribute yPosAttribute = collisions.attribute("y");
        xml_attribute rotationAttribute = collisions.attribute("rotation");
       
        string texture = textureAttribute.as_string();
        int xPos = xPosAttribute.as_int();
        int yPos = yPosAttribute.as_int();
        int rotation = rotationAttribute.as_int();
        collisionArray.push_back(collisionObject);
        collisionArray[currentCollisionObjectIndex].initCollisionObject(texture, _world, xPos, yPos, rotation);

        currentCollisionObjectIndex++;
}

[BlazeTide]

Edit: Fixed a small syntax error accidentally copied over from the code, but the program still stands as it is and is still not working.

[BlazeTide]

I should also point out that if I initialize every element of the vector array individually outside of the for loop, everything appears as it should. However, as soon as I initialize them via the for loop, every sprite except for the last one in the vector array has their texture rendered null.

[Mario]

Are you sure whatever you've got in your collisionArray is correct?

My suspect right now would be some shallow copy being made of a class (CollisionObject) using pointers due to you missing a copy constructor.

That would also explain why only your last instance is working as intended.

How is your collisionArray defined?

[BlazeTide]

Correct, collisionArray is defined using a CollisionObject class, though it doesn't use pointers as far as I can tell. The only function in that class as of now is the one I posted in the original post, initCollisionObject. The constructor of that class defines only the width, height, x, and y positions of the sprite used in that class.

How collisionArray is defined in the header file:

#ifndef Engine_h
#define Engine_h

#include <iostream>
#include <SFML\Graphics.hpp>
#include <Box2D\Box2D.h>
#include "pugixml.hpp"
#include "Collision.h"
#include "CollisionObject.h"
#include "Player.h"
#include "Enemy.h"
#include "Lighting.h"

using namespace std;
using namespace sf;
using namespace pugi;

class Engine
{
public:
        Engine();
        ~Engine();
        void init(sf::RenderWindow &_window, b2World &_world);
        void run(sf::RenderWindow &_window, b2World &_world);
        void keyEvents(sf::RenderWindow &_window);
        void physics(sf::RenderWindow &_window, sf::RenderTexture& _texture, b2World &_world);
        void rayCast(sf::RenderWindow& _window, b2World* _world);

private:
        Collision collision;
        CollisionObject collisionObject;
        Player player;
        Enemy enemy;
        Lighting lighting;

        vector<CollisionObject> collisionArray;
        int currentCollisionObjectIndex;

        int cameraX;
        int cameraY;

        float closestFraction;
        b2Vec2 p1;
        b2Vec2 p2;
        b2Vec2 intersectionPoint;

        xml_document doc;
};

#endif

[BlazeTide]

I'm starting to get the vibe that this may be an error inside SFML 2.0. Maybe one of the admins should take a look at the source code of the graphics library?

[Nexus]

I'm starting to get the vibe that this may be an error inside SFML 2.0.

Before blaming SFML, prove that the error is not in your code, by writing a minimal and complete example with only SFML code and no custom functionality.

[Laurent]

When the collisionArray vector grows, objects are moved in memory, they get new adresses. Which means that all the sprites that were previously pointing to a texture, are now pointing to an invalid address (the texture is no longer there). Therefore, your CollisionObject class should have a copy constructor which correctly assigns the new texture (the copied one) to the copy of the sprite -- otherwise the new sprite will still have a pointer to the old texture which doesn't exist anymore.

[BlazeTide]

I'm starting to get the vibe that this may be an error inside SFML 2.0.

Before blaming SFML, prove that the error is not in your code, by writing a minimal and complete example with only SFML code and no custom functionality.

Sorry, I'd forgotten how to post on these forums ever since my first post requesting for help.

I've attached all the files that are required, including images, to replicate the error in a .rar file. All you need to do is copy it into a main project directory and run main.cpp. I've compressed the project to its bare minimum so that it only replicates the error.

[Laurent]

Sorry, I'd forgotten how to post on these forums ever since my first post requesting for help.

I've attached all the files that are required, including images, to replicate the error in a .rar file. All you need to do is copy it into a main project directory and run main.cpp. I've compressed the project to its bare minimum so that it only replicates the error.

Before doing that you should have read my answer above, which gives the solution.

[BlazeTide]

When the collisionArray vector grows, objects are moved in memory, they get new adresses. Which means that all the sprites that were previously pointing to a texture, are now pointing to an invalid address (the texture is no longer there). Therefore, your CollisionObject class should have a copy constructor which correctly assigns the new texture (the copied one) to the copy of the sprite -- otherwise the new sprite will still have a pointer to the old texture which doesn't exist anymore.

Whoops, didn't see your post there Laurent. I don't know/remember how to create copy constructors, so I might as well look up a tutorial to see if I can fix the error.

Thanks for the reply!

[BlazeTide]

Sorry for restarting this topic, but I don't think I quite understand how copy constructors work in this type of scenario. Applying one automatically returns a runtime error at the point where the sprites are drawn. I know I'm most likely missing something, but I don't know what it is. Otherwise, I probably just have the entire concept wrong at this point.

Header file:

CollisionObject(const CollisionObject &_collisionObject);
CollisionObject &operator= (const CollisionObject &_collisionObject);

C++ file:

CollisionObject::CollisionObject(const CollisionObject &_collisionObject)
{
        collisionObjectString = _collisionObject.collisionObjectString;
        cout << "True" << endl;
}

CollisionObject &CollisionObject::operator= (const CollisionObject &_collisionObject)
{
        if (this == &_collisionObject)
                return *this;

        collisionObjectString = _collisionObject.collisionObjectString;

        return *this;
}

[Laurent]

It's hard to show you how to write a copy constructor without seeing the definition of the class (at least its members)

[Nexus]

For the copy assignment operator, consider the copy-and-swap idiom.

The need for if (this == other) is almost always a design problem.

[BlazeTide]

Thanks for all the help, but I've managed to solve this problem using another method. I created a separate loop to define and initialize the collisionArray size.

If any of you want to elaborate more on the copy constructor error I was having, the original post has all of the code definition for the class, and each individual sprite is drawn using the .draw() function in another for loop which takes the size of collisionArray as one of its parameters within the main loop.

Again, thank you all so much for the insight and help!