CODEWITHSUNDEEP
c#xna
Bensas
Bensas

Reputation: 101

Can't remove item from list in XNA

I'm working on a Flappy Bird clone as an exercise, for I've recently started programming on XNA and I'm stuck on this error that I cannot understand.

I included the following code inside the Update() function, which's job is to delete the pipes when they go off screen lest they infinitely move leftwards while more are created:

//Pipe despawner
foreach (var pipe in pipes)
{
   if (pipe.position1.X <= -180)
   {
         pipes.Remove(pipe);
   }
}

The game runs fine until the first pipe goes offscreen and the debugger pauses and signals this part of the code with the following message:

An unhandled exception of type 'System.InvalidOperationException' occurred in mscorlib.dll

Additional information: Colección modificada; puede que no se ejecute la operación de enumeración.

I'm sorry the second part is in spanish, it's my system's language, but hopefully you know how to solve the problem anyway.

I believe I could simply not include this part of the code, given the simplicity of the game and, by consequence, the small toll that infinitely spawning pipes take on performance, but I'd rather not adopt this practice as soon as I start learning game programming.

Upvotes: 3

Views: 1160

Answers (5)

BradleyDotNET
BradleyDotNET

Reputation: 61339

The problem is that you are modifying the collection (by removing a pipe) while enumerating the collection. This will always throw an exception in .NET (and is something to be mindful of if you ever do this in a multi-threaded environment, since you can get some nastily non-reproducible exceptions because of it).

An easy way to solve it is to just keep a "kill" list while enumerating:

List killList = new List();

foreach (var pipe in pipes)
{
   if (pipe.position1.X <= -180)
   {
      killList.Add(pipe)
   }
}

Even easier:

IEnumerable<Pipe> killList = pipes.Where (p => p.position1.X < -100);

Either way, enumerate over this new collection, and remove matching elements from the main one (thus avoiding the error condition):

foreach (Pipe p in killList)
   pipes.Remove(p);

And you are done! Again, be mindful of threads. If you are creating new pipes in another thread, you could easily "collide" with this one and cause the exception. If that is the case, make sure to put locks around these sections of code.

Note, you could actually inline "killList" if you use the LINQ method:

foreach (Pipe p in pipes.Where(p => p.Position1.X <= -100))
   pipes.Remove(p);

As @rot13 suggested, you could also just run RemoveAll, passing it the predicate from the "Where" statement like:

pipes.RemoveAll(p => p.Position1.X <= -100);

That's about as simple as it gets :)

Upvotes: 4

user3256944 salutes Monica
user3256944 salutes Monica

Reputation: 864

The other answers are perfectly valid and will resolve your issue.

However, I think you should be aware that you don't need to create a second collection just to remove an object during iteration.

We can use reverse iteration to remove an object from your collection. Try the following:

foreach (Pipe pipe in pipes.Reverse<Pipe>())
{
    pipes.Remove(pipe);
}

The above code makes the assumption that your collection is a generic List:

List<Pipe> pipes = new List<Pipe>();

Edit

Here is a sample from the game I'm currently working on:

private List<Debris> listOfDebris = new List<Debris>();

foreach (Debris debris in listOfDebris.Reverse<Debris>())
{
    CalculateDebrisLocation(debris);

    // Remove debris if it travels outside level boundries
    if (!debris.Rect.Intersects(currentLevel.MapRect))
        listOfDebris.Remove(debris);
}

Upvotes: 1

Eric Scherrer
Eric Scherrer

Reputation: 3388

It's saying you cannot modify pipes while looping through it. Try something like this:

        //Pipe despawner
        var unusedPipes = new List<Pipe>();

        foreach (var pipe in pipes)
        {
            if (pipe.position1.X <= -180)
            {
                unusedPipes.Add(pipe);
            }
        }

        foreach (var unusedPipe in unusedPipes)
        {
            pipes.Remove(unusedPipe);
        }

I just took a guess that pipes is a list of pipes.

Upvotes: 1

ChengDuum
ChengDuum

Reputation: 77

Rather than removing the pipes, you may want to simply move them to the other side of the screen and move them up/down.

Upvotes: 3

Pierre-Luc Pineault
Pierre-Luc Pineault

Reputation: 9191

You can't remove an item from the collection you're currently iterating on with foreach.

Either use a for loop and adjust the indexes yourself, or use a second collection to handle the pipes you want to delete :

List<Pipe> piesToRemove = new List<Pipe>();
// First you flag every pipes you need to remove
foreach (var pipe in pipes)
{
   if (pipe.position1.X <= -180)
   {
         pipesToRemove.Add(pipe);
   }
}

//Now you remove them from your original collection
foreach (var pipe in pipesToRemove)
{
    pipes.Remove(pipe);
}

Upvotes: 3

Related Questions