Python Asteroids on GitHub

I think I’ll start by counting missiles in the Ship and probably Saucer. Then we’ll see what’s next. The thing is to keep stepping toward the larger goal. Some interesting [mis-]steps.

One hundred articles just on the Python version of Asteroids. What kind of madman would do a thing like that? /me waves: “Hi!”

I’m pretty sure that Ship and Saucer will have to count missiles during interactions, which will be easy since they both already interact with missiles in a sort of negative way. (Boom!) Let’s review how Ship firing works first, just as a bit of orientation.

In action order, it goes like this:

class Ship(Flyer):
    def tick(self, delta_time, fleet, fleets):
        self.control_motion(delta_time, fleet, fleets)
        self.move(delta_time, fleet)

    def control_motion(self, delta_time, fleet, fleets):
        ...
        if keys[pygame.K_k]:
            self.fire_if_possible(fleets.missiles)
        ...

    def fire_if_possible(self, missiles):
        if self._can_fire and missiles.fire(self.create_missile):
            self._can_fire = False

class MissileFleet(Fleet):
    def fire(self, callback, *args) -> bool:
        if len(self) < self.maximum_number_of_missiles:
            self.append(callback(*args))
            return True
        else:
            return False

class Ship(Flyer):
    def create_missile(self):
        player.play("fire", self._location)
        return Missile.from_ship(self.missile_start(), self.missile_velocity())

We’ve deferred the decision on firing over to the MissileFleet. We plan to get rid of all the specialized fleets, so that’ll never do, but we see that MissileFleet is primed to know the maximum number of missiles (that the ship or saucer can fire) and to do the firing only if the current count is smaller.

Ship only toggles _can_fire when it does in fact fire. I think that means that if you press and hold down the “k” key after firing four missiles, it will fire one as soon as one dies.

That’s interesting enough that I try it. Yes. If I tap K quickly five times and hold it down, four missiles shoot out rapidly, then nothing happens until one of the missiles dies, and then one more missile fires. Fine, works for me. You still have to lift and re-tap to fire the next time.

OK, I don’t see this with total clarity but it seems to me that counting the ship missiles in Ship will let us move that logic on board.

We should probably think then about putting the firing logic into its own little object, instead of letting it be a responsibility of Ship itself. That might even help us avoid duplication in Saucer.

One thing at a time, I think.

It’s so tempting … SO TEMPTING … to create a FireControl object right now and plug it into Ship. I think it’s kind of a big step, though.

Let’s TDD it and see how it feels.

class TestFireControl:
    def test_class(self):
        FireControl(4)

I think this is more than enough to drive out the initial class. The 4 is the number of missiles the ship can fire. I’m already planning too far ahead, assuming that the saucer will use this object, saying 2.

Anyway we need the class. I’ll build it in the test file and move it when it done. Editing is easier that way, when things are small.

class FireControl:
    def __init__(self, number_of_missiles):
        self.number_of_missiles = number_of_missiles

Green. Should we commit this? Sure, it’s a good habit and it’s easily removable until we start using it. Commit: Initial FireControl and test.

Now how should it work? It’s not really a Flyer, because the Ship will own it, but it does want to count missiles. Of course, there’s nothing stopping us from making it a Flyer and having the Ship have a reference to it.

Too weird. We’ll drive it from Ship’s begin, interact_, end, but it’ll be subordinate to Ship.

    def test_can_fire(self):
        fc = FireControl(4)
        fc.begin_tally()
        fc.tally()
        assert fc.can_fire
        fc.tally()
        assert fc.can_fire
        fc.tally()
        assert fc.can_fire
        fc.tally()
        assert not fc.can_fire

Should be easy.

class FireControl:
    def __init__(self, number_of_missiles):
        self.number_of_missiles = number_of_missiles
        self.missile_tally = 0

    def begin_tally(self):
        self.missile_tally = 0

    def tally(self):
        self.missile_tally += 1

    @property
    def can_fire(self):
        return self.missile_tally < self.number_of_missiles

