Python Asteroids+Invaders on GitHub

Let’s find another small step toward invader shots. Maybe even more than one. Very long, I’m not sure why. And no commit at the end. Afternoon distractions. Not delight, just distractions.

I haven’t figured out anything more about the shot timing, but here are some steps we might take:

  1. Alternate among the three types of shots;
  2. No more than three shots alive;
  3. Each type can occur only once on the screen at the same time;
  4. Make shots emanate from specific columns;
  5. Make them emanate from the bottom of the column;
  6. Improve the animation of the player when hit.
  7. Pick ideal column for targeted rolling shot.

As we do these things, we’ll be on the lookout for learnings about how to do things in a better fashion.

Let’s review the ShotController’s key aspects:

class ShotController(InvadersFlyer):
    max_firing_time = 0x30

    def __init__(self):
        self.time_since_firing = 0

    def begin_interactions(self, fleets):
        self.time_since_firing += 1

    def end_interactions(self, fleets):
        if self.time_since_firing >= self.max_firing_time:
            self.time_since_firing = 0
            pos = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            shot = InvaderShot(pos, BitmapMaker.instance().squiggles)
            fleets.append(shot)

The controller does not know how many shots are in flight, nor which ones they are. I think, but am not certain, that we need not worry about the requirement that only one of each type can be present. We might even find that our customer (me) is OK with not matching that old-time rule.

let’s begin by isolating the shot-firing code into a method we can call from the tests without regard to timing:

    def end_interactions(self, fleets):
        if self.time_since_firing >= self.max_firing_time:
            self.fire_next_shot(fleets)

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        pos = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
        shot = InvaderShot(pos, BitmapMaker.instance().squiggles)
        fleets.append(shot)

Now let’s arrange for three shots to fire and each be a different type than the one before. How will we know type? Right now, there are three methods in BitmapMaker, squiggles, plungers, and rollers.

We can call a function, given its name, with Python. We’ll probably do that. At least at first.

Let’s test:

    def test_fires_three_different_shots(self):
        fleets = Fleets()
        fi = FI(fleets)
        controller = ShotController()
        for _ in range(3):
            controller.fire_next_shot(fleets)
        shots = fi.invader_shots
        assert len(shots) == 3

This much works now. Of course all three shots are squiggles. How can we determine that they’re different? We could just fetch their first map entry and compare them.

    def test_fires_three_different_shots(self):
        fleets = Fleets()
        fi = FI(fleets)
        controller = ShotController()
        for _ in range(3):
            controller.fire_next_shot(fleets)
        shots = fi.invader_shots
        assert len(shots) == 3
        s1, s2, s3 = shots
        assert s1.map != s2.map and s2.map != s3.map and s3.map != s1.map

This will work because they haven’t moved yet. For more certainty we could use maps[0] in each case.

Test fails for now.

Now I’m asking myself, if there can only be three shots, one of each kind, why not create them in the controller and reuse them? Let’s try that: it’s a reasonable thing to do.

class ShotController(InvadersFlyer):
    max_firing_time = 0x30
    available = Vector2(-1, -1)

    def __init__(self):
        self.time_since_firing = 0
        self.shots = [
            InvaderShot(self.available, BitmapMaker.instance().squiggles),
            InvaderShot(self.available, BitmapMaker.instance().rollers),
            InvaderShot(self.available, BitmapMaker.instance().plungers)]
        self.shot_index = 0

That seems to make some sense. I’m planning to somehow make use of that available thing, as a live/dead flag.

Now:

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot = self.shots[self.shot_index]
        self.shot_index = (self.shot_index + 1) % 3
        if shot.position == self.available:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            fleets.append(shot)

Nearly good but we need to be sure to set them back to available. Let’s look at the shot.

I know I’m going way beyond my current test, but I’ve got the bit in my teeth and think I’m doing good work. I’ll go back and fix up the tests.

OK, just typing that interrupted me enough so that I’ll do the tests now. I think I must be green at this point. Yes.

