Python Asteroids+Invaders on GitHub

Saucer is still too complicated. I had an idea yesterday at close of session: a helper class!

Even after all the renaming, the current Invaders Saucer is more tricky and hard to understand than is ideal. Its two-stage initialization is a definite code sign, a portent of trouble. The pluggable strategy for how it works before it’s fully initialized and after is clean enough, I suppose, but it’s hard to see what happens.

We’d like to do better, if our purpose were to have this code be in condition to be worked on at some time in the future when we don’t remember exactly what’s going on. And, late in yesterday’s session, I had an idea that seems obvious to me now:

Oh! What if we had a SaucerCreator object? Instead of dropping a Saucer into the TimeCapsule, we’d put a SaucerCreator in there. It would run, collect the necessary info, and create the Saucer all in one go. That might be a really good idea!

Now, especially if we were to call it SaucerMaker, a name more in keeping with this program’s style, this idea seems totally obvious and correct. We have other Maker objects, including PlayerMaker, ShipMaker, WaveMaker, and even SaucerMaker (Asteroids). Why didn’t I think of this right away? I have no idea. Possibly it was because the style in recent days has been the use of TimeCapsule. Perhaps the Invaders Saucer just grew in that direction and so my thoughts kept working with what I had. Perhaps I’m losing it.

Doesn’t matter. We are not permitted to edit the past, we just get to decide what to do next.

There was one clue that I surely missed. One of Kent Beck’s design patterns is Complete Construction Method, the notion that when we create an object, we should create it complete and ready to go in its constructor, rather than creating it and then sending it messages to complete it. Clearly our Saucer doesn’t work that way, and that lack should have seemed like a Big Code Sign to me.

But … it’s “obvious” that the Saucer cannot leap out of the TimeCapsule ready to go, because it needs a connection to the player, to get the shot count, and to the invader fleet to count the invaders, because the saucer does not run if there are fewer than 8 invaders. So, I guess I just kind of went with it.

But now, I think we have a better idea.

InvadersSaucerMaker

The core idea is simple. The saucer runs every N seconds. I think we have N=10 right now. It starts on the right when the player has fired an even number of shots, and left when she has fired an odd number. It does not start at all if there are fewer than 8 invaders on the screen.

We currently use a ten-second TimeCapsule containing a Saucer. We’ll switch to a ten-second capsule containing an InvadersSaucerMaker. The maker will capture the player and the invaders fleet. If all is good, the maker will inject an InvadersSaucer, and will in any case inject a new TimeCapsule with a new maker.

Let’s begin by reviewing TimeCapsule.

class TimeCapsule(InvadersFlyer):
    def __init__(self, time, added_flyer, removed_flyer=None):
        self.to_add = added_flyer
        self.to_remove = removed_flyer
        self.time = time

    def tick(self, delta_time, fleets):
        self.time -= delta_time
        if self.time <= 0:
            fleets.remove(self)
            fleets.append(self.to_add)
            if self.to_remove:
                fleets.remove(self.to_remove)

I think that’s all we need to know: we put an instance in to add, and optionally one to remove.

Shall we test-drive the new maker? Let’s do.

class TestInvadersSaucerMaker:

    def test_exists(self):
        maker = InvadersSaucerMaker()

This seems to demand a new class. PyCharm will create one for me, right in the test file. I’ll let it do that and move the class out in due time.

My new test passes. How’s this thing going to work? Well, it will set up the usual interaction stuff and needs to interact with player and invaders fleet. Before it can create the saucer, it will need the player shot count and for the invader count to be no less than 8.

Let’s just sketch in a happy path test.

    def test_happy_path(self):
        fleets = Fleets()
        fi = FI(fleets)
        invader_fleet = FakeInvaderFleet(8)
        invader_player = FakeInvaderPlayer(0)
        maker = InvadersSaucerMaker()
        maker.begin_interactions(fleets)
        maker.interact_withinvaderfleet(invader_fleet, fleets)
        maker.interact_with_invaderplayer(invader_player, fleets)
        maker.end_interactions(fleets)
        assert len(fi.time_capsules) == 1
        assert len(fi.invader_saucers) == 1

Note that I seem to have decided to use fake objects throughout. More to the point, I’m supposing that the Maker will do its work in end_interactions. I think I can just type in the object now and make it run. Well, nearly run.

PyCharm adds the empty methods for me. I’ll fill in some stuff. This passes the test:

class InvadersSaucerMaker:
    def begin_interactions(self, fleets):
        self.shot_count = None
        self.invader_count = None
        
    def interact_withinvaderfleet(self, invader_fleet, fleets):
        self.invader_count = invader_fleet.invader_count()

    def interact_with_invaderplayer(self, invader_player, fleets):
        self.shot_count = invader_player.shot_count

    def end_interactions(self, fleets):
        if self.invader_count >= 8:
            fleets.append(InvadersSaucer())
        new_maker = InvadersSaucerMaker()
        fleets.append(TimeCapsule(10, new_maker))

It’s not really good enough, though. I went too far, checking for the invader count, and not far enough, not checking for the shot count. Let’s do a few more little tests.

    def test_no_player(self):
        fleets = Fleets()
        fi = FI(fleets)
        invader_fleet = FakeInvaderFleet(8)
        maker = InvadersSaucerMaker()
        maker.begin_interactions(fleets)
        maker.interact_withinvaderfleet(invader_fleet, fleets)
        maker.end_interactions(fleets)
        assert len(fi.time_capsules) == 1
        assert len(fi.invader_saucers) == 0

Fix:

    def end_interactions(self, fleets):
        if self.shot_count is not None and self.invader_count >= 8:
            fleets.append(InvadersSaucer())
        new_maker = InvadersSaucerMaker()
        fleets.append(TimeCapsule(10, new_maker))

We have to check explicitly for None because zero is a legitimate shot count and zero is falsy. I think we’re about to get an error with our next test.

    def test_too_few_invaders(self):
        fleets = Fleets()
        fi = FI(fleets)
        invader_fleet = FakeInvaderFleet(7)
        invader_player = FakeInvaderPlayer(0)
        maker = InvadersSaucerMaker()
        maker.begin_interactions(fleets)
        maker.interact_withinvaderfleet(invader_fleet, fleets)
        maker.interact_with_invaderplayer(invader_player, fleets)
        maker.end_interactions(fleets)
        assert len(fi.time_capsules) == 1
        assert len(fi.invader_saucers) == 0

That passes, but what if there was no invader fleet at all? That can’t really happen, but let’s improve things anyway.

    def end_interactions(self, fleets):
        if self.shot_count is not None and self.invader_count is not None and self.invader_count >= 8:
            fleets.append(InvadersSaucer())
        new_maker = InvadersSaucerMaker()
        fleets.append(TimeCapsule(10, new_maker))

One more thing, and I don’t quite see how I’ll test drive all of it. We’ll pass in the shot count to the saucer constructor, which will surely break things.

    def end_interactions(self, fleets):
        if self.shot_count is not None and self.invader_count is not None and self.invader_count >= 8:
            fleets.append(InvadersSaucer(self.shot_count))
        new_maker = InvadersSaucerMaker()
        fleets.append(TimeCapsule(10, new_maker))

The argument is not expected. Let’s do what we can with the InvadersSaucer. We have a lot of tests for it, let me modify the saucer and try to keep the tests running. I think the changes we need are mostly to rip things out.

Oh, this seems promising. We have this:

class InvadersSaucer(InvadersFlyer):
    def __init__(self):
        self._initialize_what_we_can()
        self._strategy = PreInitStrategy(self)

    def _finish_initializing(self, shot_count):
        self._set_start_and_speed(shot_count)
        self._strategy = PostInitStrategy(self)

    def _set_start_and_speed(self, shot_count):
        speed = 8
        even_or_odd = shot_count % 2
        self._speed = (-speed, speed)[even_or_odd]
        left_or_right = (self._right, self._left)[even_or_odd]
        self.rect.center = Vector2(left_or_right, u.INVADER_SAUCER_Y)

I’ll do this:

class InvadersSaucer(InvadersFlyer):
    def __init__(self, shot_count=None):
        self._initialize_what_we_can()
        if shot_count is not None:
            self._finish_initializing(shot_count)
            self._strategy = PostInitStrategy(self)
        else:
            self._strategy = PreInitStrategy(self)

That will pass all the tests. And it does. Should we be committing this? Not quite yet, but let’s suppose that things are going well and move the maker to the prod side of things. Done. We’re green. Commit: tests and initial class InvadersSaucerMaker.

Now let’s start breaking the saucer tests and removing the ones that no longer matter. Let’s start by defaulting the shot count to zero instead of None.

class InvadersSaucer(InvadersFlyer):
    def __init__(self, shot_count=0):
        self._initialize_what_we_can()
        self._finish_initializing(shot_count)
        self._strategy = PostInitStrategy(self)

Notice that we’re not fiddling with the details of the saucer at all yet. We’re using the existing init methods and strategies. Of course, at this point, we can remove the pre-init strategy and as soon as the broken tests are dealt with, we’ll do that.

