Python Asteroids on GitHub

As I move toward the decentralized model, I need some thinking about next steps. But first, an interesting test.

In the Kotlin decentralized version, the program uses a Transaction object to accumulate the removals and additions until the interaction loop is over, because removing things from a collection while it is being iterated is generally not a good thing. It tends to get items skipped over.

It seems likely to me that in this version we’ll use the itertools.combinations iterator, which can generate all pairs (or triples or quints …) of a collection. So I was wondering what happens if you remove items from the collection while the iterator is running. I wrote a test:

    def test_combinations_handles_delete(self):
        # combinations seems to create a protected tuple
        numbers = [1, 2, 3]
        total = 0
        for a,b in itertools.combinations(numbers, 2):
            total = total + a + b
        assert total == 12
        total = 0
        for a,b in itertools.combinations(numbers, 2):
            if a in numbers: numbers.remove(a)
            if b in numbers: numbers.remove(b)
            total = total + a + b
        assert total == 12
        assert not numbers

The first phase there just iterates all pairs, and, no surprise, gets a total of 12. The second phase removes values as soon as they are seen, and still gets a total of twelve! And, just to be sure nothing funky was going on, we check that the list is empty at the end.

So this is good news, in that it means we probably won’t have to do anything tricky to allow changes to our fleet. Any changes we make will automatically be buffered until the next time around.

However.

This does mean that there is a small possibility of an object being used twice when it should only be used once. I have seen it happen in the game: suppose a missile happens to come into range of two asteroids on the same tick. Very unlikely, but possible. As written, the code now will destroy one and not the other. Under the new scheme, unless we do something special, we might destroy them both.

It’s good to have things to think about, I guess. Should I make a Jira for it? Sure, why not? “Double Hits??”

Anyway, that’s just a bit of learning, because I was thinking about it and wanted to know. But what are we going to do today?

Status and Plan

All five of our current “flyers” implement the interact_with protocol, sending themselves, their type, and the fleets to the other guy, such as attacker.interact_with_asteroid(self, fleets). In each case, the current code assumes that the flyers are in range for a kill, so if they are mutually destructive, each one removes itself:

class Asteroid:
    def interact_with_missile(self, missile, fleets):
        self.split_or_die(fleets)

class Missile:
    def interact_with_asteroid(self, asteroid, fleets):
        self.die(fleets)

When this new scheme is done, every object will interact with every other, so these methods will need to check to see whether they’re within kill range or not, ignoring the interaction if they’re not.

Furthermore, if they are in kill range, then it may be necessary to register the score. And here’s where it gets tricky to refactor from here to there.

In the new scheme, an object announces a score by putting a Score object into the mix, holding the number of points scored. And during the next interactions cycle, a ScoreKeeper object interacts withe Score, and adds the points to the field that it displays during draw. Meanwhile Score interacts with ScoreKeeper and removes itself, ensuring that the score is counted once and only once.

Here’s my concern. I can’t move scoring until the Score and ScoreKeeper are in the mix, and as things stand, we do scoring inside Game. It looks like this:

class Collider:
    def check_collisions(self):
        for pair in itertools.combinations(self.fleets.colliding_fleets, 2):
            self.check_individual_collisions(pair[0], pair[1])
        return self.score

    def check_individual_collisions(self, attackers, targets):
        for target in targets:
            for attacker in attackers:
                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_for_hitting(attacker)
            self.score += attacker.score_for_hitting(target)
            attacker.interact_with(target, attackers, self.fleets)
            target.interact_with(attacker, targets, self.fleets)
            return True
        else:
            return False

Unwinding this code needs to be done in a way that won’t break anything, becuase the rules are that I have to commit my code frequently, and I have to keep the game working. (And it would be a cop-out to put in a feature flag, though I’d do it if I really needed to.)

So I need a plan. How about this:

  1. Cause the flyers to check range and only remove themselves if in range, duplicating the check that the Collider is making.
  2. Move the interact_with calls above outside the within_range check, so that they are called for all combinations.
  3. A couple of possibilities for next step: a. Change over to interacting everything with everything, leaving scoring where it is, move from there, not sure how … b. Add temporary code to Collider to iterate other objects, add Score and ScoreKeeper code, and when it works, remove scoring from Collider.

