how to pop/erase/delete a vectorStuff * ?

Started by
30 comments, last by Zakwayda 13 years, 9 months ago
Hello all,

I'm trying to figure out a way to release the memory from the Entities (as I call anything that moves in my game) that I won't use anymore.

-warning: bad design coming! some tips?-

As an example, my Player class has Bullets. When there is a Fire() event, I check if the ship is allowed to fire; if it is, a new bullet is put to the entities list and the game flux goes on merry on its gammy way.

The problem is that I'm not releasing the resources used by bullets that are already dead, i.e. out of screen or collided. Sometime soon it will bite me back.

So I tried to release it by popping the dead bullets out of the entities list using this code:

for ( std::vector<Entities*> iter = entities.begin();			iter !=  entities.size(); 			++iter) {			if( (**iter).checkIfDead() ) {				(*iter).pop_front;			}		}	


I will call it a Pseudo code. After the "if" part I dunno what to do. I want to call each entities "checkIfDead()" function and them remove it from the container. I think it would be simple if it was a vector<Stuff> (not a vector of pointers), but I really need to use a vector of pointers.

hear from you!

Fabio

Advertisement
You need to be careful when removing elements from a standard container, read the documentation to see when iterators are invalidated. std::vector<>::iterators are prone to being invalidated on any operation that affects the layout of the vector. But what you are looking for is "delete".

A standard erase loop looks like this:
std::vector<Entities*> iter = entities.begin();while(iter != entities.end()){    Entity *entity = *iter;    if(entity->checkIfDead())    {        iter = entities.erase(iter);        delete entity; // assuming all entites are allocated with "new"    }    else    {        ++iter;    }}	

Because you are using raw pointers you need to be careful not to store pointers elsewhere, because delete will cause any other pointers sharing the same value to become invalid.
I think this is right.
vector<Entities*> iter = entities.begin();while( iter != entities.end() ){   if( (*iter)->checkIfDead() )   {      delete *iter;      iter = entities.erase(iter);   } else iter++;}


Ninja'd [smile]

Please don't PM me with questions. Post them in the forums for everyone's benefit, and I can embarrass myself publicly.

You don't forget how to play when you grow old; you grow old when you forget how to play.

This is where you would use smart pointers, to avoid these kinds of things.
Edge cases will show your design flaws in your code!
Visit my site
Visit my FaceBook
Visit my github
Quote:Original post by Concentrate
This is where you would use smart pointers, to avoid these kinds of things.


You guys rule!
Later on I will try a solution using smart pointers. Actually, it was my first thought, but I decided to go the "usual" way.

Rip-off & Buckeye, thank you very much! I tried a solution similar to yours, but I forgot to let
iter = entities.erase(iter); 


actually, I didn't even know that erase returned an address!

Quote:Original post by draconar
Quote:Original post by Concentrate
This is where you would use smart pointers, to avoid these kinds of things.


You guys rule!
Later on I will try a solution using smart pointers. Actually, it was my first thought, but I decided to go the "usual" way.

Rip-off & Buckeye, thank you very much! I tried a solution similar to yours, but I forgot to let
iter = entities.erase(iter); 


actually, I didn't even know that erase returned an address!


Technically it doesn't, it returns an iterator to the object after the erased element but considering an iterator is basically a pointer then technically it does return an address.

Since you are deleting through the container rather than just off the end I suggest you use a list instead of a vector as it will be a lot more efficient with larger numbers of enemies.
Quote:Actually, it was my first thought, but I decided to go the "usual" way.
As far as modern C++ goes, using smart pointers is the 'usual' way.

That said, a lot of C++ coders still manage their memory 'by hand', either to meet certain technical requirements, or just because it's what they're used to. For new code though, best practice (arguably) is to use smart pointers (or some other RAII container) for this sort of thing unless you have a good reason to do otherwise.
Would not the following be more appropriate?

bool PredicateFunction (const Entities* e) { return e->checkIfDead(); }void DeleteFunction (Entities* e) { delete e; }std::vector<Entities*> iterator = std::remove_if(entities.begin(),entities.end(),PredicateFunction);std::for_each(iterator,entities.end(),DeleteFunction);entities.erase(iterator,entities.end());


That way he could still use a vector and not have to worry about invalidating the iterators/deleting items in the middle of a vector (and forcing them all to be copied). The delete could be done at the very end (after say, more items are removed due to other game logic). Not to mention you could add/remove items every frame and have very few memory allocations (as once the vector hit its working capacity, it'd stay there and there'd be no more allocations. A list on the other hand would have allocations every frame.
Quote:Original post by Ryan_001
Would not the following be more appropriate?

std::remove_if() doesn't partition. That is to say the removed elements are not guaranteed to be placed after the returned iterator. If you want to partition use the partition() function.
Quote:Original post by Ryan_001
Would not the following be more appropriate?

bool PredicateFunction (const Entities* e) { return e->checkIfDead(); }void DeleteFunction (Entities* e) { delete e; }std::vector<Entities*> iterator = std::remove_if(entities.begin(),entities.end(),PredicateFunction);std::for_each(iterator,entities.end(),DeleteFunction);entities.erase(iterator,entities.end());


That way he could still use a vector and not have to worry about invalidating the iterators/deleting items in the middle of a vector (and forcing them all to be copied). The delete could be done at the very end (after say, more items are removed due to other game logic). Not to mention you could add/remove items every frame and have very few memory allocations (as once the vector hit its working capacity, it'd stay there and there'd be no more allocations. A list on the other hand would have allocations every frame.


Yea, I would agree that is the best solution to use, using remove_if and erase in combination had crossed my mind but I forgot it's an efficient solution, with respect to the Vector, it's plausible for it to never require allocations if enough space to store all enemies is allocated at it's construction, but I guess that's down to the game itself.

Quote:Original post by SiCrane
Quote:Original post by Ryan_001
Would not the following be more appropriate?

std::remove_if() doesn't partition. That is to say the removed elements are not guaranteed to be placed after the returned iterator. If you want to partition use the partition() function.


Forgot about that, it wouldn't have been a problem if the container wasn't pointers to Entities, or it was using some form of reference counting handle.

This topic is closed to new replies.

Advertisement