Three tests are failing. I hope they are all checking initialization.

    def test_does_not_run_with_7_invaders(self):
    def test_does_run_with_8_invaders(self):

    def test_initialized(self):
        player = InvaderPlayer()
        saucer = InvadersSaucer()
        assert isinstance(saucer._strategy, PreInitStrategy)
        saucer.interact_with_invaderplayer(player, [])
        saucer.end_interactions([])
        assert isinstance(saucer._strategy, PostInitStrategy)

    def test_sets_up_ready(self):
        saucer = InvadersSaucer()
        assert isinstance(saucer._strategy, PreInitStrategy)
        saucer._finish_initializing(0)
        assert isinstance(saucer._strategy, PostInitStrategy)

The first two are handled by the maker tests: delete. The third and fourth are hardly worth keeping. Delete them.

I could test in game now, but let’s finish the job here. First in coin:


def invaders(fleets):
    fleets.clear()
    left_bumper = u.BUMPER_LEFT
    fleets.append(Bumper(left_bumper, -1))
    fleets.append(Bumper(u.BUMPER_RIGHT, +1))
    fleets.append(TopBumper())
    fleets.append(InvaderFleet())
    fleets.append(PlayerMaker())
    fleets.append(ShotController())
    fleets.append(InvaderScoreKeeper())
    fleets.append(RoadFurniture.bottom_line())
    fleets.append(TimeCapsule(10, InvadersSaucerMaker()))
    for i in range(3):
        fleets.append(ReservePlayer(i))
    half_width = 88 / 2
    spacing = 198
    step = 180
    for i in range(4):
        place = Vector2(half_width + spacing + i * step, u.SHIELD_Y)
        fleets.append(RoadFurniture.shield(place))

And in the saucer itself:

    def _die(self, fleets):
        fleets.remove(self)
        fleets.append(TimeCapsule(10, InvadersSaucerMaker()))

However a raft of tests are failing. I seem to have an import loop. This fixes it. I’m not sure if this is how you’re supposed to do it or not:

    def _die(self, fleets):
        from invaders.invaders_saucer_maker import InvadersSaucerMaker
        fleets.remove(self)
        fleets.append(TimeCapsule(10, InvadersSaucerMaker()))

I can try this in the game. Surely something will break?

AttributeError: 'InvadersSaucerMaker' object has no attribute 'draw'

Oh, right, I didn’t make it a flyer. With that in place …

No saucer appears, and when I fire a shot it slows down and then the game obviously freezes. Oh … I think we forgot to remove the old maker.

class InvadersSaucerMaker:
    def end_interactions(self, fleets):
        if self.shot_count is not None and self.invader_count is not None and self.invader_count >= 8:
            fleets.append(InvadersSaucer(self.shot_count))
        fleets.remove(self)
        new_maker = InvadersSaucerMaker()
        fleets.append(TimeCapsule(10, new_maker))

That helps, but we still don’t create a Saucer. I find a typo in an interact_with method and fix it:

    def interact_with_invaderfleet(self, invader_fleet, fleets):
        self.invader_count = invader_fleet.invader_count()

Now, curiously enough, after ten seconds, a saucer runs. After ten more seconds, a saucer runs and right after it another one runs. After another ten seconds, three saucers run across, one after another.

Oh. I am creating saucermakers in two locations, in the saucer and in the maker. One of these guys needs to stop. Let’s make it the responsibility of the maker to create new makers.

    def _die(self, fleets):
        fleets.remove(self)

And it’s working, except that five tests are failing.

Three of them have the typo still in them. I should have used Rename and instead I just fixed it in the object, forgetting that people were calling the bad method.

Two of them are tests on saucer that are not handled by the maker. Deleted. And we are green and the game works.

Commit: game now uses InvadersSaucerMaker to make InvadersSaucers.

Now let’s unwind the strategy stuff from the saucer.

Here are the strategies:

# noinspection PyProtectedMember
class PreInitStrategy:
    def __init__(self, saucer):
        self._saucer = saucer

    def die_if_lonely(self, invader_fleet, fleets):
        self._saucer._die_if_lonely(invader_fleet, fleets)

    def finish_initializing(self, shot_count):
        self._saucer._finish_initializing(shot_count)

    def just_draw(self, screen):
        pass

    def move_or_die(self, fleets):
        pass


class PostInitStrategy:
    def __init__(self, saucer):
        self._saucer = saucer

    def die_if_lonely(self, _invader_fleet, _fleets):
        pass

    def finish_initializing(self, shot_count):
        pass

    # noinspection PyProtectedMember
    def just_draw(self, screen):
        self._saucer._just_draw(screen)

    # noinspection PyProtectedMember
    def move_or_die(self, fleets):
        self._saucer._move_or_die(fleets)