Now enhance the test by checking the positions.

    def test_fires_three_different_shots(self):
        fleets = Fleets()
        fi = FI(fleets)
        controller = ShotController()
        for _ in range(3):
            controller.fire_next_shot(fleets)
        shots = fi.invader_shots
        assert len(shots) == 3
        s1, s2, s3 = shots
        assert s1.map != s2.map and s2.map != s3.map and s3.map != s1.map
        assert s1.position != ShotController.available
        assert s2.position != ShotController.available
        assert s2.position != ShotController.available

Still green. Now can I look to see how shots die?

class InvaderShot(InvadersFlyer):
    def move(self, fleets):
        self.moves += 1
        self.update_map()
        self.position = self.position + Vector2(0, 16)
        if self.position.y >= u.SCREEN_SIZE:
            fleets.remove(self)

    def interact_with_playershot(self, shot, fleets):
        if self.colliding(shot):
            fleets.remove(self)

Change those removes to self.die(fleets). I think extract can do it.

Yes, PyCharm changes both those to refer to this:

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

And I change die to reset the position:

    def die(self, fleets):
        self.position = ShotController.available
        fleets.remove(self)

That breaks all the tests, because I have a loop in the imports. I do not love this but:

    def die(self, fleets):
        from shotcontroller import ShotController
        self.position = ShotController.available
        fleets.remove(self)

One test has failed:

    def test_dies_past_edge(self, shot):
        fleets = Fleets()
        fi = FI(fleets)
        fleets.append(shot)
        current_pos = u.CENTER
        while current_pos.y < u.SCREEN_SIZE:
            assert shot.position == current_pos
            current_pos = current_pos + Vector2(0, 16)
            shot.update(1/60, fleets)
            shot.update(1/60, fleets)
            shot.update(1/60, fleets)
        assert shot.position.y == -1
        assert not fi.invader_shots

That’s as fixed. It was formerly checking for 1024, not -1. Our new death throes changed that.

We’re green. Extend our firing test further.

    def test_fires_three_different_shots(self):
        fleets = Fleets()
        fi = FI(fleets)
        controller = ShotController()
        for _ in range(3):
            controller.fire_next_shot(fleets)
        shots = fi.invader_shots
        assert len(shots) == 3
        s1, s2, s3 = shots
        assert s1.map != s2.map and s2.map != s3.map and s3.map != s1.map
        assert s1.position != ShotController.available
        assert s2.position != ShotController.available
        assert s2.position != ShotController.available
        controller.fire_next_shot(fleets)
        assert len(fi.invader_shots) == 3

Continues to pass but now …

    def test_fires_three_different_shots(self):
        fleets = Fleets()
        fi = FI(fleets)
        controller = ShotController()
        for _ in range(3):
            controller.fire_next_shot(fleets)
        shots = fi.invader_shots
        assert len(shots) == 3
        s1, s2, s3 = shots
        assert s1.map != s2.map and s2.map != s3.map and s3.map != s1.map
        assert s1.position != ShotController.available
        assert s2.position != ShotController.available
        assert s2.position != ShotController.available
        controller.fire_next_shot(fleets)
        assert len(fi.invader_shots) == 3
        s1.die(fleets)
        s2.die(fleets)
        s3.die(fleets)
        assert len(fi.invader_shots) == 0
        for _ in range(3):
            controller.fire_next_shot(fleets)
        assert len(fi.invader_shots) == 3

Green. I want to see what the game does. Should fire random shots, cycling the three types.

Just as expected! Commit: game fires random shots cycling the three types.

Reflection

Best to settle down after all that excitement, think how we’re doing, plan what’s next.

Here’s our idea list from above:

  1. Alternate among the three types of shots;
  2. No more than three shots alive;
  3. Each type can occur only once on the screen at the same time;
  4. Make shots emanate from specific columns;
  5. Make them emanate from the bottom of the column;
  6. Improve the animation of the player when hit.
  7. Pick ideal column for targeted rolling shot.

