CODEWITHSUNDEEP
c#
tracer tong
tracer tong

Reputation: 553

Collection was modified where items are not being modified

I am developing a simple game with the XNA library for C#. In the following code snippet I am getting the

Collection was modified; enumeration operation may not execute.

error at the top of the second foreach loop. In my (relatively limited) experience of C# this occurs when attempting to modify the underlying collection during a loop. However, as far as I can see I am not modifying the enemy_positions collection in any way. All the collections in this code are of the type List<Vector2>.

What is happening here?

//defines collision behaviour when enemy is hit
int be_no = 0;
List<Vector2> tmp_bullets = bullet_i_position;
List<Vector2> tmp_enemy = enemy_positions;

foreach (Vector2 bullet in bullet_i_position)
{
    //get bullet collision box
    Rectangle bullet_col = new Rectangle(Convert.ToInt32(bullet.X - 12), Convert.ToInt32(bullet.Y - 12), 25, 26);

    int en_no = 0;

    foreach (Vector2 enemy in enemy_positions)
    {
        //get enemy collsion box
        en_box = new Rectangle(Convert.ToInt32(enemy.X), Convert.ToInt32(enemy.Y), 75, 75);

        if (temp_r.Intersects(en_box))
        {
            //remove all colliding elements
            tmp_enemy.RemoveAt(en_no);
            tmp_bullets.RemoveAt(be_no);
            bullet_direction.RemoveAt(be_no);

        }
        en_no++;
    }
    be_no++;
}

//update actual lists
bullet_i_position = tmp_bullets;
enemy_positions = tmp_enemy;

Upvotes: 4

Views: 618

Answers (3)

RoughPlace
RoughPlace

Reputation: 1121

as others have mentioned you have encountered the difference between reference and value types

i would suggest you replace the foreach loops with something similar to the below

for(int i = 0; i<bullet_i_position.count;i++)
  {
     bool hascollided = false;
     for(int j = 0; j<enemy_positions.count;j++)
        {
          if(collisionOccurs)
               {
                hasCollided = true;
                enemy_positions.RemoveAt(j);
                j--;
               }
       }
   if(hasCollided)
      {
         bullet_i_position.RemoveAt(i);
         i--;
      }
}

you need to consider what would happen if the bullet collided with 2 enemies at the same time in the above example it would take out both

Upvotes: 0

user932887
user932887

Reputation: 

List<Vector2> tmp_enemy = enemy_positions;

Does not make a copy of enemy_positions and assign it to tmp_enemy. Instead, tmp_enemy points to enemy_positions. Thus, any change in tmp_enemy is reflected in enemy_positions. If you want to make an actual copy, this is a better approach:

List<Vector2> tmp_enemy = new List<Vector2>(enemy_positions);

Upvotes: 0

Robert Rouhani
Robert Rouhani

Reputation: 14678

The lines:

List<Vector2> tmp_bullets = bullet_i_position;
List<Vector2> tmp_enemy = enemy_positions;

are not cloning the list, they're just creating a local reference to the same list. The direct solution would be to change these two lines to:

List<Vector2> tmp_bullets = new List<Vector2>(bullet_i_position);
List<Vector2> tmp_enemy = new List<Vector2>(enemy_positions);

But this will be allocating a new list every time the method is called, which is going to be terrible for garbage collection (especially in a game), so the better solution is to remove your foreach loops and replace them with reverse for loops. This works because you only get that exception when iterating a collection with enumerators. The same problem does not apply to regular for loops. For example:

for (int i = bullet_i_position.Count - 1; i >= 0; i--)
{
    Vector2 bullet = bullet_i_position[i];

    // ...
}

Also the reason for the reverse iteration is that removing an element while regularly iterating means that you'll skip the element after a removed element (beause the indexes shift down by 1)

Upvotes: 6

Related Questions