Advertisement

Remove() an instance from a List<> inside the instance!

Started by March 29, 2022 08:14 PM
13 comments, last by pompompom 2 years, 8 months ago

Hi. I've looked around but not found a decent solution to this.

  • Suppose I have a class which defines a Sprite (x, y, etc.)
  • Then I create a List<Sprite> called sprites
  • Then in my game loop I go: foreach(Sprite s in sprites) s.render();
  • But in the render function of my Sprite class, I want a bit to go: if (offscreen) sprites.Remove(this);

But when the instance tries to delete itself from the queue, I get “Collection was modified; enumeration operation may not execute”.

I can understand why this error is happening. Is there an elegant way of, within the class itself, the instance saying “I want to destroy myself”?

The main solution would be to have a simple “active” boolean as part of the class, and set to True upon instantiation and then False upon self-destruction, but the idea of the List building up with inactive instances sounds a bit rubbish.

Yeah, don't invalidate a list while in the middle of processing it.

If this is going to be a common element of the design, I'd add a flag to the item. It's actually pretty common in particle systems to let each item be flagged as active or inactive, since a flag is much faster than actually adding and removing items. If items are no longer needed a second pass to do the work or even calling remove_if() can work.

Advertisement

Not sure what exactly you are doing. If this is thousands of particles, then a bool if it is alive or not is good. Just don't perform logic on ones that are (!alive).

If you want an actual list, there is no reason in c++ you can't do something like this. I'm not sure if the foreach() statement will let you manually increment though or set that iterator. If that's C# then I definitely don't know, but you need to handle it in any language something like this:

list<Sprite>::iterator spriteIter = Sprites.begin();
while(spriteIter ≠ Sprites.End())
{
if(want to remove)
{
list<Sprite>::iterator tempIter = spriteIter;
tempIter++;
Sprites.remove(spriteIter);
spriteIter = tempIter;
}
else
{
spriteIter++;
}
}

NBA2K, Madden, Maneater, Killing Floor, Sims

Dave Haylett said:
But in the render function of my Sprite class, I want a bit to go: if (offscreen) sprites.Remove(this);

List<Sprite> sprites; // Sprites that may be on-screen, off-screen, or empty entries (null).
int next = 0;
for (int spr = 0; spr < sprites.size(); spr++) {
   Sprite s = sprites[spr];
   if (s != null && onScreen(s)) {
       paint(s);
       if (next != spr) { // Move s to 'next'
          sprites[next] = s;
          sprites[spr] = null;
       }
       next++;
   }
}
// From sprites[next] to the end are not used.
// You may want to truncate the list at this point or keep 'next' for the next
// iteration ( spr=[0..next) instead of [0..sprites.size()) ), or for adding new sprites.

Thank you for your replies. Actually as I was falling asleep last night a solution similar to what's been suggested came to me.

In my Render function, instead of attempting to Remove(), I can return a boolean, saying whether to delete or retain that instance or not. It feels better to return false to remove it and true to keep it.

Then I can amend my foreach to a for next loop, and delete offscreen sprites on the fly, like this:

for(int i=0; i < sprites.Count; i++) if (!sprites[i].Physics()) sprites.Remove(sprites[i]);

Seems to work fine ?

You might want to debug and watch that. If you are at i = 0, and call remove on the element 0, then it may shift the whole array/vector etc so that the actual second element is now at index 0. You may need to decrement i by 1 if removing, this way it will process index 0 again. Depends on what Remove does internally.

NBA2K, Madden, Maneater, Killing Floor, Sims

Advertisement

Thank you @dpadam450 I missed that. I can prove that that's exactly what happens with a test List of 3 strings, A, B and C, and if I remove B then I lose C (after it's shifted to the left), but it doesn't appear to happen with my sprites. It's either a coincidence that one of the last ones in the list is being removed, or the shifting doesn't actually occur with classes. I've not tested this properly, just visually with a load of sprites going off the screen (and not seeing random ones disappearing like I was expecting to).

But either way, if I put i--; after my Remove then that doesn't lose the subsequent element.

It's a data structure + algorithm. Independent of what data type, It will always execute the same identical code at the core.

NBA2K, Madden, Maneater, Killing Floor, Sims

I do this with lists of weak pointers all the time:

 auto it = list.begin();
 while (it != list.end())
 {
  auto thing = (*it).lock();
  if (thing == NULL)
  {
   list.erase(it++);
   continue;
  }
  thing->Update();
  it++;
 }

10x Faster Performance for VR: www.ultraengine.com

I'm kind of surprised people keep posting C++-answers when OP is using C# (he didn't say that but his syntax is that of C#). That said, you could do:

Sprites.RemoveAll(sprite =>
{
	return !sprite.Physics();
});

Might seem a bit odd at first but I stand firmly at trying not to have raw loops wherever possible, since it simplifies that operation you want to do considerably.

This topic is closed to new replies.

Advertisement