Those three are done and I feel that they are done well. I didn’t have the idea of creating the shots once and reusing them until I got to the point of solving the problem. Would it have been better to have decided that in advance? Well, not for me. I prefer to be looking at the code when I decide how to do things, and I am confident that if I do them less well than is desirable, I can improve them.

So I’m good with the top three.

Now what we want to do is to have the shots emanate from the correct columns. The order is defined in the original source code, which says:

ColFireTable:
; This table decides which column a shot will fall from. The column number is read from the
; table (1-11) and the pointer increases for the shot type. For instance, the "squiggly" shot
; will fall from columns in this order: 0B, 01, 06, 03. If you play the game you'll see that
; order.
;
; The "plunger" shot uses index 00-0F (inclusive)
; The "squiggly" shot uses index 06-14 (inclusive)
; The "rolling" shot targets the player
1D00: 01 07 01 01 01 04 0B 01 06 03 01 01 0B 09 02 08                                    
1D10: 02 0B 04 07 0A

The comments above clearly say “1-11”, but I don’t notice it and since my columns are 0-10, things go a bit awry for a while down below.

I think I can afford to duplicate those bytes. So I’m going to use these two lists:

0x01, 0x07, 0x01, 0x01, 0x01, 0x04, 0x0B, 0x01, 0x06, 0x03, 0x01, 0x01, 0x0B, 0x09, 0x02, 0x08
0x0B, 0x01, 0x06, 0x03, 0x01, 0x01, 0x0B, 0x09, 0x02, 0x08, 0x02, 0x0B, 0x04, 0x07, 0x0A, 0x01

class ShotController(InvadersFlyer):
    def __init__(self):
        self.time_since_firing = 0
        self.shots = [
            InvaderShot(self.available, BitmapMaker.instance().squiggles),
            InvaderShot(self.available, BitmapMaker.instance().rollers),
            InvaderShot(self.available, BitmapMaker.instance().plungers)]
        self.columns = [
            [0x01, 0x07, 0x01, 0x01, 0x01, 0x04, 0x0B, 0x01, 0x06, 0x03, 0x01, 0x01, 0x0B, 0x09, 0x02, 0x08],
            [0x0B, 0x01, 0x06, 0x03, 0x01, 0x01, 0x0B, 0x09, 0x02, 0x08, 0x02, 0x0B, 0x04, 0x07, 0x0A, 0x01]]
        self.shot_index = 0

Now let’s test to see if we use the columns. I think I’m going to change the fire_next_shot method’s signature. I want to specify the shot type in that call. This is a guess but I think it’s a good one. The code now:

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot = self.shots[self.shot_index]
        self.shot_index = (self.shot_index + 1) % 3
        if shot.position == self.available:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            fleets.append(shot)

Nice little refactoring sequence starts here

Ah, looking at the code always helps. Let’s instead reorder …

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot = self.shots[self.shot_index]
        if shot.position == self.available:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            fleets.append(shot)
        self.shot_index = (self.shot_index + 1) % 3

And then extract variable:

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot_index = self.shot_index
        shot = self.shots[shot_index]
        if shot.position == self.available:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            fleets.append(shot)
        self.shot_index = (self.shot_index + 1) % 3

And extract method:

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot_index = self.shot_index
        self.fire_specific_shot(shot_index, fleets)
        self.shot_index = (self.shot_index + 1) % 3

    def fire_specific_shot(self, shot_index, fleets):
        shot = self.shots[shot_index]
        if shot.position == self.available:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            fleets.append(shot)

And ends here

Why those steps? Moving the increment wasn’t actually necessary but I didn’t foresee that. It did help me gain clarity, worth it for that. Extracting the variable made PyCharm detect shot_index as a parameter for the Extract Method. And the Extract Method gives me something I can test. The steps taken allowed me to do every move with a PyCharm refactoring, which is faster than I can type and far less prone to error.

We could commit this now, if we wanted to. I don’t think we do: I want to see how this goes before I lock down.