Having written that plan, it helps me see a possibility, which is that we can figure out right now how the asteroid and missile interaction could at least know the score even if we can’t apply it yet.

As things stand now:

class Asteroid:
    def score_for_hitting(self, attacker):
        return attacker.scores_for_hitting_asteroid()[self.size]

However we do it, we need to combine the scoring table in the missile (or other flyer) and the asteroid size. Every flyer responds to that message:

class Asteroid:
    @staticmethod
    def scores_for_hitting_asteroid():
        return [0, 0, 0]

class Missile:
    def scores_for_hitting_asteroid(self):
        return self.score_list

class Saucer:
    @staticmethod
    def scores_for_hitting_asteroid():
        return [0, 0, 0]

class Ship:
    @staticmethod
    def scores_for_hitting_asteroid():
        return [0, 0, 0]

The above code may strike some readers as odd. “Couldn’t we just check the types of colliding objects and either score or not?”

Yes, we could. That is not how we roll. We do not insert conditional code checking types if we can possibly avoid it. Instead, we program so that we can send the message we’re interested in to anyone, and they all respond in a reasonable way. This produces code that is less procedural, which is good, and it produces code where we sometimes have to look at another object to find out what is going to happen.

But generally, we don’t care. The code above says get the scores table and pick the element for our size. We know, as the human, that if an asteroid collides with a saucer, the answer will be zero. We generally won’t care how it happens, so we move on. Only when we care, do we look.

So, maybe it’s odd, but it’s how we roll, where we isn’t just me but most of the really good OO programmers I know. I learned it from them, and on some days, I’m pretty good at it.

Weren’t we planning?

Yes. So here’s how much of the plan I almost believe now:

  1. Change the flyer interactions to check range; Expect duplication, expect to figure out a way to reduce it;
  2. Once they’re checking range, add code to get the score;
  3. Create a Score object;
  4. Send the Score to fleets via add_score;
  5. For now, Fleets may ignore this, but in due time it’ll add it to the mix;

Something like that, anyway.

Do It

Let’s check range in Asteroid vs Missile:

    def interact_with_missile(self, missile, fleets):
        kill_range = self.radius + missile.radius
        dist = self.position.distance_to(missile.position)
        if dist <= kill_range:
            self.split_or_die(fleets)

This method is getting pretty handsy with Demeter, pulling bits out of itself and the other object, but it’s passing the tests. If I change the if so that it doesn’t trigger, the test_missile_asteroid_scores test fails.

I think this is good. Commit: Asteroid checks kill range on interaction with missile.

Now what could we do about checking range so that we don’t have to put these two weird lines in every object?

Let’s at least extract a bit. First extract variable:

    def interact_with_missile(self, missile, fleets):
        kill_range = self.radius + missile.radius
        dist = self.position.distance_to(missile.position)
        in_range = dist <= kill_range
        if in_range:
            self.split_or_die(fleets)

Then extract method:

    def interact_with_missile(self, missile, fleets):
        in_range = self.in_range(missile)
        if in_range:
            self.split_or_die(fleets)

    def in_range(self, missile):
        kill_range = self.radius + missile.radius
        dist = self.position.distance_to(missile.position)
        in_range = dist <= kill_range
        return in_range

Then in line twice:

    def interact_with_missile(self, missile, fleets):
        if self.in_range(missile):
            self.split_or_die(fleets)

    def in_range(self, missile):
        kill_range = self.radius + missile.radius
        dist = self.position.distance_to(missile.position)
        return dist <= kill_range

Commit: refactor to create in_range method.

Now as things stand we’re going to duplicate that method into every other class that needs range checking. I’m not fond of that but right now I don’t have a better idea. Possibly worse ideas include:

  • Create a top-level function that anyone can call;
  • Inherit the method from a concrete superclass
  • Create an ephemeral RangeFinder object and use it every time;
  • Cache a RangeFinder in every class and use it;

By the time we get to the last one we might save one line and add a member variable to all our flyers. We’ll let this be for now.

