Python Asteroids+Invaders on GitHub

Yesterday I was running faster than my feet. Let’s see how the Saucer code looks and how to improve its tests and the code itself.

I’m honestly not sure what got me moving too fast yesterday, but I began putting more into a test than I needed and then a lot more into the class than the tests called for. I think the results are OK, but I never felt quite in balance.

I’ve found it to be important to pay attention to my feelings as I work, and I often mention them in these articles. Where I could improve would be in responding to those feelings in a more productive fashion. Probably the single most important thing I could do would be to pause, take a break, settle down, and start anew. But it seems that once I get going too fast, I find it difficult to want to slow down. Today, we’ll slow down, and, having slowed down, with any luck, we’ll get more done in less time. Slow down to speed up? Absolutely. Smooth is better than ragged, every time.

What we have at present is that in coin.py we create a ten-second TimeCapsule containing a Saucer, which streaks across the screen and is never seen again. Here’s the saucer code:

class InvadersSaucer(InvadersFlyer):
    def __init__(self, direction=1):
        self.direction = direction
        maker = BitmapMaker.instance()
        self.saucers = maker.saucers  # one turret, two explosions
        self._map = self.saucers[0]
        self._mask = pygame.mask.from_surface(self._map)
        self._rect = self._map.get_rect()
        self.left = 64
        self.rect.center = Vector2(self.left, u.InvaderSaucerY)

    @property
    def mask(self):
        return self._mask

    @property
    def rect(self):
        return self._rect

    @property
    def position(self):
        return Vector2(self.rect.center)

    @position.setter
    def position(self, value):
        self.rect.center = value

    def interact_with(self, other, fleets):
        other.interact_with_invaderssaucer(self, fleets)

    def update(self, delta_time, fleets):
        x = self.position.x + 16
        x_max = 960
        if x > x_max:
            fleets.remove(self)
        else:
            self.position = (x, self.position.y)

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

I’ve seen worse. I’ve written worse. I’ll probably write worse again. But, I fondly hope, not today.

What are our story slices for Saucer? Here are the ones from yesterday, plus anything I can think of in the next few seconds.

  • Flies at constant height and speed TBD
  • Direction: Comes from right if player shot count is even
  • Shares space/time with squiggly (do not implement).
  • Flies if > 7 aliens exist
  • Flies every 600 game loops (10 seconds)
  • Scores: 10 05 05 10 15 10 10 05 30 10 10 10 05 15 10 05
  • Score loops at 15, not 16 (replicate this bug?)
  • Start and stop near the side bumpers?
  • Move more magic numbers to ‘u’
  • Some values in u do not have UPPER_CASE names

The 64 and 960 up there are close to reasonable. What are the bumper settings? They’re defined in coin.py:

coin.py
def invaders(fleets):
    fleets.clear()
    left_bumper = 64
    fleets.append(Bumper(left_bumper, -1))
    fleets.append(Bumper(960, +1))

Let’s move those two magic numbers over to u.

coin.py
def invaders(fleets):
    fleets.clear()
    left_bumper = u.BUMPER_LEFT
    fleets.append(Bumper(left_bumper, -1))
    fleets.append(Bumper(u.BUMPER_RIGHT, +1))

u.py

BOTTOM_LINE_OFFSET = 50
BUMPER_LEFT = 64
BUMPER_RIGHT = 960
INVADER_PLAYER_OFFSET = 128
INVADER_SAUCER_Y = 128
RESERVE_PLAYER_OFFSET = 32
SHIELD_OFFSET = 208

I renamed all the constants to be in the UPPER_CASE style. The OFFSET ones concern me. They’re used like this:

        self.rect.center = Vector2(self.left, u.SCREEN_SIZE - INVADER_PLAYER_OFFSET)

We could provide the Y coordinate in u and avoid referring to screen size in some of the code. Let’s do that. I’ll provide the constants first:

BOTTOM_LINE_OFFSET = 50
BUMPER_LEFT = 64
BUMPER_RIGHT = 960
INVADER_PLAYER_OFFSET = 128
INVADER_PLAYER_Y = SCREEN_SIZE - 128
INVADER_SAUCER_Y = 128
RESERVE_PLAYER_OFFSET = 32
RESERVE_PLAYER_Y = SCREEN_SIZE - 32
SHIELD_OFFSET = 208
SHIELD_Y = SCREEN_SIZE - 208

