More to do with bombs, especially targeted ones. We need some refactoring as well. The results may surprise me.

As predicted, this article is in fact Space Invaders 42. The day is going well so far! Do not expect answers however, so much as more questions. This isn’t that 42.

A bit about tests

I should have mentioned one more thing about the tests from yesterday, but either I forgot or it came to me later.

It is commonly advised, by the people who advise things, that a “good” TDD test should only have one assertion. You’ll note that in many of my tests, including yesterday’s, I use multiple assertions. Sometimes we’re testing a sequence of events, so to me it makes sense to start up, expect that the thing hasn’t happened, then trigger some event, then expect that it has happened. It’s certainly possible to break that into separate tests, but it doesn’t tell the story as well as the sequence, in my opinion.

Another case is the situation where there’s rather a lot of setup. In that case, rather than duplicate the setup in a number of tests, I like to do the setup, then exercise it as many times as needed. That’s exactly what yesterday’s test was doing.

There is a “better” way of doing that with single-assertion tests, and that is to move the setup into the before part of the test group. CodeaUnit does offer that facility, but I rarely use it. There are two reasons, neither of them particularly good, but good enough that I stay where I am.

First, if you have multiple test groups, CodeaUnit separately reports each group, as you might imagine it would. That makes for multiple groups of two-line reports on the main screen, and I don’t like that. No good reason, I just don’t.

Second, setting up a CodeaUnit test isn’t easy. It’s a nested set of function definitions, and I don’t have a convenient minimal one to grab and paste in, owing to a combination of “never quite got around to it” and there being no really good way to store a tab template in Codea.

The biggest downside to multiple expects in CodeaUnit, and most TDD frameworks, is that when an assertion fails you can’t be sure which one actually failed. Some frameworks allow you to put a descriptive message on each assertion, but CodeaUnit does not.

I do find this a bit irritating, and I’m tempted to add the capability to CodeaUnit. I have a couple of other improvements in mind for it anyway. So perhaps, one of these days, we’ll divert to look at that. It’ll be somewhat educational because CodeaUnit, like most TDD frameworks, does some mysterious stuff in order to find and execute all the tests.

We’ll see about that. But today, more about bombs.

Bomb Issues

We know that our code needs refactoring. This method is the main clue:

function Bomb:targetedColumn()
    local gunnerPos = Gunner.pos.x
    local armyPos = TheArmy:xPosition()
    local relativePos = gunnerPos - armyPos
    local result = 1 + relativePos//16
    return math.max(math.min(result,11),1)
end

This code suffers from “feature envy”. It doesn’t refer to the Bomb at all. It does refer to the gunner and the army, using globals (aieee!). So it clearly belongs somewhere else.

It’s called by Bomb:nextColumn, with does refer to mostly bomb-related notions, but that function is called from here:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    self.bombCycle = 0
    aBomb.alive = true
    local col = aBomb:nextColumn()
    local leader = self.invaders[1]
    local leaderPos = leader.pos
    local leaderX = leaderPos.x
    local bombX = 8 + leaderX + (col-1)*16
    aBomb.pos = vec2(bombX, leaderPos.y - 16)
end

Since the info that targetedColumn needs is at least partly in the army, it seems to make more sense to put it here and call it back from nextColumn if need be. We do have other issues.

That long sequence of locals in dropBomb is there because of the rather long sequence it takes to decide where the coordinates of the column are. I wrote it out step by step to give myself a chance of getting it right, but it’s begging to be improved somehow.

Worst of all, it doesn’t work as well as we need.

The last line there sets the bomb’s Y coordinate to 16 less than the leader’s Y coordinate, that is, just below the lower rank of invaders. That has two undesirable effects:

First, if there are a few missing invaders in the column, we’d like the bomb to issue from the bottom-most existing one. It looks funny when the bomb starts out way below the invader.

Second, if there are no invaders in the column, the bomb is dropped anyway. That’s not good.

So today’s mission, and I’ve decided to accept it, is to improve the system so that it drops bombs from the lowest existing invader in the column. If there are no invaders, we need to decide what to do. The easiest thing is probably just not to drop the bomb. If we don’t reset the timer in that case, we’ll try another a 60th of a second later.

I want to plan this out a bit but first I want to move the targeted column calculation into the army.

To do that, we’ll pass the army down to the nextColumn method, so that that method can call back:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    self.bombCycle = 0
    aBomb.alive = true
    local col = aBomb:nextColumn(self)
    local leader = self.invaders[1]
    local leaderPos = leader.pos
    local leaderX = leaderPos.x
    local bombX = 8 + leaderX + (col-1)*16
    aBomb.pos = vec2(bombX, leaderPos.y - 16)