When I try to write a test I veer a bit away from what I just refactored:

    def test_plunger_columns(self):
        controller = ShotController()
        assert controller.next_column_for(0) == 0x01
        assert controller.next_column_for(0) == 0x07
        assert controller.next_column_for(1) == 0x0B
        assert controller.next_column_for(1) == 0x01

I’m test-driving a new method to fetch the next column for each of indexes 0 and 1, the two shots that use the table. I’m OK with the 0, 1 because … well, I am.

The method doesn’t exist. Let’s make it exist.

class ShotController(InvadersFlyer):
    def __init__(self):
        self.time_since_firing = 0
        self.shots = [
            InvaderShot(self.available, BitmapMaker.instance().squiggles),
            InvaderShot(self.available, BitmapMaker.instance().rollers),
            InvaderShot(self.available, BitmapMaker.instance().plungers)]
        self.columns = [
            [0x01, 0x07, 0x01, 0x01, 0x01, 0x04, 0x0B, 0x01, 0x06, 0x03, 0x01, 0x01, 0x0B, 0x09, 0x02, 0x08],
            [0x0B, 0x01, 0x06, 0x03, 0x01, 0x01, 0x0B, 0x09, 0x02, 0x08, 0x02, 0x0B, 0x04, 0x07, 0x0A, 0x01]]
        self.current_columns = [0, 0]
        self.shot_index = 0

    def next_column_for(self, shot_index):
        column_number = self.current_columns[shot_index]
        self.current_columns[shot_index] = (self.current_columns[shot_index] + 1) % 16
        return self.columns[shot_index][column_number]

The test is passing. Let’s make it a bit harder, checking wrap_around.

    def test_plunger_columns(self):
        controller = ShotController()
        assert controller.next_column_for(0) == 0x01
        assert controller.next_column_for(0) == 0x07
        assert controller.next_column_for(1) == 0x0B
        assert controller.next_column_for(1) == 0x01
        for _ in range(14):
            controller.next_column_for(1)
        assert controller.next_column_for(1) == 0x0B

OK, we have the column number. Commit: ShotController cycles column numbers for shots 0 and 1.

Of course it’s not using the number yet.

Let me see where that might go. I don’t quite see the test.

    def fire_specific_shot(self, shot_index, fleets):
        shot = self.shots[shot_index]
        if shot.position == self.available:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
            fleets.append(shot)

I want to use the column right there where we set the position.

Extract a method:

    def fire_specific_shot(self, shot_index, fleets):
        shot = self.shots[shot_index]
        if shot.position == self.available:
            self.select_shot_position(shot)
            fleets.append(shot)

    def select_shot_position(self, shot):
        shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)

Change signature to require the index. I wish the shot knew that fact. For now, we’ll pass it.

    def fire_specific_shot(self, shot_index, fleets):
        shot = self.shots[shot_index]
        if shot.position == self.available:
            self.select_shot_position(shot, shot_index)
            fleets.append(shot)

    def select_shot_position(self, shot, shot_index):
        shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)

Now use the index. This is a sketch:

    def select_shot_position(self, shot, shot_index):
        if shot_index == 2:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
        else:
            col = self.next_column_for(shot_index)
            pos = invader_fleet.group.bottom_of_column(col)
            if pos:
                shot.position = pos
            else:
                shot.position = Vector2(10, 10)

The rule is going to be that if we can’t get the column, we just don’t fire. We might want not to advance the firing order, I’ll have to think about it. For now, we’ll fire it from top left to make it clear what happened.

We do not have access to invader_fleet, but we can get it:

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

Tests are borked. They need to be enhanced to pass in the group. This is a pain.

Oh well, in for a penny … This required a few changes:

    def select_shot_position(self, shot, shot_index):
        if shot_index == 2:
            shot.position = Vector2(random.randint(50, u.SCREEN_SIZE - 50), u.SCREEN_SIZE / 2)
        else:
            col = self.next_column_for(shot_index)
            invader = self.invader_fleet.invader_group.bottom_of_column(col)
            if invader:
                shot.position = invader.position
            else:
                shot.position = Vector2(10, 10)

