How to release memory when deleting a player class instance?

[ingwik]

This is a problem connected to C++, I think, but I can't find a proper solution for it. Best I've found this far is from here:
http://www.cprogramming.com/tutorial/dynamic_memory_allocation.html
but it doesn't work for me.

I have a class named panzergrenade. Whenever a tank in the game shoot a grenade, I create a new grenade, put it in an array and have it live in the game until it hits something

When it hit, I make the grenade invisible and create another class instance which is an explosion, added to another array, at the very same place.

When the explosion start, I want to delete the panzergrenade to make room in the array for new ones and to retain memory, same for the explosion when it has finished exploding, but I get a debug error doing that.

Code: [Select]

class Panzergrenade
{
public :
Panzergrenade(double  out_velocity, double out_angel, bool out_is_drawable); //construction
~Panzergrenade(){};
double target_x;
double target_y;
double velocity;
double angel;
bool is_drawable;
sf::Sprite Sprite; // sprite for the grenade
sf::Image Image; //imagefile, just an image and no spritesheet
};
Panzergrenade::Panzergrenade(double  out_velocity, double out_angel, bool out_is_drawable);
{
if (!Image.LoadFromFile("grenade.png")) //load picture    
  {        
  std::cout << "Can't find picture: grenade.png " << std::endl;  
  }
velocity=out_velocity;
angel=out_angel;
is_drawable = out_is_drawable;
Sprite.SetRotation(angel);//Give the same angel as the tank when creatde
Sprite.SetImage(Image); //Give correct picture to the class
}


//Array to save the grenades in, no problem there

Code: [Select]

     
       Panzergrenade *grenadelist[100];//max 100 grenades in game
       int grenade_i=0; //0-100 shows where we are in the list
       int si =0; //counter 0-100 when checking items in list


And then, at creation:

Code: [Select]


grenadelist[grenade_i] =  new Panzergrenade(1.0f,tank1.sprite.GetRotation(), true);
grenade_i++;

//velocity / what tank the rotation is copied from / if it should be drawn
 
What I do right now, when a grenade hit, is that I just set the velocity to 0 and put is_drawable=false.
The grenade exists but it isn't shown anymore. An explosion is created on top of the invisible grenade and is also set to invisible after the explosion is finished.

This works fine for 100 grenades, the problem start to come if I want to have more than 100 grenades.
If I reset grenade_i to 0 and try to add a new grenade on the occupide place, no grenade is created at all. No error messages, nothing.
I assume this could create a horrible memory leak if it actually worked though, since the first created grenade would loose it's pointer adress and  become impossible to find for deletion.

If I instead of making the grenade invisible tries to delete it:

Code: [Select]

delete [] grenadelist[si];
The program compiles but when the actual deletion is about take place, the program crasches with:

Code: [Select]


Debug Assertion Failed!
Expression:_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)
(I've also tried:

Code: [Select]

delete [si] grenadelist;to no avail, it crasches too).

If I try to delete the whole list when closing down the program with:

Code: [Select]

delete [] grenadelist;
I get another error:

Code: [Select]


Debug Assertion Failed!
Expression:_CrtIsValidHeapPointer(pUserData)

So the questions are:

*How am I supposed to delete an item in an array to free the memory and make it possible to save a new class instance in it's place?
*How do I delete a full list of class instances put in an array when the program ends to release memory in a nice way?
*If I fill an array, how do I do if I want to reuse a place in the array for a new class instance? (starting from 0 again)

[Nexus]

Arrays are the wrong approach. In fact, they're almost always, at least std::array outperformes them anyway.

Use dynamic containers like std::vector. They provide useful methods to insert and remove elements, and they also take care of memory management.

Code: [Select]

std::vector<Panzergrenade> grenadelist;
grenadelist.push_back( Panzergrenade(30.f, 19.f, true) );
...
By the way, you should make your attributes private, use the constructor initializer list and a uniform naming convention

And in your linked article, I have only read "The C function for memory deallocation is free() and has the same behavior as delete", and that's already wrong. I were careful with this article...

[ingwik]

Thanks Nexus, I appreciate your help here.

