Python 2.7 throws ValueError: list.remove(x): x no

Question

Every time I run this program, I get this error:

ValueError: list.remove(x): x not in list

I am trying to lower the health of a single alien whenever it is hit by a bolt. That single alien should also be destroyed if its health is <= 0. Similarly, the bolt would also be destroyed. Here is my code:

def manage_collide(bolts, aliens):
    # Check if a bolt collides with any alien(s)
    for b in bolts:
        for a in aliens:
            if b['rect'].colliderect(a['rect']):
                for a in aliens:
                    a['health'] -= 1
                    bolts.remove(b)
                    if a['health'] == 0:
                        aliens.remove(a)
    # Return bolts, aliens dictionaries
    return bolts, aliens

The ValueError happens on the line aliens.remove(a). Just to clarify, both the aliens and bolts are lists of dictionaries.

What am I doing wrong?

Answer 1

There is a bug in your code that is causing this. Your code, simplified, looks like:

for b in bolts:
  for a in aliens:
    for a in aliens:
      bolts.remove(b)

That is causing you to loop over aliens multiple times for every entry in b. If the b is removed on the first loop over aliens then, when it loops over it a second time, you will get there error.

A few things to fix. First, change in the inner loop over aliens to use something other than a, so:

for b in bolts:
  for a in aliens:
    for c in aliens:
      if hit:
        bolts.remove(b)

Second, only remove b from bolts once. so:

for b in bolts:
  for a in aliens:
    should_remove = False
    for c in aliens:
      if hit:
        should_remove = True
    if should_remove:
      bolts.remove(b)

There are other issues with this code as well, I think, but that is the cause your main problem. Martijn's post may also help.

Answer 2

You should not remove items from a list you are looping over. Create a copy instead:

for a in aliens[:]:

and

for b in bolts[:]:

Modifying a list while looping over it, affects the loop:

>>> lst = [1, 2, 3]
>>> for i in lst:
...     print i
...     lst.remove(i)
... 
1
3
>>> lst
[2]

Removing items from a list you are looping over twice makes things a little more complicated still, resulting in a ValueError:

>>> lst = [1, 2, 3]
>>> for i in lst:
...     for a in lst:
...         print i, a, lst
...         lst.remove(i)
... 
1 1 [1, 2, 3]
1 3 [2, 3]
Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
ValueError: list.remove(x): x not in list

When creating a copy of the lists you are modifying at each level of your loops, you avoid the problem:

>>> lst = [1, 2, 3]
>>> for i in lst[:]:
...     for i in lst[:]:
...         print i, lst
...         lst.remove(i)
... 
1 [1, 2, 3]
2 [2, 3]
3 [3]

When you have a collision, you only need to remove the b bolt once, not in the loop where you hurt the aliens. Clean out the aliens separately later:

def manage_collide(bolts, aliens):
    for b in bolts[:]:
        for a in aliens:
            if b['rect'].colliderect(a['rect']) and a['health'] > 0:
                bolts.remove(b)
                for a in aliens:
                    a['health'] -= 1
    for a in aliens[:]:
        if a['health'] <= 0:
            aliens.remove(a)
    return bolts, aliens

Answer 3

Give the bolts a "health" as well, initialized to 1. Then you can do one nested loop to calculate all the damage, and two separate unnested "loops" to remove everything that's "dead". Except, don't do it quite like that, because you still don't want to modify the list that you're looping over. Making a copy is still too complicated. What you really want to do is directly build a new list of only the still "alive" things, and you can do that descriptively with list comprehensions (or as shown here, with filter).

# for example
class Alien:
    # ... other stuff
    def damage(self): self.hp -= 1
    def alive(self): return self.hp > 0

# similarly for Bolt

def collide(an_alien, a_bolt):
    # etc.

def handle_collisions(aliens, bolts):
    for a in aliens:
        for b in bolts:
            if collide(a, b):
                a.damage()
                b.damage()

    return list(filter(Alien.alive, aliens)), list(filter(Bolt.alive, bolts))