Green. Commit: initial tally logic.

Now we have an interesting question … as written, the caller is responsible for knowing whether to tally the missile: the FireControl never sees it. Another possibility is to have FireControl know how to determine its own missiles. For now … we’ll leave it this way and see how it looks in place. We can always change it as we learn.

We are in the business of changing code, as GeePaw Hill so often tells us.

Note that the current scheme has a callback:

class Ship(Flyer):
    def create_missile(self):
        player.play("fire", self._location)
        return Missile.from_ship(self.missile_start(), self.missile_velocity())

We do this via callback, because we only want to create the Missile object if in fact we’re going to fire.

Let’s do it the same way with FireControl. We’re just using it as a counter.

Meh. If we’re just using it as a tricky way to count to 4, the object is useless.

We have learned something. Perhaps several things. Don’t worry, I’ll forget them soon.

  1. Creating an object on spec is risky, as it may not turn out to be as useful as we imagine;
  2. There is an easier way to count to four;
  3. Starting with the need might be better than starting with the solution.

No harm done, we’ve definitely learned how to count to four.

I’ll remove this whole file and work on Ship. We’ll see about duplication when we have some. Done. Safe Delete. Commit: remove FireControl experiment.

Now then let’s see if we can TDD the ship to count missiles. But wait. Why not make it easier to tell the missiles apart. We have this:

class Missile(Flyer):
    @classmethod
    def from_ship(cls, position, velocity):
        return cls(position, velocity, u.MISSILE_SCORE_LIST, u.SAUCER_SCORE_LIST)

    @classmethod
    def from_saucer(cls, position, velocity):
        return cls(position, velocity, [0, 0, 0], [0, 0])

    def __init__(self, position, velocity, missile_score_list, saucer_score_list):
        self.score_list = missile_score_list
        self.radius = 2
        self._timer = Timer(u.MISSILE_LIFETIME, self.timeout)
        self._saucer_score_list = saucer_score_list
        self._location = MovableLocation(position, velocity)

Let’s implement two new methods. I won’t TDD these, because they are trivial. That’s my story and I’m going to stick to it until it becomes obvious that I’m wrong.

    def __init__(self, position, velocity, missile_score_list, saucer_score_list):
        self.score_list = missile_score_list
        if missile_score_list[0] == 0:
            self.is_ship_missile = False
            self.is_saucer_missile = True
        else
            self.is_ship_missile = True
            self.is_saucer_missile = False
        self.radius = 2
        self._timer = Timer(u.MISSILE_LIFETIME, self.timeout)
        self._saucer_score_list = saucer_score_list
        self._location = MovableLocation(position, velocity)

Commit: missiles know is_ship_missile and is_saucer_missile

I just committed code with an error in it. No colon after else. Fixed.

So one paragraph later shows me wrong. If I’d had a test it would have failed to compile.

Write the test.

    def test_missiles_know_type(self):
        ship_missile = Missile.from_ship(u.CENTER, Vector2(0, 0))
        assert ship_missile.is_ship_missile
        assert not ship_missile.is_saucer_missile
        saucer_missile = Missile.from_saucer(u.CENTER, Vector2(0, 0))
        assert not saucer_missile.is_ship_missile
        assert saucer_missile.is_saucer_missile

Commit: posthumous test for is_saucer_missile and is_ship_missile.

See what happens? Even the simplest change benefits from a test.

OK, now let’s test counting ship missiles in ship. Having been burned by not testing “trivial” code, I’m on my good behavior for a while.

    def test_ship_counts_missiles(self):
        fleets = Fleets()
        ship = Ship(u.CENTER)
        ship.begin_interactions(fleets)
        m_ship = Missile.from_ship(Vector2(0, 0), Vector2(0, 0))
        m_saucer = Missile.from_saucer(Vector2(0, 0), Vector2(0, 0))
        ship.interact_with_missile(m_ship, fleets)
        ship.interact_with_missile(m_saucer, fleets)
        assert ship._missile_tally == 1