Reflection and Planning

OK, we have one side of the asteroid-missile family checking distance. We can change the Asteroid to do the checking everywhere its needed. We can do the same for Missile … and for Saucer and Ship …

Oh, and I did want to see how to access score. Let’s just spike that in to see that we can get it.

    def interact_with_missile(self, missile, fleets):
        if self.in_range(missile):
            print(self.score_for_hitting(missile))
            self.split_or_die(fleets)

That prints the right values. Let’s put in add_score right now. Ah, no, we can’t, quite. We’d need the Score class and … oh heck, let’s do it. Are we going to create a lot of zero-score objects, or not? I think the OO way to do it is to create them.

class Score:
    def __init__(self, score):
        self.score = score

Now to use it:

    def interact_with_missile(self, missile, fleets):
        if self.in_range(missile):
            fleets.add_score(Score(self.score_for_hitting(missile)))
            self.split_or_die(fleets)

Things are failing, but Fleets isn’t au courant yet.

class Fleets:
    def add_score(self, score):
        pass

We’ll just ignore them until we’re ready. Commit: Asteroid issues Score object for missile collisions.

What else do we need to do with asteroid? Currently Collider is asking it for scores against everything it might collide with, ships and so on. In the new scheme, we know we only consider missiles, so this will be done less often. Interesting.

I think asteroid is done. Let’s modify our test for implementors to demand that everyone implement in_range.

    def test_double_dispatch_readiness(self):
        classes = [Asteroid, Missile, Saucer, Ship, Fragment]
        errors = []
        for klass in classes:
            attrs = dir(klass)
            methods = ["interact_with",
                       "interact_with_asteroid",
                       "interact_with_missile",
                       "interact_with_saucer",
                       "interact_with_ship",
                       "in_range"]
            for method in methods:
                if method not in attrs:
                    errors.append((klass.__name__, method))
        assert not errors

Generalize the one in Asteroid a bit:

class Asteroid:
    def in_range(self, other):
        kill_range = self.radius + other.radius
        dist = self.position.distance_to(other.position)
        return dist <= kill_range

Put the same into the other classes. I implement it as return False in Fragment: they don’t collide with anything. But wait. When two objects interact, normally each will ask if it is in_range of the other. It will ask for the other’s radius to do that. Instead, we should ask the other if it is in range of us. That will allow the Fragment to answer correctly that it isn’t.

We can do better with this. I’d really like to roll back a few commits right now, but that never happens. We move forward.

New Plan

For object x to find out if it is colliding with object u, it will send y.are_we_colliding(x.position, x.radius) and avoid the demeter stuff and allow for some objects to answer yes or no without checking the numbers.

I think we’ll roll back this last change moving in_range over back to the prior commit.

Now change this:

class Asteroid:

    def interact_with_missile(self, missile, fleets):
        if self.in_range(missile):
            fleets.add_score(Score(self.score_for_hitting(missile)))
            self.split_or_die(fleets)

    def in_range(self, missile):
        kill_range = self.radius + missile.radius
        dist = self.position.distance_to(missile.position)
        return dist <= kill_range

To ask the Missile

    def interact_with_missile(self, missile, fleets):
        if missile.are_we_colliding(self.position, self.radius):
            fleets.add_score(Score(self.score_for_hitting(missile)))
            self.split_or_die(fleets)

Missile does not yet know the answer. We provide:

class Missile:
    def are_we_colliding(self, position, radius):
        kill_range = self.radius + radius
        dist = self.position.distance_to(position)
        return dist <= kill_range

Now I think everyone needs to provide that method. So I’ll demand it in that test that demands people do things.

I put it everywhere, again returning False from Fragment.

Commit: in range now determined by asking the attacker are_we_colliding.

Why did you do that?

I did that because the existing in_range function required the other guy to implement radius and position, so people would be calling for those values from every object in the universe. That’s not really good design, even if we were sure we’d never ask anyone we shouldn’t. It’s better to provide our information to the other object, to use as it sees fit.

In the old way, another object asks us for radius and position. In the new way the code is provided a radius and position and uses only its own local information to fetch its own member variables. Better.