Then find references to the offsets and change them:

        ...
        self.rect.center = Vector2(x, u.RESERVE_PLAYER_Y)
        ...
        self.rect.center = Vector2(self.left, u.INVADER_PLAYER_Y)
        ...
    for i in range(4):
        place = Vector2(half_width + spacing + i * step, u.SHIELD_Y)
        fleets.append(RoadFurniture.shield(place))

We’re green and good. Commit: Move magic numbers to u, renames, provide Y coordinates.

What have we been doing? I thought we were here to work on the Saucer. Yes … and sometimes we clean the campground before we even start using it. We can now use those bumper numbers in Saucer, and the other cleaning occurred because we were referring to u and took the opportunity to make things a bit better.

Now to Saucer. I think that the first actual features will be:

  • reduce speed from “streak” to “fly”
  • make saucer repeat every 10 seconds
  • do not show saucer with less than 8 invaders on screen
  • arrange to use shot count, not input parameter direction.

Let’s begin with the init:

class InvadersSaucer(InvadersFlyer):
    def __init__(self, direction=1):
        self.direction = direction
        maker = BitmapMaker.instance()
        self.saucers = maker.saucers  # one turret, two explosions
        self._map = self.saucers[0]
        self._mask = pygame.mask.from_surface(self._map)
        self._rect = self._map.get_rect()
        self.left = 64
        self.rect.center = Vector2(self.left, u.INVADER_SAUCER_Y)

The Saucer is 24 bits wide, I think, so 96 at our scale. We can get the half width from the rectangle, so:

class InvadersSaucer(InvadersFlyer):
    def __init__(self, direction=1):
        self.direction = direction
        maker = BitmapMaker.instance()
        self.saucers = maker.saucers  # one turret, two explosions
        self._map = self.saucers[0]
        self._mask = pygame.mask.from_surface(self._map)
        self._rect = self._map.get_rect()
        half_width = self._rect.width // 2
        self._left = u.BUMPER_LEFT + half_width
        self._right = u.BUMPER_RIGHT - half_width
        self.rect.center = Vector2(self._left, u.INVADER_SAUCER_Y)

    def update(self, delta_time, fleets):
        x = self.position.x + 16
        if x > self._right:
            fleets.remove(self)
        else:
            self.position = (x, self.position.y)

We’re inching toward better. I freely grant that I do not have an overall plan for this code. I plan to improve it by changing things I don’t like into things that I like better, until I am satisfied. Why? Well, possibly my brain is dull this morning, but I claim it’s because the code currently has lots of little things needing improvement, and when those are improved, either it’ll be code we can accept, or we’ll see whatever larger thing it may need.

I had to change one of my two tests:

    def test_saucer_moves(self):
        saucer = InvadersSaucer(1)
        left_side_start = Vector2(u.BUMPER_LEFT + 48, 128)
        assert saucer.position == left_side_start
        saucer.update(1.0/60.0, [])
        assert saucer.position.x > left_side_start.x

This is a truly stupid test. I intended just to drive out the update method. Let’s just test that it moves, like the name says.

    def test_saucer_moves(self):
        saucer = InvadersSaucer(1)
        start = saucer.position
        saucer.update(1.0/60.0, [])
        assert saucer.position.x != start.x

Now at least it tests what it says it tests. I am not proud of the tests for this thing. Let’s work on making it only run if there are at least 8 invaders. I think we can write a test for that.

This turns out to be harder than I thought it would be. Recall that our invaders do not all interact with everything: they are kept in an InvaderGroup inside the InvaderFleet, and only interact as the Fleet and Group allow. In particular, there is no interact_with_invader method anywhere. I want that, or thought I did.

Well. We can have the Saucer interact with the Fleet and ask the Fleet for the invader count. That should work.

