Attaching image to a sprite

[sammy]

i'm sure that i just mad a very stupid mistake but i can't help myself
here's my code

Code: [Select]


void CDrawable::SetResource(std::string strID)
{
m_strResourceID = strID;

CResourceBMP* tempRes;
tempRes = static_cast<CResourceBMP* > (RESOURCEMANAGER->GetResource(m_strResourceID));
m_pSprite.SetImage(tempRes->GetImage());
delete tempRes;
tempRes = NULL;
}
//------------------------------------------------------
sf::Image CResourceBMP::GetImage()
{
return m_pImage;
}


The GetImage() method hands over a reference to its image, but m_pSprite just contains a NULL-pointer as image after calling SetImage() so that the programm just draws white rectangles or i get an acces violating.
the RESOURCEMANAGER stores the resources in a map, but its already completed when setting the resources to the objects. so it can't be an error cause of moving the image in memory because the vector/map expands.

i don't see the forrest because of all the trees, as you say here in Germany;)

[downtimes]

I think i have an idea what is causing this Problem.

The function tempRes->GetImage() returns a Copy of your image to the
call of SetImage(). Because of the fact that Sprites only store a Resource pointer and that the tempory Object from tempRes->GetImage() does not get stored anywhere it gets deleted instantly after calling this function.

Thats the Reason the Sprites image is NULL.

Hope you understood what i meant -.-'

[Nexus]

This code is quite user-unfriendly and overly complicated. I don't wonder that you lose track of it

Code: [Select]

static_cast<CResourceBMP* > (RESOURCEMANAGER->GetResource(m_strResourceID));Can't the resource manager return the correct type? Does it return a pointer to base class? This is not generally bad, but you have to be careful, since static_cast doesn't perform a runtime check. So, if the pointer type is wrong, you get undefined behaviour.

The other problem of this method is that it returns a pointer to memory of which the caller is responsible. Maybe you should use an RAII alternative that automatically frees the owned memory (std::auto_ptr), or at least call the method "Create..." or similar.

Code: [Select]

delete tempRes;Here you destroy the object. I guess that the image is invalidated, so you can't access it later.

Code: [Select]

tempRes = NULL;This is completely useless, since the pointer goes directly out of scope anyway.

[sammy]

hmm, that were my solutions, too. but when executing the code line-by-line in visual studio, the GetImage-function is called properly and the right image is returned. but after calling the SetImage-method and befor the tempRes is deleted, the sprite contains no image. so as i concluded, the deleting of the tempRes can't cause this problem. or am i wrong?

@ nexus
yes your right, there is a lot of redundance in my code, i just have to simplify it... :roll:
the manager returns the base class, but it would be better to overload the GetRes-function

[Groogy]

Like already been said, you return a copy of the image allocated on the stack, not the heap, and when it is destroyed the sprite is changed to point to NULL.