The bottom_of_column returns an invader not a position. I want to test this in game.

I should mention: distractions have started. I’d be wise to stop.

Doesn’t look right in the game. I have it print the shot index, column, and coordinate and get this:

0 1 [264, 512]
0 7 [672, 512]
0 1 [304, 512]
1 1 [312, 512]
0 1 [328, 512]
1 6 [656, 512]
0 1 [352, 512]
1 3 [480, 512]
0 4 [528, 544]
1 1 [328, 544]
1 1 [304, 544]
0 1 [272, 544]
0 6 [576, 544]
1 9 [760, 544]
0 3 [360, 544]
1 2 [288, 544]

I would kind of expect to see the shot_index going 0, 1, 2, 0, 1, 2, and for 0 the columns should go 1, 7, 1, 1, 1, 4, B and we have 1, 7, 1, 1, 1, 4, 1, 6, 3, which is odd.

Here is where I wish I had caught the one-based vs zero-based issue. Would have saved a little time. I was blaming my code at this point, mistakenly. I’m glad I decided against more tests, they would have just hardened my belief in the wrong numbers.

I need better tests. But first …

Let me bash this, though I shouldn’t.

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot_index = self.shot_index
        self.fire_specific_shot(shot_index, fleets)
        self.shot_index = (self.shot_index + 1) % 3

    def fire_specific_shot(self, shot_index, fleets):
        shot = self.shots[shot_index]
        if shot.position == self.available:
            self.select_shot_position(shot, shot_index)
            fleets.append(shot)

Let’s not increment the shot_index unless we fire. I’ll move that code down into the fire_specific. I wind up here after some debugging:

    def fire_next_shot(self, fleets):
        self.time_since_firing = 0
        shot_index = self.shot_index
        self.fire_specific_shot(shot_index, fleets)

    def fire_specific_shot(self, shot_index, fleets):
        shot = self.shots[shot_index]
        if shot.position == self.available:
            self.select_shot_position(shot, shot_index)
            fleets.append(shot)
            self.shot_index = (self.shot_index + 1) % 3

    def select_shot_position(self, shot, shot_index):
        if shot_index == 2:
            shot.position = Vector2(1000, 64)
        else:
            col = self.next_column_for(shot_index)
            invader = self.invader_fleet.invader_group.bottom_of_column(col)
            if invader:
                shot.position = invader.position
            else:
                shot.position = Vector2(10, 10)

    def next_column_for(self, shot_index):
        column_number = self.current_columns[shot_index]
        self.current_columns[shot_index] = (self.current_columns[shot_index] + 1) % 16
        return self.columns[shot_index][column_number]

I finally figured out that the indexes of the columns are one-based and I need zero. So I just edited the tables:

        self.columns = [
            [0x00, 0x06, 0x00, 0x00, 0x00, 0x03, 0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07],
            [0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07, 0x01, 0x0A, 0x03, 0x06, 0x09, 0x00]]

With that in place, things look good. I have two tests broken, but here’s what it does now. Note that the plunger ones all run down the right side. I wanted them out of the way so that I could watch the other two types.

I don’t think I like how non-random the fall seems. We might want to change that, but it’s what the doctor ordered.

I have tests broken and they’ll take time to fix. Since I’ve been derailed, I’ll leave the commit open. I may want to refactor a bit to make it easier.

Somehow this has become immense. Enough.

Summary

I’ve made good progress. I think the code could be better, but as soon as I upgrade these two broken tests, I’ll be in a position to commit.

I would have done well to have stopped when distractions started. But no harm done, I think what we have is roughly what we need, a refactoring or two away from decent.

I am quite tempted to make a tiny object that holds the columns and ticks through them. And I think the code wants a little regrouping to make it more testable, which will probably also make it generally better. It usually does.

See you next time!