Here’s the test reflecting that idea:

    def test_does_not_run_with_7_invaders(self):
        fleets = Fleets()
        fi = FI(Fleets)
        fleets.append(invader_fleet := InvaderFleet())
        invader_group = invader_fleet.invader_group
        assert invader_group.invader_count() == 55
        while invader_group.invader_count() > 7:
            invader_group.kill(invader_group.invaders[0])
        assert invader_group.invader_count() == 7
        fleets.append(saucer := InvadersSaucer())
        start = saucer.position
        saucer.update(1.0/60.0, fleets)
        assert saucer.position == start

That’s a bit involved, isn’t it? We build the invader fleet and group and then remove invaders until there are only 7. Then we update the saucer and expect it not to move. We also expect it not to be in Fleets, but we’re not checking that yet. Let’s do.

        assert saucer.position == start
        assert not fi.invader_saucers

I add the new invader_saucers property to FI. Now let’s make this work. It’s a little more tricky than we might like. The effect we want is that if there are less than 8 invaders, the saucer never appears at all. We can’t check that sooner than the first begin_interactions, but what if we’re flying the saucer and the you shoot the 8th invader? We don’t want the saucer to die. We need a more powerful test to drive out all the behavior.

Rather than reach beyond the test, let’s do the simplest thing:

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

    def update(self, delta_time, fleets):
        if fleets.invader_count() < 8:
            self.die(fleets)
            return
        x = self.position.x + 16
        if x > self._right:
            self.die()
        else:
            self.position = (x, self.position.y)

Ah. Can’t do it there. We don’t want to change Fleets. We need to interact with InvaderFleet to do this. Change the test:

    def test_does_not_run_with_7_invaders(self):
        fleets = Fleets()
        fi = FI(Fleets)
        fleets.append(invader_fleet := InvaderFleet())
        invader_group = invader_fleet.invader_group
        assert invader_group.invader_count() == 55
        while invader_group.invader_count() > 7:
            invader_group.kill(invader_group.invaders[0])
        assert invader_group.invader_count() == 7
        fleets.append(saucer := InvadersSaucer())
        start = saucer.position
        saucer.interact_with_invaderfleet(invader_fleet, fleets)
        saucer.update(1.0/60.0, fleets)
        assert saucer.position == start
        assert not fi.invader_saucers

And:

class InvadersSaucer(InvadersFlyer):
    def interact_with_invaderfleet(self, invader_fleet, fleets):
        if invader_fleet.invader_count() < 0:
            self.die(fleets)

class InvaderFleet(InvaderShot):
    def invader_count(self):
        return self.invader_group.invader_count()

This, however, isn’t sufficient to stop the update. The object won’t really be removed until after all the interactions are over. I think we have to remove that check from the test and just make sure that we’re gone from the fleets. We’ll then test further, dealing with visibility, which I think will sort this out.

Confusion
Some confusion starts here, because of two simple typos.

I am surprised when the test fails on the check for invader_saucers.

class FleetsInspector:
    @property
    def invader_saucers(self):
        return self.select_class(InvadersSaucer)

Here’s what the test failure says:

>       return self.fleets.select(condition)
E       TypeError: Fleets.select() missing 1 required positional argument: 'condition'

More FleetsInspector code:

    @property
    def invader_players(self):
        return self.select_class(InvaderPlayer)

    @property
    def invader_saucers(self):
        return self.select_class(InvadersSaucer)

    @property
    def invader_shots(self):
        return self.select_class(InvaderShot)

    def select(self, condition):
        return self.fleets.select(condition)

    def select_class(self, klass):
        return self.select(lambda a: isinstance(a, klass))

That sure looks the same as the others. Let me enhance the test to check earlier for the object, when we know it is there. Same failure.

I hate to do this but I think I have to debug. Ah. Here’s the bug:

Resolving
I find one typo, “F” where I needed “f”.
    def test_does_not_run_with_7_invaders(self):
        fleets = Fleets()
        fi = FI(Fleets)  $ <=== should be lower case: instance.
        fleets.append(invader_fleet := InvaderFleet())

The test still fails, saying that the object isn’t removed. Let’s recheck the code:

    def interact_with_invaderfleet(self, invader_fleet, fleets):
        if invader_fleet.invader_count() < 0:
            self.die(fleets)
Resolved
I finally spot the second typo.