end

Now we change nextColumn:

function Bomb:nextColumn(army)
    if self.targeted then return army:targetedColumn() end
    local next = self.columnTable[self.columnIndex]
    self.columnIndex = self.columnIndex+1
    if self.columnIndex > self.columnEnd then
        self.columnIndex = self.columnStart
    end
    return next
end

And we move and modify targetedColumn:

function Army:targetedColumn()
    local gunnerX = Gunner.pos.x
    local armyX = self:xPosition()
    local relativeX = gunnerPos - armyPos
    local result = 1 + relativeX//16
    return math.max(math.min(result,11),1)
end

I renamed the Pos variables to X, since they compute only on the X coordinate.

I expect this to work in the game and a quick inspection tells me that the test should be happy as well. We’ll see. I’ll fire it up.

Well, almost. Our test doesn’t pass in the army. We should be able to back out the hackery setting the global as well. Out with the old:

        _:test("rolling shot finds correct column", function()
            local cacheArmy = TheArmy
            TheArmy = Army()
            setupGunner()
            local bomb = TheArmy.rollingBomb
            TheArmy:setPosition(vec2(10,180))
            Gunner.pos = vec2(10,32)
            local col = bomb:nextColumn()
            _:expect(col).is(1)
            Gunner.pos = vec2(26,32)
            col = bomb:nextColumn()
            _:expect(col).is(2)
            Gunner.pos = vec2(30,32)
            col = bomb:nextColumn()
            _:expect(col).is(2)
            Gunner.pos = vec2(0,32)
            col = bomb:nextColumn()
            _:expect(col).is(1)
            Gunner.pos = vec2(208,32)
            col = bomb:nextColumn()
            _:expect(col).is(11)
            TheArmy = cacheArmy
        end)