So, to get it stright, I've looked up vector and it seems good:
http://www.cplusplus.com/reference/stl/vector/

Three question, if you or someone else knows:
-What makes vector better than list or deque when it comes to store tankgrenade class instances?
-Is it possible to delete certain class instance in a vector? (since it seems to be impossible with an array).
-Is it possible to delete a complete vector with all its class instances when the program ends so the memory can be returned (since it seems to be impossible with an array).

By the way, you should make your attributes private, use the constructor initializer list and a uniform naming convention

It's public just too keep down the amount of errors, making debugging easier.

What do you mean with "use the constructor initializer"? I 've read about it but never understood why it's a better way.

Uniform naming convention? Like this? http://geosoft.no/development/cppstyle.html#Naming Conventions I'm trying to understand C++ right now so naming convention is not on the priority list, but you're right. I'm a bit sloppy with it.

[Nexus]

-What makes vector better than list or deque when it comes to store tankgrenade class instances?

vector is somehow the default container, it uses a dynamic array (like new[] and delete[] do, but automatically). For special requirements, you can use the other containers. For example, std::deque can insert/remove elements fast at beginning and the end. std::list is a doubly-linked list, so it has no random access, but fast insertion and removal of any element.

Is it possible to delete certain class instance in a vector? (since it seems to be impossible with an array).

Yes, lookup the erase() method.

Is it possible to delete a complete vector with all its class instances when the program ends so the memory can be returned (since it seems to be impossible with an array).

This is done automatically in the destructor. If you need to remove all elements in order to reuse the vector, you can call clear().

[ingwik]

I have changed all code now so it use vectors instead of arrays. It works about the same, but the graphic is just white squares now. Both the pictures for the grenades and the pictures for the explosions.  I can't figure out why.

The class declaration is the same.

The grenade vector:

Code: [Select]

std::vector<Panzergrenade> vGrenadelist;

Creation:

Code: [Select]

vGrenadelist.push_back( Panzergrenade(1.0f, Tank1.sprite.GetRotation(), true) );

Positioning it below the tank

Code: [Select]

vGrenadelist.at(grenade_i).Sprite.SetPosition(Tank1.sprite.GetPosition().x,Tank1.sprite.GetPosition().y);

I won't get an error message showing that the program is unable to load the graphic, I just get a white square instead of the grenade.

[Nexus]

You must make sure that the sf::Image referenced by the sf::Sprite remains alive. It is better not to store the sf::Image inside the grenade class. Create a single image and share it for multiple sprites.

[ingwik]

You must make sure that the sf::Image referenced by the sf::Sprite remains alive. It is better not to store the sf::Image inside the grenade class. Create a single image and share it for multiple sprites.

I always thought keeping image and sprite inside the same class was a sure way to keep them connected and avoiding wite squares (*sighs and walks back to the drawing table*).

[Nexus]

keeping image and sprite inside the same class

...is not the intention behind the design. sf::Image and sf::Sprite have been separated so that multiple graphical entities (sprites) using the same image are able to share it, and not every sprite must keep its own copy of the pixels.

Just create a sf::Image outside the class and pass a pointer/reference to the grenades, which you use to initialize the sprite.

[ingwik]

Everything works now. But I'm still puzzled by some facts:

I forgot to add #include <vector> but everything works anyway.

I don't send the image to the class instance by pointer or reference, just:

Code: [Select]

sf::Image Image;
if (!Image.LoadFromFile("grenade.png")) //load picture            
  std::cout << "Can't find picture: grenade.png " << std::endl;  And at creation:

Code: [Select]

vGrenadelist.push_back( Panzergrenade(1.0f,Tank1.sprite.GetRotation(), true) );
vGrenadelist.at(grenade_i).Sprite.SetImage(Image);
grenade_i++;

The picture works just fine.

[Nexus]

The forgotten #include may work, if another header includes it. But don't rely on this behavior, always include the headers you need.

And I already told you to encapsulate your data. In the long term, the public sprite may lead to problems: Bugs are harder to track because you cannot control access, your implementation is fixed and cannot be altered without breaking code that uses it, ...

By the way, std::vector has a back() function.