Python 033 - The list

Python Asteroids on GitHub

I’ll pick something off the yellow sticky-notes Jira on my keyboard tray.

Here’s “Jira” as it stands:

• Smart collections?
• Game object
• Main loop
• Globals etc
• Hyperspace
• Saucer
• Available Ship Display tuning
• missile vs ship (hyperspace)

That last one, to remind myself, reflects that missiles are not currently tested for colliding with the ship, because they generally can’t get to it, but that if the ship were to emerge from hyperspace in the path of a missile, it should be destroyed.

We could do that easily enough. Might be good for warmup. Let’s see what we have now in code and tests. Tests first. We have a few tests that all look like this one:

    def test_missile_asteroid_scores(self):
        u.score = 0
        pos = Vector2(100, 100)
        asteroid = Asteroid(2, pos)
        asteroids = [asteroid]
        missile = Missile(pos, Vector2(0, 0))
        missiles = [missile]
        asteroid.collide_with_attacker(missile, missiles, asteroids)
        assert not missiles
        assert u.score == 20

    def test_asteroid_ship_does_not_score(self):
    	...
    def test_asteroid_saucer_does_not_score(self):
    	...

I’ll do one for missile-ship, following that pattern.

    def test_missile_ship_does_not_score(self):
        u.score = 0
        pos = Vector2(100, 100)
        ship = Ship(pos)
        ships = [ship]
        missile = Missile(pos, Vector2(0, 0))
        missiles = [missile]
        ship.collide_with_attacker(missile, missiles, ships)
        assert not missiles
        assert not ships
        assert u.score == 20

The ship does not have a collide_with_attacker method yet. Let’s find one to plagiarize. Asteroid has this:

    def collide_with_attacker(self, attacker, attackers, asteroids):
        if self.withinRange(attacker.position, attacker.radius):
            if attacker in attackers: attackers.remove(attacker)
            u.score += attacker.score_list[self.size]
            self.split_or_die(asteroids)

Ship should be much simpler. No scoring, but remove self from the second collection, which is by convention ships.

I notice that I’ve named the withinRange in the wrong style. PyCharm helps me with that in ship:

    def collide_with_attacker(self, attacker, attackers, ships):
        if self.within_range(attacker.position, attacker.radius):
            if attacker in attackers: attackers.remove(attacker)
            if self in ships: ships.remove(self)

    def within_range(self, point, other_radius):
        dist = point.distance_to(self.position)
        return dist < self.radius + other_radius

Test is not passing. Ah, it’s checking score. No score, sorry.

    def test_missile_ship_does_not_score(self):
        u.score = 0
        pos = Vector2(100, 100)
        ship = Ship(pos)
        ships = [ship]
        missile = Missile(pos, Vector2(0, 0))
        missiles = [missile]
        ship.collide_with_attacker(missile, missiles, ships)
        assert not missiles
        assert not ships
        assert u.score == 0

That’s what happens when you copy and paste code: you get it nearly right. Well, that’s what happens to me. I don’t know what happens to you (but I know how I’d bet).

Now we need to do the check in the main loop. I don’t have tests for that.

def check_collisions():
    check_asteroids_vs_ship()
    check_asteroids_vs_missiles()

We’ll add a new function. And I think I’d like to rename the first one, but not right now, I’m on a mission. No, right now, because PyCharm will just do it. So:

def check_collisions():
    check_ship_vs_asteroids()
    check_asteroids_vs_missiles()
    check_ship_vs_missiles()

Now, I copied this manually from the asteroids one. They seem rather similar:

def check_ship_vs_asteroids():
    for the_ship in ships.copy():  # there's only one, do it first
        for asteroid in asteroids.copy():
            asteroid.collide_with_attacker(the_ship, ships, asteroids)
            if not ships:
                set_ship_timer(u.SHIP_EMERGENCE_TIME)
                return


def check_ship_vs_missiles():
    for the_ship in ships.copy():
        for missile in missiles.copy():
            ship.collide_with_attacker(missile, missiles, ships)
            if not ships:
                set_ship_timer(u.SHIP_EMERGENCE_TIME)
                return

My tests are still green, of course, and I want to know if this works. I can set the missile lifetime to a large number for manual testing. I then fire a missile up my wazoo and the ship dies. So that’s good. What about this duplication, though? We should be able to extract a function.

PyCharm can’t cope with that return in the middle. I’ll do it manually. The function wants to take the attackers and their collection as parameter.

No, wait! The first function calls collides_with on the attacker, the second calls it on the ship. These are not the same.

Let’s review all three of the collision checkers:

def check_asteroids_vs_missiles():
    for asteroid in asteroids.copy():
        for missile in missiles.copy():
            asteroid.collide_with_attacker(missile, missiles, asteroids)


def check_ship_vs_asteroids():
    for the_ship in ships.copy():  # there's only one, do it first
        for asteroid in asteroids.copy():
            asteroid.collide_with_attacker(the_ship, ships, asteroids)
            if not ships:
                set_ship_timer(u.SHIP_EMERGENCE_TIME)
                return


