Python Asteroids on GitHub

Let’s see about pulling collision logic off into its own class. Should be easy, he said…

It seems wise, having said it’d be easy, to at least review how collisions work. It goes like this:

    def check_collisions(self):
        self.check_individual_collisions(self.ships, self.asteroids)
        self.check_individual_collisions(self.asteroids, self.missiles)
        self.check_individual_collisions(self.ships, self.missiles)
        if not self.ships:
            self.set_ship_timer(u.SHIP_EMERGENCE_TIME)

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

    def mutual_destruction(self, target, targets, attacker, attackers):
        if self.within_range(target, attacker):
            self.score += target.score_against(attacker)
            self.score += attacker.score_against(target)
            attacker.destroyed_by(target, attackers)
            target.destroyed_by(attacker, targets)

    def within_range(self, target, attacker):
        in_range = target.radius + attacker.radius
        dist = target.position.distance_to(attacker.position)
        return dist <= in_range

Well, that looks truly simple. We’ll have to return the score to the caller. One question that comes to mind is whether we should create a new collision object on each game cycle, or just have one that we hold on to. Let’s assume that we’ll create a new one.

OK. The thing will be created with all the space object collections, asteroids, missiles, saucers, and ships. We’ll send it check_collisions and it will do its thing, returning score.

Seems like a reasonable idea. Let’s think about tests. We have a veritable plethora of tests in the TestCollisions class about a dozen of them.

A number of them, I note, are testing that the ship spawns at the right time, whether safe to emerge works correctly, whether asteroids split properly, and a few actually check whether mutual_destruction works as advertised. There are no tests for check_individual_collisions, and none for within_range. I don’t know whether to feel badly about that or not, but I think I probably should.

And I’m going to proceed without more tests, but will pay attention to what breaks and decide about writing more tests based on that.

Nota Bene
I freely grant that that’s counter to the official TDD dicta. You’re supposed to write tests before code, you’re supposed to have tests for every line, and so on. That is not what I do. I think I’d do better overall if I did do that, but I am just some guy, and my behavior is human, not god-like and not obsessive. I have reached a balance in my work that is comfortable enough to live without guilt and that keeps me from making mistakes that are too embarrassing to write about.

Let’s hope that today goes like that.

I’ll begin by intention, with new code in Game.process_collisions, a new method written to allow me to do what I intend. What do I intend? I intend to move methods over to a new class one at a time. I think it’ll go smoothly. We’ll see.

class Game ...
    def process_collisions(self):
        self.check_collisions()

And I replace the call in the main loop to call process_collisions.

class Game ...
			...
            self.move_everything(self.delta_time)
            self.process_collisions()
            self.draw_everything()
            ...

Now I’ll create a new class, and because it’s so easy, I will write a test for it. See me responding to guilt? That’s a good thing.

    def test_collider(self):
        collider = Collider(asteroids=[],missiles=[], saucers=[], ships=[])
        score = collider.check_collisions()
        assert score == 0

By naming the construction parameters, I’ve given myself and python a clue about how this thing will be created. PyCharm offers to create the class right in the test module. Let’s go that way for a while and see what happens. We can move it when it’s a bit stronger.

class Collider:
    pass

Not very impressive, PyCharm. I remove that and decide to make the new file collider.py:

class Collider:
    def __init__(self, asteroids, missiles, saucers, ships, game):
        self.asteroids = asteroids
        self.missiles = missiles
        self.saucers = saucers
        self.ships = ships
        self.game = game

Note that I’ve added a parameter, game. And I’ll put that in the test:

    def test_collider(self):
        game = Game(True)
        collider = Collider(asteroids=[],missiles=[], saucers=[], ships=[], game=game)
        score = collider.check_collisions()
        assert score == 0

PyCharm wants to create the method for me. I’ll allow it. I fill in the blanks:

    def check_collisions(self):
        self.game.check_collisions()
        return 0

Test runs green. I need a new test. Let’s see how far we can push this deferral before we start moving methods over.

    def test_collider_with_score(self):
        game = Game(True)
        asteroid = Asteroid(2, Vector2(100, 100))
        missile = Missile(Vector2(100, 100), Vector2(3, 3))
        collider = Collider(asteroids=[asteroid],missiles=[missile], saucers=[], ships=[], game=game)
        score = collider.check_collisions()
        assert game.score == 20
        assert score == 20

This succeeds on the game.score check and fails on the return score. Not a big surprise there.

Let’s review the code in Game and see if we can pull the score upward.

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

    def mutual_destruction(self, target, targets, attacker, attackers):
        if self.within_range(target, attacker):
            self.score += target.score_against(attacker)
            self.score += attacker.score_against(target)
            attacker.destroyed_by(target, attackers)
            target.destroyed_by(attacker, targets)

OK first of all, the if statement there can’t ever succeed because mutual_destruction returns None. I intended it to return True if we collided and False otherwise, because of the low but real risk of a double kill, which I believe I’ve seen happen.

