SFML community forums
Help => General => Topic started by: prchakal on December 03, 2010, 10:30:26 pm
-
Hi,
On my game i have a vector with all my enemies, and i want that when it collide with some other objects, be removed from vector and destroyed.
My vector:
static std::vector<Enemy*> *enemies;
What i have tested:
void Enemy::remove()
{
//GameObjects::enemies->erase(*this);
//delete this;
for (std::vector<Enemy*>::iterator it = GameObjects::enemies->begin(); it!=GameObjects::enemies->end(); ++it) {
if (((Enemy*)*it) == this)
{
GameObjects::enemies->erase(it);
}
}
}
But allways that i do it, i get a fatal error.
-
First solution:
void Enemy::remove()
{
GameObjects::enemies->erase(std::find(GameObjects::enemies.begin(), GameObjects::enemies.end(), this));
}
Second one:
void Enemy::remove()
{
for (std::vector<Enemy*>::iterator it = GameObjects::enemies->begin(); it != GameObjects::enemies->end(); ++it)
{
if (*it == this)
{
GameObjects::enemies->erase(it);
break;
}
}
}
-
You should remove your entity from GameObjects at end of loop. You can use a message for GameObjects as
void Enemy::remove()
{
GameObjects::atRemove(this);
}
....
void GameObjects::atRemove(Enemy *e)
{
listAtRemove.push_back(e);
}
//Call end of loop, to avoid calls of entity
void GameObjects::removeEnemys()
{
....//Remove the objects of the list in GameObjects::enemies
}
-
Hi,
I resolve the problem with:
for (long x = 0; x < (long)GameObjects::enemies->size(); ++x)
{
Enemy *enemy = GameObjects::enemies->at(x);
if (enemy == this)
{
GameObjects::enemies->erase(GameObjects::enemies->begin() + x);
}
}
When i erase an element from vector, it is all removed from the memory?
-
I resolve the problem with
Not the best one. You shouldn't ignore our replies ;)
When i erase an element from vector, it is all removed from the memory?
The pointer is removed from the vector, but the object is not destroyed, you still have to call delete.
-
You should really use boost::ptr_vector<Enemy> or std::vector< boost::shared_ptr<Enemy> > instead.
-
Hey laurent,
Sorry man, i dont see your posts.
Now i change my solution for yours, its simply perfect.
Thanks very much, it help a lot.
-
Hi,
The problem is that if im in the loop, and a method in this loop remove an element from the vector, the program crashes, an example:
for (std::vector<Enemy*>::iterator it = GameObjects::enemies->begin(); it!=GameObjects::enemies->end(); ++it) {
((Enemy*)*it)->update();
}
The update check the collision and call remove method:
if (Collision::PixelPerfectTest(sprite, GameObjects::planet->getSprite()))
{
GameObjects::enemies->erase(std::find(GameObjects::enemies->begin(), GameObjects::enemies->end(), this));
}
When ERASE method is called, the program crashes.
-
maybe something about vector::erase (http://www.dinkumware.com/manuals/?manual=compleat&page=vector.html#vector::erase)
Erasing N elements causes N destructor calls and an assignment for each of the elements between the insertion point and the end of the sequence. No reallocation occurs, so iterators and references become invalid only from the first element erased through the end of the sequence.
-
Hi,
What i do to prevent it, is make a loop on this vector objects without look it, an example:
for (long x = 0; x < (long)GameObjects::enemies->size(); ++x)
{
Enemy *enemy = GameObjects::enemies->at(x);
...
enemy->remove();
}
The remove method is:
GameObjects::enemies->erase(std::find(GameObjects::enemies->begin(), GameObjects::enemies->end(), this));
So, when i need loop vectors, i will use FOR without iterator, then i can remove without problems.
With this solution my problem is solved.
This is a good practice?
-
Yes, it seems good to me.
-
To me, it doesn't.
- Don't use at() when your indices are correct. In fact, I wouldn't even use it at all, since wrong indices mostly indicate programming mistakes.
- Why do you cast the size to long? You should rather use an unsigned type like std::size_t.
- If I see that correctly, you iterate through all enemies, and at each enemy, you search it again from beginning, although you actually know its position in the container. This is unnecessarily complicated.
- When you allocate your objects dynamically with new, you have to use delete to free their memory.
-
It seems to me I didn't read deep enough. :roll:
Good advices here!
-
So nexus, what you recommend?
-
To delete the class from memory, i invoke:
delete this;
And now the destructor is called.
-
So nexus, what you recommend?
I have already given you some improvement suggestions, so I don't know what you mean.
To delete the class from memory, i invoke:
delete this;And now the destructor is called.
I wouldn't do that. In the very most cases, the instance which is in charge of allocating the object should also deallocate it for symmetry reasons. With your approach, you enforce the use of the new operator and therefore manual memory management (which should be avoided wherever possible). One can neither lay an Enemy on the stack, nor create a temporary object of it, nor put it inside a smart-pointer.
If I were you, I would either use Boost's pointer containers (the probably best approach), or delete each element before it is erased. If you have the TR1, std::tr1::shared_ptr might be an alternative.
-
Hi,
With boost smartpointer, when i do the ERASE from vector it will be deleted?
Do you have a function example?
-
With boost smartpointer, when i do the ERASE from vector it will be deleted?
Yes. As soon as the smart-pointer object is destroyed, the memory is deallocated. But if you are already using Boost, you should try the pointer-containers. They are much more specialized and optimized for this purpose.
Anyway, I wonder why you need pointers. Do you need polymorphism, or are your Enemy instances noncopyable or unique? If not, the straightforward approach is to store Enemy objects using STL containers:
std::vector<Enemy> v;
v.push_back( Enemy(...) );
v[0].Attack();
v.erase(v.begin()); // Memory is freed
Then the Boost pointer-container approach, storing Enemy* pointers:
boost::ptr_vector<Enemy> v;
v.push_back( new Enemy(...) ); // Insert pointer to object
v[0].Attack(); // No additional dereferencing (no ->, just .)
v.erase(v.begin()); // Memory is freed