def check_ship_vs_missiles():
    for the_ship in ships.copy():
        for missile in missiles.copy():
            ship.collide_with_attacker(missile, missiles, ships)
            if not ships:
                set_ship_timer(u.SHIP_EMERGENCE_TIME)
                return

There are two things going on with the ship. First, once the ship has been killed, we don’t want to continue to collide things against it. It is barely possible that two asteroids could hit the ship at the same cycle and both get scored, so if the ship has been killed, we should stop. And I’ve seen it happen, or think that I have.

Strictly speaking the same is true with asteroids and missiles. Once they are gone, we should stop processing them.

The other matter is the ship timer, which needs to be set if the ship is killed. We could leave that out and do it in the calling method. Let’s do that first.

def check_collisions():
    check_ship_vs_asteroids()
    check_asteroids_vs_missiles()
    check_ship_vs_missiles()
    if not ships:
        set_ship_timer(u.SHIP_EMERGENCE_TIME)

This is needing tests more and more, isn’t it? But it’s a big test and hard to write. Need to think this out. But I am weak and I think I can make this code right without a test in code.

Moving that up lets me make these three functions more similar:

def check_asteroids_vs_missiles():
    for asteroid in asteroids.copy():
        for missile in missiles.copy():
            asteroid.collide_with_attacker(missile, missiles, asteroids)


def check_ship_vs_asteroids():
    for the_ship in ships.copy():  # there's only one, do it first
        for asteroid in asteroids.copy():
            asteroid.collide_with_attacker(the_ship, ships, asteroids)
            if not ships:
                return


def check_ship_vs_missiles():
    for the_ship in ships.copy():
        for missile in missiles.copy():
            ship.collide_with_attacker(missile, missiles, ships)
            if not ships:
                return

I observe that in the first function, if the asteroid does collide, we should stop checking that asteroid. If not, in principle we could score twice on one asteroid, and split it twice.

Could we just break? I can’t think of the last time I used break, don’t even know if Python has it. But if we did it would return us to the outer loop, which would either produce another asteroid or ship, or not. In the case of the ship … not. There can be only one.

So let’s do this:

def check_asteroids_vs_missiles():
    for asteroid in asteroids.copy():
        for missile in missiles.copy():
            asteroid.collide_with_attacker(missile, missiles, asteroids)
            if asteroid not in asteroids:
                break


def check_ship_vs_asteroids():
    for the_ship in ships.copy():  # there's only one, do it first
        for asteroid in asteroids.copy():
            asteroid.collide_with_attacker(the_ship, ships, asteroids)
            if the_ship not in ships:
                break


def check_ship_vs_missiles():
    for the_ship in ships.copy():
        for missile in missiles.copy():
            ship.collide_with_attacker(missile, missiles, ships)
            if the_ship not in ships:
                break

These are looking pretty similar now.

I tried to test this in the game and the ship does not appear. Oh. We can’t check for ships in check_collisions, because we only want to set the timer once, not every time through the loop. Let’s see if we can change this:

def set_ship_timer(seconds):
    global ship_timer
    ship_timer = seconds

Right. We’ll only set it if it needs setting:

def set_ship_timer(seconds):
    global ship_timer
    if ship_timer <= 0:
        ship_timer = seconds

If this doesn’t work I’d better revert. It does work and the game runs. Time for a commit: Making collision code look similar enough to combine.

It’s not similar enough, but I wanted to capture a working state. Here are our three functions now:

def check_asteroids_vs_missiles():
    for asteroid in asteroids.copy():
        for missile in missiles.copy():
            asteroid.collide_with_attacker(missile, missiles, asteroids)
            if asteroid not in asteroids:
                break


def check_ship_vs_asteroids():
    for the_ship in ships.copy():  # there's only one, do it first
        for asteroid in asteroids.copy():
            asteroid.collide_with_attacker(the_ship, ships, asteroids)
            if the_ship not in ships:
                break


def check_ship_vs_missiles():
    for the_ship in ships.copy():
        for missile in missiles.copy():
            ship.collide_with_attacker(missile, missiles, ships)
            if the_ship not in ships:
                break

The three are so similar as to confuse me but they are not the same. The first and last send the collision message to the outer loop parameter, and the middle one sends the message to the inner loop parameter. Let me rename the parameters for a moment to better see what’s up:

Eeek, I notice just now that I’m sending to ship, not the_ship in the last one. I change that with my other renaming:

    for outer in asteroids.copy():
        for missile in missiles.copy():
            outer.collide_with_attacker(missile, missiles, asteroids)
            if outer not in asteroids:
                break


def check_ship_vs_asteroids():
    for outer in ships.copy():  # there's only one, do it first
        for inner in asteroids.copy():
            inner.collide_with_attacker(outer, ships, asteroids)
            if outer not in ships:
                break


def check_ship_vs_missiles():
    for outer in ships.copy():
        for inner in missiles.copy():
            outer.collide_with_attacker(inner, missiles, ships)
            if outer not in ships:
                break