I’m positing a “private” variable for the tally.

In Ship:

    def begin_interactions(self, fleets):
        self._missile_tally = 0

    def interact_with_missile(self, missile, fleets):
        if missile.is_ship_missile:
            self._missile_tally += 1
        if missile.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

Test is green. Commit: ship tallies ship missiles for firing purposes.

Now I think we should be able to recast the firing logic. Let’s review it:

class Ship(Flyer):
    def fire_if_possible(self, missiles):
        if self._can_fire and missiles.fire(self.create_missile):
            self._can_fire = False

We have three tests for this, so I’m going to recode it and see if they tell me I’ve done the right thing. I suspect we’ll have to recast them a bit.

Here’s the issue. I need access to Fleets to fire a missile. I do have a missiles parameter, which, strictly speaking, I could add to. I do this:

    def fire_if_possible(self, missiles):
        if self._can_fire and self._missile_tally < 4:
            missiles.append(self.create_missile())
            self._can_fire = False

One of the tests breaks.


    def test_firing_limit(self):
        ship = Ship(u.CENTER)
        count = 0
        missiles = MissileFleet([], u.MISSILE_LIMIT)
        while len(missiles) < u.MISSILE_LIMIT:
            ship._can_fire = True
            ship.fire_if_possible(missiles)
            count += 1
            assert len(missiles) == count
        assert len(missiles) == u.MISSILE_LIMIT
        ship._can_fire = True
        ship.fire_if_possible(missiles)
>       assert len(missiles) == u.MISSILE_LIMIT
E       assert 5 == 4
E        +  where 5 = len(<fleet.MissileFleet object at 0x103acff10>)
E        +  and   4 = u.MISSILE_LIMIT

We’re not using the length of missiles in Ship any more, so this code isn’t following the real protocol.

OK, redo it. But I’m also getting this message:

missing sound fire
missing sound fire
missing sound fire
missing sound fire
missing sound fire

That’s only in the tests, so it’s probably OK.

How can we best recast this test:

    def test_firing_limit(self):
        ship = Ship(u.CENTER)
        count = 0
        missiles = MissileFleet([], u.MISSILE_LIMIT)
        while len(missiles) < u.MISSILE_LIMIT:
            ship._can_fire = True
            ship.fire_if_possible(missiles)
            count += 1
            assert len(missiles) == count
        assert len(missiles) == u.MISSILE_LIMIT
        ship._can_fire = True
        ship.fire_if_possible(missiles)
        assert len(missiles) == u.MISSILE_LIMIT

Let’s imagine that we really want to count all the missiles, all the way down. And let’s remember that we’re moving toward no specialized fleets.

I go this far:

    def test_firing_limit(self):
        ship = Ship(u.CENTER)
        count = 0
        fleets = Fleets()
        for i in range(4):
            ship._can_fire = True
            for flyer in fleets.all_objects:
                flyer.interact_with(ship)
            ship.fire_if_possible(fleets)
        missile_count = len([m for m in fleets.all_objects])
        assert missile_count == u.MISSILE_LIMIT

I’ve changed the expected protocol for fire_if_possible to expect a Fleets. In the code:

    def fire_if_possible(self, missiles):
        if self._can_fire and self._missile_tally < 4:
            missiles.add_flyer(self.create_missile())
            self._can_fire = False

That’s where we want to wind up. Two tests now break.

    def test_missile_timeout(self):
        ship = Ship(Vector2(100, 100))
        missiles = MissileFleet([], 4)
        ship.fire_if_possible(missiles)
        assert len(missiles) == 1
        missile = missiles[0]
        missile.tick_timer(0.5, missiles)
        assert len(missiles) == 1
        missile.tick_timer(3.0, missiles)
        assert len(missiles) == 0

If I make a Fleets here this should work. But I don’t want to be tied to the MissileFleet at all.

Oh this is getting nasty. The new missile code works perfectly, but we’ve changed things so that the MissileFleet is not being used for ship missiles.