And in with the new:

        _:test("rolling shot finds correct column", function()
            local army = Army()
            setupGunner()
            local bomb = army.rollingBomb
            TheArmy:setPosition(vec2(10,180))
            Gunner.pos = vec2(10,32)
            local col = bomb:nextColumn(army)
            _:expect(col).is(1)
            Gunner.pos = vec2(26,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(2)
            Gunner.pos = vec2(30,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(2)
            Gunner.pos = vec2(0,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(1)
            Gunner.pos = vec2(208,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(11)
        end)

That’s nicer but the test fails, telling us that our refactoring wasn’t quite correct:

19: rolling shot finds correct column -- Army:100: attempt to perform arithmetic on a nil value (global 'gunnerPos')

Yes. I missed some of the changes from Pos to X. I need refactoring tools or at least a reasonable replace.

function Army:targetedColumn()
    local gunnerX = Gunner.pos.x
    local armyX = self:xPosition()
    local relativeX = gunnerX - armyX
    local result = 1 + relativeX//16
    return math.max(math.min(result,11),1)
end

Wow. Curiously, I now get two errors, both saying:

19: rolling shot finds correct column -- Actual: 1.0, Expected: 2

This is where the one assertion per test comes in. I’ll have to figure out where those two errors are. One clue is that they come one after the other with no intervening OK, and another is that there’s one OK above those two and two OKs below. So …

        _:test("rolling shot finds correct column", function()
            local army = Army()
            setupGunner()
            local bomb = army.rollingBomb
            TheArmy:setPosition(vec2(10,180))
            Gunner.pos = vec2(10,32)
            local col = bomb:nextColumn(army)
            _:expect(col).is(1)
            Gunner.pos = vec2(26,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(2)
            Gunner.pos = vec2(30,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(2)
            Gunner.pos = vec2(0,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(1)
            Gunner.pos = vec2(208,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(11)
        end)

And, fortunately,, there are only two 2’s in there. But why did they fail? I am surprised. I also note, looking at this test, that I only check for 1, 2, and 11, because I was confident that if my divide trick worked for 1 and 2, it’d work for the intermediate values as well, because I know how the code works. Or thought I did. Should we try another value, or just find the bug? Let’s try another value: it might give a clue where to look.

I’m adding this test:

            Gunner.pos = vec2(91,32)
            col = bomb:nextColumn(army)
            _:expect(col).is(6)

Check my arithmetic. The army is at x = 10, so with gunner at = 10, we should (and do) get 1 as the column desired. Five times 16 is 80, plus 10 is 90, plus 1 just to be sure, is five more columns along, so we should get six.

What do we get?

19: rolling shot finds correct column -- Actual: 5.0, Expected: 6

How could this happen? I do not know. This makes me want to know what the xPosition is, because I know what the gunner position is.

            Gunner.pos = vec2(91,32)
            col = bomb:nextColumn(army)
            _:expect(army:xPosition()).is(10)
            _:expect(col).is(6)

Isn’t 10, is 16. I should have just let it be 16, shouldn’t I? So my setPosition isn’t working, just as it didn’t when the army global wasn’t what I thought it was. That’s because I didn’t refactor correctly:

        _:test("rolling shot finds correct column", function()
            local army = Army()
            setupGunner()
            local bomb = army.rollingBomb
            TheArmy:setPosition(vec2(10,180)) -- <---
            Gunner.pos = vec2(10,32)
            local col = bomb:nextColumn(army)
            _:expect(col).is(1)

Changing that to army gets the tests to green. Our refactoring is done. Commit: move targetedColumn to Army.

It’s 0840 and I think it’s time to go fetch a chai.

Planning the Column

I’ve successfully hunted and gathered an iced chai, so it’s time to turn our attention to dealing with the column contents when we drop the bomb. I think our spec is this:

  • If there are no invaders in the designated column, do not drop a bomb;
  • If there are invaders, start the bomb 16 pixels below the bottom-most invader.

Our current drop function looks like this:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    self.bombCycle = 0
    aBomb.alive = true
    local col = aBomb:nextColumn(self)
    local leader = self.invaders[1]
    local leaderPos = leader.pos
    local leaderX = leaderPos.x
    local bombX = 8 + leaderX + (col-1)*16
    aBomb.pos = vec2(bombX, leaderPos.y - 16)
end

This is in a rather strange order, isn’t it? It has decided early on that the bomb is going down. Then it decides where it starts. That’s kind of backward. Also there’s that long list of locals.

We’re at a green bar, so we can legitimately improve this code to allow us to place our new feature better. Let’s extract the x calculation. We have the convenient function xPosition to use, which wasn’t available when we wrote the function above:

function Army:xPosition()
    local got = self.invaders[1].pos
    return got.x
end

Let’s clean that while we’re here:

function Army:xPosition()
    return self.invaders[1].pos.x
end

And use it:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    self.bombCycle = 0
    aBomb.alive = true
    local col = aBomb:nextColumn(self)
    local leader = self.invaders[1]
    local leaderPos = leader.pos
    aBomb.pos = vec2(bombX, leaderPos.y - 16)
end

function Army:bombX(col)
    return 8 + self:xPosition() + (col-1)*16
end

Hm that doesn’t work at all. All the bombs are calling from x = 0. Oh. Forgot to call it:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    self.bombCycle = 0
    aBomb.alive = true
    local col = aBomb:nextColumn(self)
    local leader = self.invaders[1]
    local leaderPos = leader.pos
    aBomb.pos = vec2(self:bombX(col), leaderPos.y - 16)
end

This is why one makes tiny changes: it makes it easier to find mistakes like that. Of course an extract method tool would have done it right, but this is Codea.

So this bit is working. Now we could extract yPosition similarly, but we don’t really want it. What we want is the position of the bottom-most invader in col, and if there is none, some indication thereof.

I have that feeling that this is tricky enough to need a test. It’ll be a difficult one to write, however, because we’d need to correctly set some invaders in a column to not so very alive, and then call our function. Still, it’s worth doing, I think.

I usually regret it when I feel I “should” test and then don’t, so let’s test. I don’t think I’ve ever regretted test driving something.

        _:test("finding bottom-most invader", function()
            local army = Army()
            local y
            y = army:lowestInvaderY(1)
            _:expect(y).is(105)
        end)

So the function lowestInvaderY returns the y coordinate of the lowest invader (or nil, I guess). We happen to know that the lowest invaders in a starting army are at 105. So:

function Army:lowestInvaderY(col)
    return 105
end

It works. Ship it.

Well, maybe not. I want a helper function to kill an invader by number. The current invader kill logic doesn’t lend itself to this so we can either rip into the guts of the army from here, or call upon it to do the deed itself. The latter is better.

        _:test("finding bottom-most invader", function()
            local army = Army()
            local y
            y = army:lowestInvaderY(1)
            _:expect(y).is(105)
            army:directKillInvader(1)
            _:expect(army:lowestInvaderY(1)).is(121)
        end)

I think 105 plus 16 is 121. Now

function Army:directKillInvader(invaderNumber)
    self.invaders[invaderNumber].alive = false
end

This test will fail, saying got 105 expected 121. Or my name’s not … Ron. Fails as expected. It’s possible that I didn’t type “Ron” until I knew the test would fail as I planned.

Now we have the small matter of making it work. First, I do this:

function Army:lowestInvaderY(col)
    local startIndex = 1 + (col-1)//5
    return self.invaders[startIndex].pos.y
end

I think startIndex is the index of the bottom-row alien in col. The bottom row indices are 1,6,11,16, etc. That’s 1 + 0,5,10,15. And `(col=1)//5’ goes 0,1,2 by 5’s. I’m feeling really good about this mission, Dave.

So far so good, I still got my 105 and didn’t get my 121. Well, after I fixed the problem where I fetched .x instead of .y above. I’m still feeling pretty good about this mission, Dave.

Now do to the actual work.

function Army:lowestInvaderY(col)
    local startIndex = 1 + (col-1)//5
    for row = 0,4 do
        if self.invaders[startIndex + 5*row].alive then
            return self.invaders[startIndex + 5*row].pos.y
        end
    end
    return nil
end

Curiously, despite my confidence, that didn’t work. It still returned 105, suggesting that my alien isn’t as dead as I had thought.

OK, yeah, well there are 11 columns, not 5 rows. So the step is by 11s not by 5s. So:

function Army:lowestInvaderY(col)
    local startIndex = 1 + (col-1)//11
    for row = 0,4 do
        local index = startIndex + 11*row
        if self.invaders[index].alive then
            return self.invaders[index].pos.y
        end
    end
    return nil
end

That passes. Let’s now nuke them all in the first column:

        _:test("finding bottom-most invader", function()
            local army = Army()
            local y
            y = army:lowestInvaderY(1)
            _:expect(y).is(105)
            army:directKillInvader(1)
            _:expect(army:lowestInvaderY(1)).is(121)
            army:directKillInvader(1 + 11)
            _:expect(army:lowestInvaderY(1)).is(137)
            army:directKillInvader(1 + 2*11)
            _:expect(army:lowestInvaderY(1)).is(153)
            army:directKillInvader(1 + 3*11)
            _:expect(army:lowestInvaderY(1)).is(169)
            army:directKillInvader(1 + 4*11)
            _:expect(army:lowestInvaderY(1)).is(nil)
        end)

So that’s good. But because of my problem with the 5 vs 11, I wonder if we’re OK for other columns. Let’s look at the code:

function Army:lowestInvaderY(col)
    local startIndex = 1 + (col-1)//11
    for row = 0,4 do
        local index = startIndex + 11*row
        if self.invaders[index].alive then
            return self.invaders[index].pos.y
        end
    end
    return nil
end

Yes, I’m confident, I see the 11 there at the top. We could write another test but I’m good with what we’ve got.

Now let’s use our new function in here:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    self.bombCycle = 0
    aBomb.alive = true
    local col = aBomb:nextColumn(self)
    local leader = self.invaders[1]
    local leaderPos = leader.pos
    aBomb.pos = vec2(self:bombX(col), leaderPos.y - 16)
end

Let’s do this:

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    local col = aBomb:nextColumn(self)
    local y = self:lowestInvaderY(col)
    if y ~= nil then
        aBomb.pos = vec2(self:bombX(col), y - 16)
        self.bombCycle = 0
        aBomb.alive = true
    end
end

I think this should do the job for us. Let’s run the program and see. Weirdly, all the bombs are still starting at 105-16.

A judicious print tells me two odd things. First, printing y gives me a lower than expected y for a lot of columns, even if I’ve not killed any aliens in those columns, once I kill some in any column. Second, the bombs don’t seem to be dropping from the adjusted coordinate anyway. Strange.

First, looks like trusting myself or not, I need to beef up that test. First, though, a look at the code:

function Army:lowestInvaderY(col)
    local startIndex = 1 + (col-1)//11
    for row = 0,4 do
        local index = startIndex + 11*row
        if self.invaders[index].alive then
            return self.invaders[index].pos.y
        end
    end
    return nil
end

No, that really looks right to me now. It isn’t, because this added test fails:

        _:test("finding bottom-most invader", function()
            local army = Army()
            local y
            y = army:lowestInvaderY(1)
            _:expect(y).is(105)
            army:directKillInvader(1)
            _:expect(army:lowestInvaderY(1)).is(121)
            _:expect(army:lowestInvaderY(2)).is(105) -- <---
            army:directKillInvader(1 + 11)
            _:expect(army:lowestInvaderY(1)).is(137)
            army:directKillInvader(1 + 2*11)
            _:expect(army:lowestInvaderY(1)).is(153)
            army:directKillInvader(1 + 3*11)
            _:expect(army:lowestInvaderY(1)).is(169)
            army:directKillInvader(1 + 4*11)
            _:expect(army:lowestInvaderY(1)).is(nil)
        end)

How’s that happening? Oh:

function Army:lowestInvaderY(col)
    local startIndex = 1 + (col-1)//11 -- <---
    for row = 0,4 do
        local index = startIndex + 11*row
        if self.invaders[index].alive then
            return self.invaders[index].pos.y
        end
    end
    return nil
end

Starting index is seriously wrong. With column 2, we want to start at 12. So I think that should be:

    local startIndex = 1 + (col-1)*11

At column 1 that gives 1, at column 2 it gives 12. So that seems right. However, my test still fails:

20: finding bottom-most invader -- Actual: 121.0, Expected: 105

After two OKs, so it’s this last line:

        _:test("finding bottom-most invader", function()
            local army = Army()
            local y
            y = army:lowestInvaderY(1)
            _:expect(y).is(105)
            army:directKillInvader(1)
            _:expect(army:lowestInvaderY(1)).is(121)
            _:expect(army:lowestInvaderY(2)).is(105)

Again I am confuse.

No. Start index goes 1,2,3 and that is in fact the index of the invader at the bottom of the column. Whee. G*d I’m bad at this programming stuff.

Interesting and confusing fact: The normal shots never are fired from column 5: There is no entry 5 in the table. That’s the only column number that’s omitted. So that explains why when I shot a bunch of aliens in that column, I never saw any shots emerging from higher up.

What’s the lesson here? Well, perhaps we should have had a function for calculating the starting invader index in a column and tested that directly. It seems pretty silly to test a function whose output is its input but in fact I needed something.

Failing that, testing values outside the chosen column would have found the problem sooner.

Anyway, it’s nearing lunch time, so let’s clean this up a bit and then commit:

function Army:lowestInvaderY(col)
    for row = 0,4 do
        local index = col + 11*row
        if self.invaders[index].alive then
            return self.invaders[index].pos.y
        end
    end
    return nil
end

Now that I look at the calculation of index in this form, I recognize it as the standard form for indexing a one-dimensional array via two dimensions. That would have been a useful thing to think about.

Another useful thing might be some kind of an atXY or atRowCol function on Army, embedding the right calculation once and for all. A larger topic overall would be to figure out a standard way to code so as to avoid most of my one-vs-zero issues, which have cropped up so often. I still don’t see offhand how to resolve that. I program daily in a zero-based language and a one-based language. I may be doomed.

Anyway: commit: bombs drop from bottom-most aliens, empty columns skipped.

We’ll quickly wrap up and then get outa here.

Wrapping Up

Today went well except for my foolish inability to compute row/column indexes. I’m sure this is partly exacerbated by the fact that sometimes I think row/column and sometimes x/y, and rows are y not x so I keep flipping things in my head. Apparently I’m not very good at that.

The refactoring to move the targeted column code was well chosen. It simplified things a bit and put the functionality in a better place. We could consider whether the bombs should be actively involved in decision making at all, but generally, as an OO person, I like objects that carry the weight of behavior, not just data. That said, there could be other OO models.

For example, we could think of a fire control computer at the Army level, that sent remote commands to invaders, loading their bombs with control software, leaving the bombs to just blindly fall. It could make sense. Right now, I like what we’ve got,

I’m really glad I decided to TDD that targeted column calculator. I think it would have been really hard to debug without TDD. And I’m not glad that I limited the tests to column 1. That allowed a serious misunderstanding to seep into the code and it was tricky to discover.

When I spent more time writing compilers, or writing assembler, I was more adept with subscripting, and had at my fingertips the snippets of code that one always uses to access multi-dimensional arrays in memory. I don’t do that much any more, and the rust on my brain is showing.

I think we’re in good shape just now, and the code is decent, but it’s fragile, more because of my mind being fragile than the actual code. Fortunately, when these things work, we tend to leave them alone, so we’re probably OK.

Overall, it went well, and the game’s a bit better for it, both externally and internally. That’s how a programming day should go.

See you next time!

Invaders.zip