I have a test for that and it must be wrong:

    # it's barely possible for two missiles to kill the
    # same asteroid. This used to cause a crash, trying
    # to remove the same asteroid twice.
    def test_dual_kills(self):
        asteroid = Asteroid(2, Vector2(0, 0))
        asteroids = [asteroid]
        asteroid.split_or_die(asteroids)
        assert asteroid not in asteroids
        assert len(asteroids) == 2
        asteroid.split_or_die(asteroids)
        assert len(asteroids) == 2  # didn't crash, didn't split again

No, it’s OK, it’s just that the check isn’t taking advantage of the break. I’d like to break anyway, both for efficiency and because there could be other possibilities of dual hits and I want to avoid them, just because it can’t be good.

    def mutual_destruction(self, target, targets, attacker, attackers):
        if self.within_range(target, attacker):
            self.score += target.score_against(attacker)
            self.score += attacker.score_against(target)
            attacker.destroyed_by(target, attackers)
            target.destroyed_by(attacker, targets)
            return True
        else:
            return False

This does break. I found out by putting a print in before the break. I think we can work from here. Let’s start moving code. Make a collider in process and call it. It should bounce right back and work.

    def process_collisions(self):
        collider = Collider(asteroids=self.asteroids, missiles=self.missiles, saucers=[], ships=self.ships, game=self)
        collider.check_collisions()

This does work, but I have to test it by running the game. My second test isn’t working. I’m not sure why. I’ve modified it to look like this:

    def test_collider_with_score(self):
        game = Game(True)
        asteroid = Asteroid(2, Vector2(100, 100))
        missile = Missile(Vector2(100, 100), Vector2(3, 3))
        collider = Collider(asteroids=[asteroid],missiles=[missile], saucers=[], ships=[], game=game)
        score = collider.check_collisions()
        assert score == 20
        assert game.score == 20

And the assert on the returned score is zero. But the current implementation is this:

    def check_collisions(self):
        self.game.check_collisions()
        return self.game.score

I am too clever for my pants. The current callback isn’t passing the lists back to Game. I think we need to change the test:

    def test_collider_with_score(self):
        game = Game(True)
        asteroid = Asteroid(2, Vector2(100, 100))
        game.asteroids=[asteroid]
        missile = Missile(Vector2(100, 100), Vector2(3, 3))
        game.missiles=[missile]
        collider = Collider(asteroids=game.asteroids, missiles=game.missiles, saucers=[], ships=[], game=game)
        score = collider.check_collisions()
        assert score == 20
        assert game.score == 20

We are green. Now we “just” have to move things and stay green. First move the ship check: we want to keep that in Game:

    def process_collisions(self):
        collider = Collider(asteroids=self.asteroids, missiles=self.missiles, saucers=[], ships=self.ships, game=self)
        collider.check_collisions()
        if not self.ships:
            self.set_ship_timer(u.SHIP_EMERGENCE_TIME)
        return self.score

    def check_collisions(self):
        self.check_individual_collisions(self.ships, self.asteroids)
        self.check_individual_collisions(self.asteroids, self.missiles)
        self.check_individual_collisions(self.ships, self.missiles)

Still green. Now move check_collisions over to Collider.

    def check_collisions(self):
        self.game.check_individual_collisions(self.ships, self.asteroids)
        self.game.check_individual_collisions(self.asteroids, self.missiles)
        self.game.check_individual_collisions(self.ships, self.missiles)
        return self.game.score

Green and good. I should be committing this, that’s the point of doing it step by step. Commit: collisions done in Collider, with help from Game.

Now implement check_individual_collisions locally, to defer. Could have done that last time I suppose.

    def check_collisions(self):
        self.check_individual_collisions(self.ships, self.asteroids)
        self.check_individual_collisions(self.asteroids, self.missiles)
        self.check_individual_collisions(self.ships, self.missiles)
        return self.game.score

    def check_individual_collisions(self, attackers, targets):
        self.game.check_individual_collisions(attackers, targets)

Commit: refactor to be ready for move.

Move the code:

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

    def mutual_destruction(self, target, targets, attacker, attackers):
        return self.game.mutual_destruction(target, targets, attacker, attackers)

Green. Commit: Move check_individual_collisions to Collider, defer mutual_destruction.

OK, move mutual destruction. This will break a score test I think. Well, I can make it keep running.

I moved two methods, getting impatient:

    def mutual_destruction(self, target, targets, attacker, attackers):
        if self.within_range(target, attacker):
            self.score += target.score_against(attacker)
            self.score += attacker.score_against(target)
            attacker.destroyed_by(target, attackers)
            target.destroyed_by(attacker, targets)
            return True
        else:
            return False

    def within_range(self, target, attacker):
        in_range = target.radius + attacker.radius
        dist = target.position.distance_to(attacker.position)
        return dist <= in_range

