Jump to content

  • Log In with Google      Sign In   
  • Create Account

We're offering banner ads on our site from just $5!

1. Details HERE. 2. GDNet+ Subscriptions HERE. 3. Ad upload HERE.


Killing AI


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
34 replies to this topic

#21 Servant of the Lord   Crossbones+   -  Reputation: 20937

Like
1Likes
Like

Posted 21 January 2013 - 05:28 PM

Just so I'm clear, the iterator isn't invalidated by pop_back(), except when the iterator is pointing at the last element, right?

 

std::vector::pop_back() - The end iterator and any iterator, pointer and reference referring to the removed element are invalidated.
Iterators, pointers and references referring to other elements that have not been removed are guaranteed to keep referring to the same elements they were referring to before the call.


So only if there is just one element left, then it will crash. I'm not seeing how it will crash on the first element.
 
So, would this work:

 

for(ZombieIterator it = zombies.begin(); it != zombies.end();)
{
	if(it->dead)
	{
		//Swap the value of the current zombie with the zombie at the back of the container.
		std::swap(*it, zombies.back());
		//Pop the back of the container.
		zombies.pop_back();
		
		if(zombies.empty())
		{
			break;
		}
	}
	else
	{
		++it;
	}
}


Or am I just digging myself deeper? smile.png


Edited by Servant of the Lord, 21 January 2013 - 05:31 PM.

It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal

[Fly with me on Twitter] [Google+] [My broken website]

[Need web hosting? I personally like A Small Orange]


Sponsor:

#22 Mercile55   Members   -  Reputation: 136

Like
0Likes
Like

Posted 22 January 2013 - 02:41 AM

this will do the job :)

#23 TheChubu   Crossbones+   -  Reputation: 4746

Like
1Likes
Like

Posted 22 January 2013 - 04:10 AM

while ( it != zombies.end() )
{
    if  ( it->dead )
    {
        it = zombies.erase( it );
    }
    else
    {
        ++it;
    }
}

I remembered I had a similar issue before and using a while loop instead worked better :D


"I AM ZE EMPRAH OPENGL 3.3 THE CORE, I DEMAND FROM THEE ZE SHADERZ AND MATRIXEZ"

 

My journals: dustArtemis ECS framework and Making a Terrain Generator


#24 belfegor   Crossbones+   -  Reputation: 2716

Like
3Likes
Like

Posted 22 January 2013 - 04:30 AM

Without iterators:
for(std::size_t i = 0; i < v.size(); ++i)
	{
		if(v[i].dead)
		{
			
			std::swap(v[i], v.back());
			v.pop_back();
			--i;
		}
	}

Edited by belfegor, 22 January 2013 - 04:32 AM.


#25 EWClay   Members   -  Reputation: 659

Like
0Likes
Like

Posted 22 January 2013 - 04:58 AM

It's not a question of whether there's one element left or not, when you pop_back the last element the iterator is invalidated right before you compare it with end().

Now, I think a sensible implementation would not crash here (in contrast to the first version), but it should give an error in debug mode. You could get around it by special-casing the last element.

Or use indices, which don't have this problem.

#26 mypel16000   Members   -  Reputation: 46

Like
0Likes
Like

Posted 22 January 2013 - 08:14 AM

@bleferog: That's it! I no longer count with the problem of various zombies being deleted at the same time. But one last thing, the swap function is not working..... only the last one is getting deleted.



#27 mypel16000   Members   -  Reputation: 46

Like
0Likes
Like

Posted 22 January 2013 - 01:00 PM

anyone know why the swap isn't happening?



#28 belfegor   Crossbones+   -  Reputation: 2716

Like
2Likes
Like

Posted 22 January 2013 - 01:02 PM

...But one last thing, the swap function is not working..... only the last one is getting deleted.

If your vector is not holding pointers as elements then you must implement "proper" copy constructor and assignment operator, as implicit one might not do the job well depending on the data your zombie class is holding.

 

What not use shared_ptr?

 