If this is not clear, please toot me up and I’ll try to explain it better.

What Now?

I’d like to complete Missile in the new style of local checking, partly because we’ve done the one side and doing the other just seems right, and partly because a second implementation will verify that we have a good scheme in place.

It should be easy, there’s no scoring to do.

class Missile:
    def interact_with_asteroid(self, asteroid, fleets):
        self.die(fleets)

That becomes:

    def interact_with_asteroid(self, asteroid, fleets):
        if asteroid.are_we_colliding(self.position, self.radius):
            self.die(fleets)

Green, good. Commit: missile checks collision with asteroid.

All its interactions …

While we’re here, why not change all the interactions?

class Missile:
    def interact_with_saucer(self, saucer, fleets):
        if saucer.are_we_colliding(self.position, self.radius):
            self.die(fleets)

    def interact_with_ship(self, ship, fleets):
        if ship.are_we_colliding(self.position, self.radius):
            self.die(fleets)

Green, good. Commit: missile checks collision in all cases.

Now pick up Asteroid …

Let’s go back and do that in Asteroid.

class Asteroid:
    def interact_with_saucer(self, saucer, fleets):
        if saucer.are_we_colliding(self.position, self.radius):
            self.split_or_die(fleets)

    def interact_with_ship(self, ship, fleets):
        if ship.are_we_colliding(self.position, self.radius):
            self.split_or_die(fleets)

Green, good. Commit: asteroid checks collision in all cases.

And Saucer ….

OK, moving along to saucer

class Saucer:
    def interact_with_asteroid(self, asteroid, fleets):
        if asteroid.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

    def interact_with_missile(self, missile, fleets):
        if missile.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

    def interact_with_saucer(self, saucer, fleets):
        pass

    def interact_with_ship(self, ship, fleets):
        if ship.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

Commit: saucer checks collision in all cases.

Finally Ship …

Now ship:

class Ship:
    def interact_with_asteroid(self, asteroid, fleets):
        if asteroid.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

    def interact_with_missile(self, missile, fleets):
        if missile.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

    def interact_with_saucer(self, saucer, fleets):
        if saucer.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

Commit: ship checks collisions in all cases.

Collider can stop checking before interacting!

I think I can now stop suppressing the interaction calls in Collider, changing this:

class Collider:
    def mutual_destruction(self, target, targets, attacker, attackers):
        if self.within_range(target, attacker):
            self.score += target.score_for_hitting(attacker)
            self.score += attacker.score_for_hitting(target)
            attacker.interact_with(target, attackers, self.fleets)
            target.interact_with(attacker, targets, self.fleets)
            return True
        else:
            return False

To this:

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

This calls for a test in the game, it’s a major change.

Everything works perfectly. Callooh Callay!

Commit: Collider now interacts all objects whether in range or not.

Let’s sum up!

Summary!

This is a major step forward, much further than I had thought we’d get today. The big deal is that now all our flyers are interacting with each other, mostly harmlessly, but processing collisions when they are in range. This means that Game and Collider don’t need know that objects even do collide, they just know that they interact.

We should rename some methods in Collider to reflect that fact.

The changes we made were all small and easy. I did make one set of changes and then roll them back, because putting the change into Fragment told me that there was a better way. As things stand, each object involved in determining whether it’s colliding refers only to its own members and to parameters it is given, rather than ripping the guts out of the other guy. That sets the stage for objects that have no position or radius at all.

The Collider is still asking for scores, and is checking range for that, but we’ve shown that we can get the score as needed, and in one case we are actually passing it back to Fleets, where it is being ignored for now.

There are still quite a few steps to go before we’re fully decentralized, including the creation of a number of new objects such as WaveChecker, ShipChecker, ShipMaker, WaveMaker, ScoreKeeper, HyperspaceExploderator, and so on. But our main objects, Asteroid, Missile, Saucer, and Ship, are well on the way to operating independently of the Game and Collider.

Will we get rid of Collider entirely? Maybe, maybe not. It will certainly be simpler and know less than it does now, but we might retain it just in support of good separation of concerns.

I am pleasantly surprised to be this far along.

See you next time!