We’re not using the PreInitStrategy at all. We can remove the method it calls, _die_if_lonely, because now the saucer is never born if there aren’t enough invaders. We delete that method and the whole strategy.

Deleting the strategy doesn’t work, it breaks all the tests. A test was importing it and didn’t need it. fixed.

Now we find references to the strategy variable and essentially inline them.

    def interact_with_invaderfleet(self, invader_fleet, fleets):
        self._strategy.die_if_lonely(invader_fleet, fleets)

We no longer care about this. Remove it.

    def interact_with_invaderplayer(self, player, fleets):
        self._player_shot_count = player.shot_count
        self._strategy.finish_initializing(self._player_shot_count)

No longer needed. Remove it. Test breaks. Oh, it’s this one, checking the “mystery score” of the saucer:

    def test_first_score(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(saucer := InvadersSaucer())
        fleets.append(keeper := InvaderScoreKeeper())
        fleets.append(player := InvaderPlayer())
        saucer.interact_with_invaderplayer(player, fleets)
        shot = PlayerShot()
        shot.position = saucer.position

        def kill_saucer(expecting):
            saucer.interact_with_playershot(shot, fleets)
            score = fi.scores[-1]
            assert score.score == expecting
            player.fire(fleets)
            saucer.interact_with_invaderplayer(player, fleets)
        kill_saucer(100)
        kill_saucer(50)
        kill_saucer(50)
        kill_saucer(100)

        kill_saucer(150)
        kill_saucer(100)
        kill_saucer(100)
        kill_saucer(50)

        kill_saucer(300)
        kill_saucer(100)
        kill_saucer(100)
        kill_saucer(100)

        kill_saucer(50)
        kill_saucer(150)
        kill_saucer(100)
        kill_saucer(100)

The saucer no longer acquires the shot count from the player: it’s provided at creation time. We’ll want to improve the kill_saucer function.

    def test_first_score(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(saucer := InvadersSaucer())
        fleets.append(keeper := InvaderScoreKeeper())
        fleets.append(player := InvaderPlayer())
        saucer.interact_with_invaderplayer(player, fleets)
        shot = PlayerShot()
        shot.position = saucer.position
        shot_count = 0

        def kill_saucer(expecting):
            nonlocal shot_count
            saucer._player_shot_count = shot_count
            shot_count += 1
            saucer.interact_with_playershot(shot, fleets)
            score = fi.scores[-1]
            assert score.score == expecting
            player.fire(fleets)
            saucer.interact_with_invaderplayer(player, fleets)
        kill_saucer(100)
        ...

That does the trick. Here’s the next use of _strategy:

    def update(self, delta_time, fleets):
        self._strategy.move_or_die(fleets)

We can just inline _move_or_die:

    def update(self, delta_time, fleets):
        self._move_along_x()
        self._adjust_stereo_position()
        self._die_if_done(fleets)

Now this one:

    def _finish_initializing(self, shot_count):
        self._set_start_and_speed(shot_count)
        self._strategy = PostInitStrategy(self)

We just remove the last line as no longer in use.

    def draw(self, screen):
        self._strategy.just_draw(screen)

Next in draw we inline _just_draw:

    def draw(self, screen):
        screen.blit(self._map, self.rect)

I could be committing all these, what’s wrong with me? I think I’m rushing. Commit: removed pre and post init strategy code. PyCharm objects to the comment, wanting pre- and post-. I ignore it.

I want one more code review but I think we’re good. I need a break. You can take one too, if you feel like it.

Reflection / Summary

This went very smoothly. The test-driving of the Maker went nicely, using some fake objects, which is not my usual style. The object itself is of course quite standard and simple: look for some info during interactions and take action based on whether or not you find it.

I think it was wise to leave most of the strategy code in place, and simply call the init functions immediately, as we’ll see in a moment. That meant that few changes were needed to the Saucer. I did make one notable mistake when I thought that the saucer should insert a TimeCapsule with maker instead of a new saucer, where what it really needed to do was to do nothing, because the maker replicates itself as needed.

I forgot to make the object adhere to the Flyer rules, but that showed up as soon as I ran it. I wonder if there is a change of practice that would have avoided that error. Possibly the Fleets object (and any fake ones) should ensure that everything appended is a subclass of Flyer. That might have shown up the error sooner. Seems like something I wouldn’t generally do, however.

Overall this has gone quite nicely and Saucer is much more direct and simpler.

We’ll look at the code, and improve it, in the next article. See you there!