Let me get a decent test for the firing limit first. The current one won’t do.

OK this test is a crock, but it’s correct and running:

    def test_firing_limit(self):
        ship = Ship(u.CENTER)
        count = 0
        missile_count = 0
        fleets = Fleets()
        for i in range(5):
            ship._can_fire = True
            ship._missile_tally = 0
            for flyer in fleets.all_objects:
                flyer.interact_with(ship, fleets)
            ship.fire_if_possible(fleets)
            missile_count = len([m for m in fleets.all_objects])
        assert missile_count == u.MISSILE_LIMIT
        ship._can_fire = True
        ship._missile_tally = 0
        for flyer in fleets.all_objects:
            flyer.interact_with(ship, fleets)
        ship.fire_if_possible(fleets)
        missile_count = len([m for m in fleets.all_objects])
        assert missile_count == u.MISSILE_LIMIT

Let’s extract that common code.

    def test_firing_limit(self):
        ship = Ship(u.CENTER)
        count = 0
        missile_count = 0
        fleets = Fleets()
        for i in range(5):
            missile_count = self.attempt_fire(fleets, missile_count, ship)
        assert missile_count == u.MISSILE_LIMIT
        missile_count = self.attempt_fire(fleets, missile_count, ship)
        assert missile_count == u.MISSILE_LIMIT

    @staticmethod
    def attempt_fire(fleets, missile_count, ship):
        ship._can_fire = True
        ship._missile_tally = 0
        for flyer in fleets.all_objects:
            flyer.interact_with(ship, fleets)
        ship.fire_if_possible(fleets)
        missile_count = len([m for m in fleets.all_objects])
        return missile_count

OK, that’s better. Now let’s turn back to the other broken test, which is just trying to determine that missiles can time out.

I am slightly curious as to how this is working. In Ship.fire_if_possible, we add the missile with add_flyer, which puts it in the flyers collection. The missile tick:

class Missile(Flyer):
    def tick_timer(self, delta_time, missiles):
        self._timer.tick(delta_time, missiles)

    def tick(self, delta_time, fleet, _fleets):
        self.tick_timer(delta_time, fleet)
        self.move(delta_time)

What is Fleets.tick doing?

    def tick(self, delta_time):
        if self.asteroids and self.ships:
            self.thumper.tick(delta_time)
        else:
            self.thumper.reset()
        for fleet in self.fleets:
            fleet.tick(delta_time, self)

Ah. Each fleet gets to do its own tick, so the ship missiles get ticked while in the others fleet and saucer missiles are still in their own MissileFleet.

It’s all good. So the test …

    def test_missile_timeout(self):
        # invasive but works for now.
        # expect to revise when Fleets goes to just one collection.
        ship = Ship(Vector2(100, 100))
        fleets = Fleets()
        others = fleets.others
        ship.fire_if_possible(fleets)
        assert len(others) == 1
        missile = others[0]
        missile.tick_timer(0.5, others)
        assert len(others) == 1
        missile.tick_timer(3.0, others)
        assert len(others) == 0

Is now green. Commit: Ship counts its own missiles and fires appropriately.

Let’s relax a moment and reflect.

Relaxoflection

The actual work on this went well. But on the periphery there were issues.

Speculative FireControl
I imagined the code that would be in Ship and thought it might not be cohesive (it isn’t), so I guessed at a FireControl object that would be helpful. I wasn’t even done with it when I realized it wasn’t helpful.

Was that a mistake? I suppose you could count it as one, but it did focus my mind on the issue better than it had been, and having the rudimentary object in hand made it more clear that what I needed didn’t justify that object, and perhaps didn’t justify any object.

It was a misstep, for sure, because I removed it, stepping back to where I was before … but better equipped to solve the problem. I stepped to a new location, looked around, said “not this way”, and went back to step in another direction. Perfectly fine, I’d say. Sure, I wish I were more clever but I am already too clever for my pants, so that might not be the ideal wish.

