texture problem

[thunderlight]

Hi everyone I am new and I have a problem where one of the sprites(npc) being drawn is a different texture than I set it to. It is the same texture as another sprite. I am loading the texture and setting it in the constructor of the class. The texture shouldn't be destroyed since it is not local and I don't have the white square problem.

int main()
{
        TileMap t;   //contains each tile in a map and other info like coordinates.
        Player p;
        .....
}
Npc.class
#include "Npc.h"


Npc::Npc(int x, int y,int gid, std::string location)
{
        ......
        Npc::texture1.loadFromFile(Npc::location);
        Npc::sprite1.setTexture(texture1);
}
 

The tilemap gets its data from a file and has a vector of Npcs. Also, If I switch the tilemap and player lines around, the sprite being drawn becomes the one being declared last.

[MadMartin]

Hi there.
Unfortunately, the code you showed is to short to answer your question.
Please reduce your project to a minimal and complete example (i.e. an example that shows the problem). Minimal is crucial, nobody has the time to read through several pages of code.

During the preparation of a minimal example, one often has the chance to solve the problem 


By the way, your use of the namespace operator :: is way over the top. It is sufficient to use it in the method signature, but not in its body. Whats that with Npc::location? That is an attribute, not the parameter location you are referring to.

[The Terminator]

Why are you passing Npc::location to loadFromFile()? You're invoking a string contained within Npc, when you really want to pass the argument called "location". Remove the Npc:: and see if that solves your issue.

[thunderlight]

Sorry for not being clear but I was actually storing the parameter to an attribute.

Npc.cpp
Npc::Npc(int x, int y,int gid, std::string location)
{
        Npc::x = x;
        Npc::y = y;
        Npc::location = location;
        Npc::gid = gid;
        Npc::texture1.loadFromFile(Npc::location);
        Npc::sprite1.setTexture(texture1);
}
TileMap.cpp
 while(count >= 0)
                                                        {
                                                                if(atoi(attr->value()) >= locations[count].gid)
                                                                {
                                                                        Npc temp(j,i,atoi(attr->value()) -
                                                                                locations[count].gid, locations[count].location);
                                                                        Npcs.push_back(temp);
                                                                        break;
                                                                }
                                                                count--;
                                                        }
//I omitted alot of code but basically its just loops that take data from file. Then creating and storing each npc's position, image path etc inside the vector.
Main.cpp
int main()
{
        TileMap t;
        Player p;
        //inside the game loop draws the tiles
        for(int q = 0; q < t.Npcs.size(); q++)
                                {
                                       //t.Npcs[q].sprite1.setTexture(t.Npcs[q].texture1);
                                        t.Npcs[q].sprite1.setPosition(t.Npcs[q].getX()*32,t.Npcs[q].getY()*32);
                                        t.Npcs[q].sprite1.setTextureRect(IntRect(0,0,32,48));
                                        window.draw(t.Npcs[q].sprite1);
                                }
         p.sprite.setTextureRect(IntRect(p.source.x%4*p.width,p.source.y*p.height,p.width,p.height));
         window.draw(p.sprite);
         window.display();
//above cove: draws the npc to screen and then draw the character.
 

*EDIT: Although it works if I add that commented out line t.Npcs[q].sprite1.setTexture(t.Npcs[q].texture1). I am not sure if that is a good way to do this because it will need to set the texture for every iteration in the loop.

[MadMartin]

Why do you write the attribute access like

Npc::x = x;

instead of

this->x = x;

? The latter is much more readable and doesn't imply access to a static attribute. (Best would be different naming for parameters and members)
It is even incosistent, look at the setTexure() call in the Npc constructor 

Why do you repeat setting the texture rect if it stays at these magic numbers? Just do that in the constructor. And replace the for-loop with a for-range-loop.

The code is still too fragmented. Just try to reduce your code to a minimal full example.

[thunderlight]

Thanks, forgot about using

this->x=s;

I was messing around with the Npc constructor hoping it might change something but thanks for pointing it out.
And I believe I found out the problem (correct me if I am wrong please). Since I am using a vector and copying the temporary Npc, the vector's sprite is storing a pointer to the temporary Npc's texture instead of its own(the vector's texture). Therefore when the loop finishes, the temporary Npc is out of scope and the vector is no longer pointing to any valid texture. My temporary solution is to set the texture outside the loop or in another function which is working fine for now.

[MadMartin]

Just define a copy constructor for your Npc class where you update the texture. That would be a suitable solution.
(Just be aware of the Rule of Three/Five)

[thunderlight]

Thanks, I will give that a go