"Vector erase iterator out range"

[kong288]

This is the code:

for(int i = 0; i < zombies.size(); i++)
	{
		zombies[i]->Move();
		for(int x = 0; x < player.ReturnVector().size(); x++)
		{
			if(zombies[i]->RS().getGlobalBounds().contains(player.ReturnVector()[x].RS().getPosition()))
			{
				zombies.erase(zombies.begin() + i); //works perfectly
				player.ReturnVector().erase(player.ReturnVector().begin() + x); //line that gives error
			}
		}

	}

I don't understand why it's happening because the line above it has the exact same values passing though it and it works.

When I used debug, I found that after I shoot a bullet, X (from the for loop) is equal to 0 and player.vector has a value of 1. Which is expected.

[keskiverto]

What are the types of zombies and player.ReturnVector()?

[kong288]

zombies: std::vector<Zombie*> zombies; (Zombie is a class)

player's vector: std::vector<Bullet> bullets; (Bullet is a class)
player's function:

std::vector<Bullet> Player::ReturnVector()
{
	return bullets;
}

Sorry for not adding this to start with.

[keskiverto]

Good. Guess why on line 9 the player.ReturnVector().begin() does not refer to the same memory as the vector that you try to erase from.

Hint: function returns a copy.

[kong288]

But wouldn't every time the if statement is called it'd return a fresh copy of the vector?

[keskiverto]

Every time you call ReturnVector(), you get a new copy. You never touch the vector that is inside player ...

[kong288]

OH! I understand now.

What would be the best way to change the vector from the function rather than just returning it?

[cire]

std::vector<Bullet>& Player::ReturnVector()
{
    return bullets ;
}

However, that won't be the end of your problems. I wouldn't suggest using for loops here.

Whenever you delete an element from a vector, the vector gets smaller. What happens in the inner for loop when the vector's size becomes equal to i? What's more, when you erase an element every element is moved down one index. (When you erase vector[7], vector[8] becomes vector[7].) So you end up not checking some elements against other elements.

I would expect your code to look more like the following (disclaimer: Not tested, may require some minor adjustments to work correctly.)

    auto zombie = zombies.begin() ;
    while ( zombie != zombies.end() )
    {
        (*zombie)->Move() ;

        auto & bulletVec = player.ReturnVector() ;
        auto bullet = bulletVec.begin() ;

        while ( bullet != bulletVec.end() && !((*zombie)->RS().getGlobalBounds().contains(bullet->RS().getPosition())) )
            ++bullet ;

        if ( bullet != bulletVec.end() )
        {
            zombie = zombies.erase(zombie) ;
            bulletVec.erase(bullet) ;
        }
        else
            ++zombie ;
    }

Also, I think the design would benefit from the bullets vector being divorced from the player object.

[kong288]

Oh wow, there's so much I don't know. Ha.

Thanks a lot. I'll definitely experiment with that! (the reference worked).

I just go into messing with iterators (you're using iterators right there, right?)

[cire]

you're using iterators right there, right?

Correct.