Tricky Tests
The tests that I had to fix were both a bit tricky, because I’m changing the basic rules for ship missiles, moving them to the others collection instead of leaving them in the Ship’s MissileFleet for the time being. Perhaps that would have been better done in two steps. By the time I realized I had a bit more trouble than I wanted, I had fiddled both tests, and wasn’t sure where a rollback would put me.

I changed focus to the other test, fixed it, then went back, reviewed a bit of code and quickly made the first test run. I do think, as shown in the comments on the test, that it’s a bit weird, but testing a timer like that is always going to result in a strange test.

The Actual Code
The actual code looks like this:
class Ship(Flyer):
    def fire_if_possible(self, fleets):
        if self._can_fire and self._missile_tally < u.MISSILE_LIMIT:
            fleets.add_flyer(self.create_missile())
            self._can_fire = False

    def create_missile(self):
        player.play("fire", self._location)
        return Missile.from_ship(self.missile_start(), self.missile_velocity())

The tally is maintained like this:

class Ship(Flyer):
    def begin_interactions(self, fleets):
        self._missile_tally = 0

    def interact_with_missile(self, missile, fleets):
        if missile.is_ship_missile:
            self._missile_tally += 1
        if missile.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

We could extract two tiny methods from the interact, but come on, would it really be more clear? Shall we try it? OK.

OK, this is interesting!

I refactored like this:

    def interact_with_missile(self, missile, fleets):
        self.tally_ship_missiles(missile)
        self.explode_if_hit(fleets, missile)

    def explode_if_hit(self, fleets, attacker):
        if attacker.are_we_colliding(self.position, self.radius):
            self.explode(fleets)

    def tally_ship_missiles(self, missile):
        if missile.is_ship_missile:
            self._missile_tally += 1

Then PyCharm asked me if I wanted to use my new explode_if_hit in the two other locations it found:

    def interact_with_asteroid(self, asteroid, fleets):
        self.explode_if_hit(fleets, asteroid)

    def interact_with_saucer(self, saucer, fleets):
        self.explode_if_hit(fleets, saucer)

So there’s an interesting learning. Extracting that method with just a tiny almost useless increment of clarity resulted in simplification of two other methods, replacing their conditional logic with a call to a single copy of the logic. I changed the parameter name to attacker since it was no longer just a missile.

Nice!

Commit: improve interaction code.

Are we there yet?

Not quite. We are no longer using the MissileFleet intended for the ship. We can convert to a standard Fleet for sure:

class Fleets:
    def __init__(self, asteroids=None, missiles=None, saucers=None, saucer_missiles=None, ships=None):
        asteroids = asteroids if asteroids is not None else []
        missiles = missiles if missiles is not None else []
        saucers = saucers if saucers is not None else []
        saucer_missiles = saucer_missiles if saucer_missiles is not None else []
        ships = ships if ships is not None else []
        self.fleets = (
            Fleet(asteroids),
            Fleet(missiles),  # <===
            SaucerFleet(saucers),
            MissileFleet(saucer_missiles, u.SAUCER_MISSILE_LIMIT),
            ShipFleet(ships),
            Fleet([]),  # explosions not used
            Fleet([]))
        self.thumper = Thumper(self.beat1, self.beat2)
        self.score = 0

There are some tests providing the collection, but they all run. So we can commit: Ship missiles no longer used in Game, standard Fleet for now.

I’d like to remove that list entirely but that will have to wait for a time when I want to sort through the tests, and that time is not now.

Summary

Small steps FTW, for sure. Speculation was a bit wasteful but not without value in providing better understanding of what was needed. I will be a bit slower to invent an object in hopes that it’ll be useful, at least for a while until the burn wears off.

We’re another step along the path to having all the game logic in the individual Flyer subclasses. We’ll need to do this same trick to Saucer to allow us to remove the MissileFleet entirely, and there may be tests that we’ll want to replace. Or there may not. We’ll see.

I’m just tired enough to prohibit working on Saucer right now. I could do it, but I don’t want to. I’ll go pair with my book or something.

See you next time!