Nice finger fumbling. Should say < 8 not < 0.

    def interact_with_invaderfleet(self, invader_fleet, fleets):
        if invader_fleet.invader_count() < 8:
            self.die(fleets)

Green! OK now to test that it doesn’t remove itself at 8:

    def test_does_run_with_8_invaders(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(invader_fleet := InvaderFleet())
        invader_group = invader_fleet.invader_group
        assert invader_group.invader_count() == 55
        while invader_group.invader_count() > 8:
            invader_group.kill(invader_group.invaders[0])
        assert invader_group.invader_count() == 8
        fleets.append(saucer := InvadersSaucer())
        assert fi.invader_saucers
        saucer.interact_with_invaderfleet(invader_fleet, fleets)
        saucer.update(1.0/60.0, fleets)
        assert fi.invader_saucers

Let’s commit: Saucer removes itself if started with less than 8 invaders present.

That was strange. The typos both threw me off, and when the first one didn’t fix it, it set me back on my heels for a bit. I have an eyelid irritation and my eyes are a bit blurry this morning. I’ll blame that for the typo, though how it affected my fingers isn’t quite clear.

Let’s make the saucer come back after it’s gone. We’ll write another test.

    def test_returns_after_dying(self):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(saucer := InvadersSaucer())
        stop_loop = 10000
        while fi.invader_saucers and stop_loop > 0:
            saucer.update(1/60, fleets)
            stop_loop -= 1
        assert stop_loop > 0
        assert not fi.invader_saucers
        assert fi.time_capsules

I figure that’s a decent start. FI doesn’t know about time capsules yet. With that added, the test fails not finding a time capsule. That’s the point of the test and we implement:

    def die(self, fleets):
        fleets.remove(self)
        fleets.append(TimeCapsule(10, InvadersSaucer()))

Green. And I expect to see the saucer streak across every ten seconds. And I do.

Commit: Saucer streaks every ten seconds if at least 8 invaders exist.

Lets see. What else shall we do? We’re about two hours in, time to break.

We’ll cut the speed in half, and paint the saucer red like it is supposed to be.

class InvadersSaucer(InvadersFlyer):
    def __init__(self, direction=1):
        self.direction = direction
        maker = BitmapMaker.instance()
        self.saucers = maker.saucers  # one turret, two explosions
        self._map = self.saucers[0]
        self._mask = pygame.mask.from_surface(self._map)
        self._rect = self._map.get_rect()
        half_width = self._rect.width // 2
        self._left = u.BUMPER_LEFT + half_width
        self._right = u.BUMPER_RIGHT - half_width
        self.rect.center = Vector2(self._left, u.INVADER_SAUCER_Y)
        self._speed = 8

    def update(self, delta_time, fleets):
        x = self.position.x + self._speed
        if x > self._right:
            self.die(fleets)
        else:
            self.position = (x, self.position.y)

class BitmapMaker:
        self.saucers = [self.make_and_scale_surface(saucer, scale, (24, 8), "red") for saucer in saucers]

red saucer]

Summary

My eyes need a break and probably my brain does as well. Let’s sum up.

We have better tests in place. They’re a bit larger than one would like, but given the decentralized design, we can’t really do much better. It’s possible that some fake objects could help a bit, but then we’d just be setting up those instead.

The saucer object is more clean than it was. We’ve introduced a new magic number, but at least it is identified as speed now. What about the handling of the other magic 8, the count of active invaders? We should get that over to u, certainly. Other Flyers count things. In this case, we just asked the InvaderFleet for the count of invaders, it asked the InvaderGroup, and we decided what to do about it. I think making the go-no go decision in the saucer is the right place.

We’ve improved the saucer and also enhanced FleetInspector, and added some much better tests for the saucer.

We have not tested that the TimeCapsule created has the correct contents, and perhaps we should. It does have the correct contents, however, and we’re ready for next steps. We’ll decide next time what they’ll be.

One interesting story is that the saucer is supposed to fly left to right if there has been an even number of player shots and right to left if odd. Or maybe the other way around. I think the whole idea is odd.

And, then there’s scoring. So we have things to test and things to do. We’ll decide on next steps next time.

See you then!