class CZombie
{
private:
    bool dead;
    int health;
public:
    CZombie(int h) : dead(false), health(h) {}
    ...
    bool isDead() const { return dead; }
    void takeDamage(int damage)
    {
        health -= damage;
        if(health <= 0)
            dead = true;
    }
};
 
typedef std::shared_ptr<CZombie> sp_Zombie;
typedef std::vector<sp_Zombie> v_Zombie;
...
v_Zombie zombies;
...
    zombies.push_back( std::make_shared<CZombie>(100) );
    zombies.push_back( std::make_shared<CZombie>(120) );
    zombies.push_back( std::make_shared<CZombie>(100) );
    zombies.push_back( std::make_shared<CZombie>(150) );
...
    for(std::size_t i = 0; i < zombies.size(); ++i)
    {
        if(zombies[i]->isDead())
        {
            std::swap(zombies[i--], zombies.back());
            zombies.pop_back();
        }
    }

Notice that i moved decrement up and make it post-decrement, witch does the same thing, but have one line of code less. laugh.png



#29 mypel16000   Members   -  Reputation: 46

Like
0Likes
Like

Posted 22 January 2013 - 01:38 PM

I appreciate you giving me a solution to the problem, but this doesn't work on my C++.... y get the error 

expected initializer before '<' token| for the line typedef std::shared_ptr<CZombie> sp_Zombie;

Isn't there a simpler way of initialising a vector of pointers like maybe creating an array of objects and then using them to create a vector.... Please, I have got really far and I am just stuck on this silly little thing that I just can't seem to fix



#30 Servant of the Lord   Crossbones+   -  Reputation: 20937

Like
0Likes
Like

Posted 22 January 2013 - 01:44 PM

Are you actually needing pointers? Just use std::vector<Zombie> unless you actually need to, and use this code posted a couple posts ago.


It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal

[Fly with me on Twitter] [Google+] [My broken website]

[Need web hosting? I personally like A Small Orange]


#31 mypel16000   Members   -  Reputation: 46

Like
0Likes
Like

Posted 22 January 2013 - 02:14 PM

actually your code doesn't work. It doesn't actually swap the objects, it just deletes the last one. I'm in need of a solution for your code



#32 belfegor   Crossbones+   -  Reputation: 2716

Like
1Likes
Like

Posted 22 January 2013 - 02:15 PM

Do you have included <memory> header? It is necessary for shared_ptr.

What compiler are you using?

 

If you want "pure" pointers then code is a bit different:

class CZombie
{
...
};
 
typedef std::vector<CZombie*> v_Zombies
 
class CZombieMgr
{
    v_Zombies zombies;
public:
...
 
~CZombieMgr()
{
    for(std::size_t i = 0; i < zombies.size(); ++i)
    {
        delete zombies[i];
    }
}
 
void addZombie(int h)
{
    zombies.push_back( new CZombie(h) );
}
 
void removeDeadZombies()
{
    for(std::size_t i = 0; i < zombies.size(); ++i)
    {
        if(zombies[i]->isDead())
        {
            std::swap( zombies[i--], zombies.back() );
            delete zombies.back();
            zombies.pop_back();
        }
    }
};
...



#33 belfegor   Crossbones+   -  Reputation: 2716

Like
1Likes
Like

Posted 22 January 2013 - 02:19 PM

@moderators

Sorry for double post, but if i go to EDIT then it somehow messes my code up.

 

@mypal1600000

Can you post relevant bits from your code? it will be easier to help you.



#34 mypel16000   Members   -  Reputation: 46

Like
0Likes
Like

Posted 22 January 2013 - 02:55 PM

No! It won't be needed as my code has been fixed. Thanks to all of you who, by discussing together, have found the solution to the problem.



#35 Mercile55   Members   -  Reputation: 136

Like
1Likes
Like

Posted 22 January 2013 - 06:20 PM

Without iterators:

for(std::size_t i = 0; i < v.size(); ++i)
	{
		if(v[i].dead)
		{
			
			std::swap(v[i], v.back());
			v.pop_back();
			--i;
		}
	}

Remember that modification of the for-loop index inside the loop is one bad programming practice. It usually leads to infinite loops and computer hang ups.






Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS