Python Asteroids on GitHub

OK, let’s do collisions differently and see what we think.

The game selects groups of objects that may or may not be colliding and checks them pairwise, asteroids vs missiles, ships vs missiles, ships vs asteroids, etc. The work is done in the objects, two of which have a collide_with_attacker method:

class Asteroid ...
    def collide_with_attacker(self, attacker, attackers, asteroids):
        if self.within_range(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)

I think I want to do this another way. Rather than have the objects deciding whether they are or are not within range, I’ll have the decision made at the game level and then inform the individual objects that they are moribund and to deal with it. I plan to have every space object respond to a message telling it that it is colliding, providing the object’s owner collection, and the other object it’s colliding with. At that level, the colliding object will remove itself from the collection and may or may not add new objects in. For now, only the asteroid will ever add, but we do need explosions.

I will try not to have to tell the moribund object the type of its nemesis, but the objects will be able to send messages to the nemesis if need be. And I think the asteroids, at least, will need to.

We’ll see where this leads us. It seems likely that this may not get us to a perfect solution, and some possibility exists that we won’t like it at all. I do expect to like it, or I wouldn’t try it.

Let’s begin by sending the you-have-collided message from the methods above, which will put them in place for use as we do the upper-level work. Let’s call the message … destroyed_by.

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

    def destroyed_by(self, attacker, ships):
        if self in ships: ships.remove(self)

And then for Asteroid …

class Asteroid ...
    def collide_with_attacker(self, attacker, attackers, ships):
        if self.within_range(attacker.position, attacker.radius):
            attacker.destroyed_by(self, attackers)
            self.destroyed_by(attacker, ships)

    def destroyed_by(self, attacker, ships):
        if self in ships: ships.remove(self)

And I think missile needs the new method and perhaps even Saucer, depending on how the tests are working.

class Missile ...
    def destroyed_by(self, attacker, missiles):
        if self in missiles: missiles.remove(self)

I have two tests auto-running red. Let’s see what they are.

This one is scoring zero, not twenty:

    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

These tests are going to need to be fixed anyway, because the collide_with will go away, but what has gone wrong? We know that missiles was emptied. Missile instances do have the score list. Ah. I didn’t put the scoring code in yet. That definitely explains that.

class Asteroid ...
    def destroyed_by(self, attacker, ships):
        u.score += attacker.score_list[self.size]
        if self in ships: ships.remove(self)

That test is running. The other relates to Saucer. PyTest tells me just what’s wrong. I like this PyCharm tool:

    def collide_with_attacker(self, attacker, attackers, ships):
        if self.within_range(attacker.position, attacker.radius):
>           attacker.destroyed_by(self, attackers)
E           AttributeError: 'Saucer' object has no attribute 'destroyed_by'

True enough. And easy enough to fix:

class Saucer ...
    def destroyed_by(self, attacker, saucers):
        if self in saucers: saucers.remove(self)

The tests are green. Commit: implement new destroyed_by method on all space objects.

Now then, I want to move some of this logic up to the game level.

My Cunning Plan

Roughly, what I intend is that the game level will have a method that takes pairs of collections to be collided. It will loop over the pairs with the same kind of loop we have now, including the break. It will call another method that checks a pair for being within striking range, and if they are, calls destroyed_by on each of them.

It would be nice to be able to do some or all of this in small steps, and to have PyCharm do as much as possible with its refactoring steps, which are “guaranteed” to be correct.

We have these two functions now:

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


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

Clearly we want to get rid of the second one and this scheme’s main point is to do the first one generally enough to do all the combinations.

What if we were to inline collide_with_attacker? PyCharm can’t do it, there are two versions of the method and it doesn’t know which one to use.

I think I’ll have to code this.

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

We don’t have within_range, but we can have it:

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

Tests are green. Let me try the game. Not quite. Asteroids do not split. Let me extract a method here and test it.

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


def mutual_destruction(target, targets, attacker, attackers):
    if within_range(target, attacker):
        attacker.destroyed_by(target, attackers)
        target.destroyed_by(attacker, targets)

I’ll make sure this still doesn’t work. And if I don’t then see the bug quickly, I’ll roll back anyway.

OK, let’s write a new test here. We have this test:

    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

We’re going to get rid of the collide_with_attacker methods, so let’s repurpose this test right now.

    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]
        main.mutual_destruction(asteroid, asteroids, missile, missiles)
        assert not missiles
        assert u.score == 20

Curiously, the test runs. Why doesn’t the game work? And why don’t we have a test showing that it doesn’t work? Oh. I know. This:

    def destroyed_by(self, attacker, asteroids):
        u.score += attacker.score_list[self.size]
        if self in asteroids: asteroids.remove(self)

I must have copied that function over. It used to say ships as the collection parameter. And it should call split_or_die:

    def destroyed_by(self, attacker, asteroids):
        u.score += attacker.score_list[self.size]
        self.split_or_die(asteroids)

I think this will work and I can improve my test. It does work, and:

    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]
        main.mutual_destruction(asteroid, asteroids, missile, missiles)
        assert not missiles
        assert u.score == 20
        assert len(asteroids) == 2

Now I think I can use our new function for all the cases.

def check_ship_vs_asteroids():
    check_individual_collisions(ships, asteroids)

We’re green. Lets inline those three calls in check_collisions:

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

Now we can remove those collide_with methods. There are callers, in the tests. Fix those:

    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]
        main.mutual_destruction(asteroid, asteroids, missile, missiles)
        assert not missiles
        assert u.score == 20
        assert len(asteroids) == 2

    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]
        main.mutual_destruction(ship, ships, missile, missiles)
        assert not missiles
        assert not ships
        assert u.score == 0

    def test_asteroid_ship_does_not_score(self):
        u.score = 0
        pos = Vector2(100, 100)
        asteroid = Asteroid(2, pos)
        print("position", asteroid.position)
        asteroids = [asteroid]
        ship = Ship(pos)
        ships = [ship]
        main.mutual_destruction(asteroid, asteroids, ship, ships)
        assert not ships
        assert u.score == 0

I expect those methods to be removable now. They are. We are green, game works. Commit: convert to game-level collision checking, destroyed_by sent to all colliders.

Let’s review. Collision is now handled the same for all groups that can collide:

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


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


def mutual_destruction(target, targets, attacker, attackers):
    if within_range(target, attacker):
        attacker.destroyed_by(target, attackers)

The individual objects deal with their own demise, like this:

class Asteroid ...
    def destroyed_by(self, attacker, asteroids):
        u.score += attacker.score_list[self.size]
        self.split_or_die(asteroids)

class Missile ...        
    def destroyed_by(self, attacker, missiles):
        if self in missiles: missiles.remove(self)

class Saucer ...
    def destroyed_by(self, attacker, saucers):
        if self in saucers: saucers.remove(self)

class Ship ...
    def destroyed_by(self, attacker, ships):
        if self in ships: ships.remove(self)

Three of these are the same, but I don’t quite see a way that I like to deal with that. We could perhaps have the upper guy call special_action_before_destructin, and three of them ignore the message and one, Asteroid, does something, but that doesn’t reduce duplication.

We could do an inheritance trick, but that’s a waste of a high card. We could invent delegation and have two classes and two methods instead of what we have now.

I don’t see a better way.

What I do like is that, unlike some other Asteroids designs I’ve done, I have no Transaction object and no reaching up from the space objects to find their collections. Instead, their collection is passed to them when they need it. This is the way.

This is definitely better than it was, and we observe that we set up for this last time, making some small changes / improvements that shined the light on this opportunity. Then, rather than go after it while tired, we waited until a fresh session.

And one lesson learned for sure: copy-paste is fraught for this author. I seem to make mistakes almost every time I do it, and PyCharm’s limitations (or my failure to know how to get it to do what it can) leads me to more copy-paste than would be ideal for me. I hope you’re more clever, more careful and notice more things than I do.

I say lesson learned. That doesn’t mean that I’ll change, but I’ll try, if I can think of a way to do better. I am perfectly willing to let you see my mistakes, in case it’s useful to you, but I hate to be quite as good at making mistakes as I appear to be.

See you next time!


Jira:

  • 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)