And I init self.score to zero, and return it from here:

    def check_collisions(self):
        self.check_individual_collisions(self.ships, self.asteroids)
        self.check_individual_collisions(self.asteroids, self.missiles)
        self.check_individual_collisions(self.ships, self.missiles)
        return self.score

Test break, but here’s why:

    def process_collisions(self):
        collider = Collider(asteroids=self.asteroids, missiles=self.missiles, saucers=[], ships=self.ships, game=self)
        collider.check_collisions()
        if not self.ships:
            self.set_ship_timer(u.SHIP_EMERGENCE_TIME)
        return self.score

We need to collect the score from collider, and add it to our score. And I don’t think we should have to return it but maybe we do.

    def process_collisions(self):
        collider = Collider(asteroids=self.asteroids, missiles=self.missiles, saucers=[], ships=self.ships, game=self)
        self.score += collider.check_collisions()
        if not self.ships:
            self.set_ship_timer(u.SHIP_EMERGENCE_TIME)
        return self.score

The game works, but five tests do not. And the only call to process_collisions is in the game loop, so that’s not the issue. Let’s see what the tests are doing that need improvement. I’m a little disappointed, I thought I had this one dead to rights.

    def test_missile_asteroid_scores(self):
        game = Game(True)
        pos = Vector2(100, 100)
        asteroid = Asteroid(2, pos)
        asteroids = [asteroid]
        missile = Missile(pos, Vector2(0, 0))
        missiles = [missile]
        game.mutual_destruction(asteroid, asteroids, missile, missiles)
        assert not missiles
        assert game.score == 20
        assert len(asteroids) == 2

There are a few of these and they just can’t work that way any more. We need to test a Collider here, not Game.

    def test_missile_asteroid_scores(self):
        pos = Vector2(100, 100)
        asteroid = Asteroid(2, pos)
        asteroids = [asteroid]
        missile = Missile(pos, Vector2(0, 0))
        missiles = [missile]
        collider = Collider(asteroids, missiles, [], [], None)
        collider.mutual_destruction(asteroid, asteroids, missile, missiles)
        assert not missiles
        assert collider.score == 20
        assert len(asteroids) == 2

I had to pass in None for the game parameter, but we don’t need it any more. Change Signature.

class Collider:
    def __init__(self, asteroids, missiles, saucers, ships):
        self.asteroids = asteroids
        self.missiles = missiles
        self.saucers = saucers
        self.ships = ships
        self.score = 0

Change a few more tests. Four still failing, the one changed works now. Three others were similar to that one. I think we want to change mutual_destruction signature but first the broken test:

    def test_collider_with_score(self):
        game = Game(True)
        asteroid = Asteroid(2, Vector2(100, 100))
        game.asteroids=[asteroid]
        missile = Missile(Vector2(100, 100), Vector2(3, 3))
        game.missiles=[missile]
        collider = Collider(asteroids=game.asteroids, missiles=game.missiles, saucers=[], ships=[])
        score = collider.check_collisions()
        assert score == 20
        assert game.score == 20

This is failing on the game score, which is not set and should not be, unless we were to call process_collisions on game. Let’s do that, closing the loop.

    def test_collider_via_game_with_score(self):
        game = Game(True)
        asteroid = Asteroid(2, Vector2(100, 100))
        game.asteroids=[asteroid]
        missile = Missile(Vector2(100, 100), Vector2(3, 3))
        game.missiles=[missile]
        game.process_collisions()
        assert game.score == 20

We’re green. Let’s commit: Collider now handling collisions for game.

Now we review collider and clean it up:

    def __init__(self, asteroids, missiles, saucers, ships):
        self.asteroids = asteroids
        self.missiles = missiles
        self.saucers = saucers
        self.ships = ships
        self.score = 0

    def check_collisions(self):
        self.check_individual_collisions(self.ships, self.asteroids)
        self.check_individual_collisions(self.asteroids, self.missiles)
        self.check_individual_collisions(self.ships, self.missiles)
        return self.score

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

    def mutual_destruction(self, target, targets, attacker, attackers):
        if self.within_range(target, attacker):
            self.score += target.score_against(attacker)
            self.score += attacker.score_against(target)
            attacker.destroyed_by(target, attackers)
            target.destroyed_by(attacker, targets)
            return True
        else:
            return False

    def within_range(self, target, attacker):
        in_range = target.radius + attacker.radius
        dist = target.position.distance_to(attacker.position)
        return dist <= in_range

Ah. I was thinking that maybe I could use the members in some of the lower-down methods but that’s not the case. I think this is just about what we need.

Python wants to make within_range a static method. I don’t think I’ve tried that before and stuck with it. Let’s try it.

    @staticmethod
    def within_range(target, attacker):
        in_range = target.radius + attacker.radius
        dist = target.position.distance_to(attacker.position)
        return dist <= in_range

Remove the self and it’s happy. So am I. Commit: make method static.

I think we’re done here. We’ve removed a responsibility from the game and given it to the Collider which is a nice little single-purpose object.

I call that progress. See you next time!