Two outers, one inner. And, curiously enough, the outers that differ are both in the ship case.

What would happen if we changed that middle one to send to outer, the asteroid? I think we’d need to add a scoring vector to ship but perhaps that would be all. I’ll try it.

Well, I’m not sure why, but the ship dies immediately? Oh, I did it wrong.

Also I’ve confused myself. This is why I wanted the save point. Roll back. Change the middle one:

def check_ship_vs_asteroids():
    for the_ship in ships.copy():  # there's only one, do it first
        for asteroid in asteroids.copy():
            the_ship.collide_with_attacker(asteroid, asteroids, ships)
            if the_ship not in ships:
                break

With that in place, the bug is that when the ship collides with the asteroid, the asteroid does not split. The reason is that when the asteroid does the colliding, it knows to split, but when the ship does the colliding, it does not know to split the asteroid, and the asteroid never gets involved.

What could we do? I believe that Chet would tell us that we should use double dispatch. That is, at least when we ask the ship to collide with something, it should send a message to the other object telling it that it’s colliding with a ship. In principle, we do them all that way, so that both classes are known at the time of collision. And we would probably then call back to the original guy to tell him he was dying, as a matter of courtesy.

We could check the range on the first call of course, and then dispatch.

This is too tricky for Sunday. I’ll roll back and combine the two that are the same.

I claim that these two are the same:

def check_asteroids_vs_missiles():
    for asteroid in asteroids.copy():
        for missile in missiles.copy():
            asteroid.collide_with_attacker(missile, missiles, asteroids)
            if asteroid not in asteroids:
                break


def check_ship_vs_missiles():
    for the_ship in ships.copy():
        for missile in missiles.copy():
            the_ship.collide_with_attacker(missile, missiles, ships)
            if the_ship not in ships:
                break

PyCharm seems to be willing to extract a function. But it doesn’t know what parameters I want. I’ll do it manually again.

def check_individual_collisions(targets, attackers):
    for target in targets.copy():
        for attacker in attackers.copy():
            target.collide_with_attacker(attacker, attackers, targets)
            if target not in targets:
                break

Now the other two methods should be able to call this one:

def check_asteroids_vs_missiles():
    check_individual_collisions(asteroids, missiles)


def check_ship_vs_missiles():
    check_individual_collisions(ships, missiles)


def check_individual_collisions(targets, attackers):
    for target in targets.copy():
        for attacker in attackers.copy():
            target.collide_with_attacker(attacker, attackers, targets)
            if target not in targets:
                break

And that works. Let’s commit: combined check_asteroids_vs_missiles and check_ships_vs_missiles. ships_vs_asteroids can’t be combined yet.

I’m torn. It’s only 0930. Should I push to get target combined as well? In favor would be the removal of that duplication. Opposed would be that the real solution deserves more thought. Right now, we have two implementors of collide_with_attacker, one in asteroid, that split asteroids, and one in ships that just dies.

class Asteroid ...
    def collide_with_attacker(self, attacker, attackers, asteroids):
        if self.withinRange(attacker.position, attacker.radius):
            if attacker in attackers: attackers.remove(attacker)
            u.score += attacker.score_list[self.size]
            self.split_or_die(asteroids)

class Ship ...
    def collide_with_attacker(self, attacker, attackers, ships):
        if self.within_range(attacker.position, attacker.radius):
            if attacker in attackers: attackers.remove(attacker)
            if self in ships: ships.remove(self)

We could improve this, or change it, in more than one way. We could go to a full double dispatch. We could pull the within_range up … oh, hold on a moment, I’ll rename that wrong one. Done and committed.

We could pull that up and send a “you have had a collision” message to each of the two, including their collection, which we have in hand, and probably the other member of the colliding pair. Then each object could do its own thing on collision.

That may actually be best. But I want to let that idea perk a bit longer.

We’ll call this done for the day.

Summary

We’ve added a new capability and test, for missiles killing the ship. We’ve removed some duplication after some futzing around, and have ideas for how to improve things. I am not certain that I’d call removing that duplication an improvement. I really like to removed duplication, and it always represents some idea, in this case a generic method that checks collisions between members of two collections. But there’s always a cost in clarity, and when there are just two cases, it’s almost a wash.

More interesting is that the third function has almost the same shape as the other two, so similar that I did not at first see that they were different. I tried to make them the same, but the third one didn’t want to cooperate, and the changes necessary to make it work seem a bit large for a quick improvement.

So that’s deferred. I’ll make a Jira note: Combine ship v asteroids. Pull up collision and message both? I hope that’s enough to remind me what I have in mind.

So we’ve made a little progress and removed “missile vs ship” from our list.

• Smart collections?
• Game object
• Main loop
• Globals etc
• Hyperspace
• Saucer
• Available Ship Display tuning
• Combine ship v missile; pull up collision and message both?
• missile vs ship (hyperspace)

Not bad for a Sunday morning.

See you next time!