Space Invaders 45

Well this goes entirely not as planned. Double Arrgh.

Spoiler: None of the code written today can ship. It’s all reverted. Read with that in mind.

It’s time to ship some new capability. If we don’t, our zero to three users will put their quarters into some other machine. But I really want to clean up some of those globals and other messes.

All too often, in my past, I thought of these two needs as being in inevitable conflict. One had to give. But, looking into history back, I realize that whenever that happened, there was something else more important happening:

We didn’t have anything to ship.

When the company or the users are kept waiting for long periods, months or years, they become impatient. When they become impatient, they “whip the ponies harder”, and we’re the ponies. When they whip us harder, we try to go faster, and as soon as we’re going faster than we can, we stumble, fall, trample on perfectly good code, and things get worse rather than better.

So to “keep the customer satisfied”, we need to have a version always available, always improving in the ways that the customer wants.

To have a version always available, always improving in the ways that the customer wants, we need a version that is always improving in its design.

If we hold back from capability, the customer fires us. If we hold back on design, we slow down, and the customer fires us.

The only way, in my view, is to do both. We can’t do them at exactly the same time, so we do a bit of one and a bit of the other, rinse, repeat.

And that’s what I’m going to do today.

Saucer

The main game-play feature we’re missing is the saucer. At intervals during the game, the saucer flies across the top of the field, making a truly irritating sound. If we manage to hit it, we score a variable amount, from 50 points to 300.

We can glean some useful facts from the code and comments of the original program.

• The saucer uses the rolling shot’s motion slot, so only one or the other can be on the screen at a time;
• The saucer goes left if the player has made an odd number of shots, and right otherwise;
• The saucer only flies if the aliens are below some given level, I think 0x78.
• There is a time-for-saucer timer. I don’t know its value or when it is ticked, yet.

We want to get some reasonable subset of this implemented. I’d like to get something like this: once the aliens are below 0x78, with some low probability, say 1/10, instead of firing a rolling shot, we fly the saucer. We’ll probably pick a random direction, since we don’t have a shot count yet. But that ought to be easy. We’ll see. We’ll try to make it possible to shoot the saucer.

That seems like a lot, and I’m starting late today. And I want to improve at least a bit of the code campground around here.

Tests?

I owe it to myself to think about tests. What aspects of this new feature can we reasonably expect to test, especially in the sense of TDD?

I’m a bit scared by my experience of a day or so ago, because in testing behavior around the game runner and army, I released a version that didn’t work at all. That was terribly embarrassing, or would have been were I not always ever so welcoming to discovering that I’m even less smart than I fear. So, given that I’m afraid to write tests against the army, let’s do that.

Where’s a good place to put in the code for this? It must be in the firing logic:

function Army:possiblyDropBomb()
    if self:canDropBomb() then 
        self:dropRandomBomb()
    end
end

function Army:canDropBomb()
    return self.bombCycle > self.bombDropCycleLimit
end

function Army:dropRandomBomb()
    local bombs = {self.rollingBomb, self.plungerBomb, self.squiggleBomb}
    local bombType = math.random(3)
    self:dropBomb(bombs[bombType])
end

The really nice thing about this code is that it doesn’t know what kind of bomb it’s going to drop. Let’s read on: we know we deal with rolling bombs separately somewhere:

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

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

Only the bomb knows its type. That’s marvelous and as it should be. Bombs update their position internally as well, and when they finally hit something (and they always do) they tell the army to delete them.

Could we possibly just plunk down a saucer instead of a bomb and have this same logic run the saucer sideways instead of vertically? One issue is that the Army knows about its three bombs, and it knows them directly:

function Army:update()
    self:updateBombCycle()
    self:possiblyDropBomb()
    local continue = true
    while(continue) do
        continue = self:nextInvader():update(self.motion, self)
    end
    self.rollingBomb:update(self)
    self.plungerBomb:update(self)
    self.squiggleBomb:update(self)
end

But who said that the thing in rollingBomb has to really be a bomb? And who said that a bomb couldn’t look like a saucer and fly sideways? No one, that’s who.

I think the biggest sticking point is here:

function Army:dropRandomBomb()
    local bombs = {self.rollingBomb, self.plungerBomb, self.squiggleBomb}
    local bombType = math.random(3)
    self:dropBomb(bombs[bombType])
end

The army has these three bombs, associated with those three slots, and it’s really always updating all three, whether they’re alive or not. If we wanted to spoof that rolling bomb slot with a saucer, we’d have to store something new in the slot, and the code treats them as invariant. We could change that, but it might not be best.

Another possibility is to change what’s in there from the beginning. The init looks like this:

    self.rollingBomb = Bomb(vec2(0,0), bombTypes[1], bombExplosion, true, 1,16)
    self.plungerBomb = Bomb(vec2(0,0), bombTypes[2], bombExplosion, false, 1,16)
    self.squiggleBomb = Bomb(vec2(0,0), bombTypes[3], bombExplosion, false, 7,21)

It’s that true that makes the rolling shot special. But we certainly could use an entirely different kind of object in that slot, a switchable bomb or something like that. Imagine an object with the Bomb protocol, but that either contained a Bomb or a Saucer, and that just forwarded the messages on to whichever it was holding. That could work nicely.

Is it too much, or too big a bite? Couldn’t we just check the targeted flag in more places and be just as well off?

Or why spoof at all? Why not just have another slot for the saucer and enable it, and disable the rolling shot, as we will. Quite possibly in the original game, the saucer was an afterthought, and quite possibly they didn’t want to use another slot for memory reasons.

We don’t have that constraint. We don’t have to do anything clever, and we have every reason not to.

I think we’ll drive straight at this. We’ll have a new object, the saucer, and it will have its own place to live. Let’s see if we can TDD that.

Sauce Object

We have a plan. What does a saucer object have to do? It initializes at one side of the screen or the other, moves to the other or the one, and disappears. Honestly, I think there isn’t anything to TDD here yet. Let me sketch it in.

-- Saucer
-- RJ 20200914

Saucer = class()

function Saucer:init(army)
    self.army = army
    self.alive = false
end

function Saucer:draw()
    pushMatrix()
    pushStyle()
    translate(self.pos:unpack())
    if self.alive then
        tint(255,0,0)
        sprite(asset.saucer,0,0)
        popStyle()
        popMatrix()
    end
end

function Saucer:update()
    self.pos = self.pos + self.move
    if self.pos.x < self.minx or self.pos.x > self.maxx then
        self.army:deleteBomb(self)
    end
end

I think if I were to create one of those it would nearly work. It needs a move direction in order to come alive, and I’d like to have it have a message to tell it to go.

A small bit of adjustment and I have this spike class in operation:

-- Saucer
-- RJ 20200914

Saucer = class()

function Saucer:init(army)
    self.army = army
    self.alive = false
    self.step = vec2(2,0)
    self.move = self.step
    self.pos = vec2(0,0)
    self.minx = 8
    self.maxx = 208
end

function Saucer:go(direction)
    self.move = self.step*direction
    if direction > 0 then
        self.pos.x = self.minx
    else
        self.pos.x = self.maxx
    end
    self.pos.y = self.army:saucerTop()
    self.alive = true
end

function Saucer:draw()
    pushMatrix()
    pushStyle()
    translate(self.pos:unpack())
    if self.alive then
        tint(255,0,0)
        sprite(asset.saucer,0,0)
        popStyle()
        popMatrix()
    end
end

function Saucer:update()
    self.pos = self.pos + self.move
    if self.pos.x < self.minx or self.pos.x > self.maxx then
        self.army:deleteBomb(self)
    end
end

It’s triggered by these patches:

    self.rollingBomb = Bomb(vec2(0,0), bombTypes[1], bombExplosion, true, 1,16)
    self.plungerBomb = Bomb(vec2(0,0), bombTypes[2], bombExplosion, false, 1,16)
    self.squiggleBomb = Bomb(vec2(0,0), bombTypes[3], bombExplosion, false, 7,21)
    self.saucer = Saucer(self)


function Army:dropRandomBomb()
    if not self.saucer.alive and self.invaders[1].pos.y < 0x78 then
        self.saucer:go(1)
    end
    ...

function Army:update()
    self:updateBombCycle()
    self:possiblyDropBomb()
    local continue = true
    while(continue) do
        continue = self:nextInvader():update(self.motion, self)
    end
    self.rollingBomb:update(self)
    self.plungerBomb:update(self)
    self.squiggleBomb:update(self)
    self.saucer:update(self)
end

function Army:draw()
    pushMatrix()
    pushStyle()
    for i,invader in ipairs(self.invaders) do
        invader:draw()
    end
    if self.rollingBomb.alive then self.rollingBomb:draw() end
    if self.plungerBomb.alive then self.plungerBomb:draw() end
    if self.squiggleBomb.alive then self.squiggleBomb:draw() end
    self.saucer:draw()
    popStyle()
    popMatrix()
end

Note that I just call draw on the saucer. It decides whether and how to draw itself. I think the code on the bombs violates our “tell, don’t ask” principle.

This works, and shows me some things that are wrong, most notably that the saucer drops down a level when the invaders drop, and the original game doesn’t do that. That’s because I’m using saucerTop() to decide where it goes, which is a dynamic value used, up until now, only to decide when a missile has gone high enough not to hit anything. If we convert that to return a suitable constant, that issue will go away.

I also learned that whatever the 0x29 and 0x68 may be, they are certainly not the x coordinates for the saucer to enter and leave. I set those values to 8 and 208 for now, which are the limits for invaders.

I also found that although the code in the original seems to step the saucer by dx = 2, that’s far too fast on my iPad. There are all those timers in the original, so possibly it doesn’t trigger every time around. But I believe that I’m only triggering every third time around as well. For now, I cranked it down to dx = 1.

What about that TDD you spoke of??

Yes. I didn’t TDD this at all. I coded it, ran it, had it explode twice for missing values and saw it running in too small a range and fixed that. I just didn’t see anything that called for tests. A smarter person than I am probably would have.

We’re here now, with a rudimentary saucer ball-peened into the code. It’s not exactly bad: I think most of it is just about where it belongs given the current design. It’s certainly not complete, not sufficiently parameterized, and the saucer goes far too often. But we have code in the right place, or nearly the right place, to deal with each of these issues.

We can choose how to go forward. If, after some consideration we see now how it might be TDD’d, we could remove everything and do it again with TDD. We could accept where we are now as a basis and TDD forward. Or we could just keep going as we have been, without TDD support.

My experience is that the last approach, as appealing as it is, often gets me in trouble. My experience with scrapping a small amount of work and starting over is that it always goes better. Let’s think what we might TDD from a clean slate and then decide.

What could we test?

The most fruitful testing area, I think, is in the code that decides whether to create a saucer, and does so. Updating the saucer may be fruitful, although it’s pretty simple. Naturally this leads us to think about creating the saucer and whether we should TDD that. Instead, let’s not TDD creation, or even implement it, and instead start from the triggering and see what happens.

Byebye code, I’m reverting you.

Poof, we’re back as of Saturday. Now a test. Something about the game having a live saucer when something something is the case.

Leaving out the “something something”, the test might look like this:

        _:test("army creates saucer", function()
            _:expect(army.saucer.alive).is(true)
        end)

This test is pretty invasive. (Many of the others are as well.) What would be better, more in the spirit of “tell, don’t ask”? Unless we look at the screen (and we can’t really do that), I’m not clear what we can tell the army to do at this point that’s much better than getting its saucer and checking whether it’s alive. We could at least require an actual method to return the saucer rather than probe into the army’s private variables.

        _:test("army creates saucer", function()
            _:expect(army:getSaucer():isAlive()).is(true)
        end)

This is a bit better in that it covers the implementation details of getting the saucer and determining whether it’s alive. That test will fail a few times. Let’s let it drive:

21: army creates saucer -- Tests:232: attempt to index a nil value (global 'army')

No army. That seems fair:

        _:test("army creates saucer", function()
            local army = Army()
            _:expect(army:getSaucer():isAlive()).is(true)
        end)

21: army creates saucer -- Tests:233: attempt to call a nil value (method 'getSaucer')

function Army:getSaucer()
    return self.saucer
end

We’d better have a saucer to return:

function Army:defineBombs()
    local bombTypes = {
{readImage(asset.rolling1), readImage(asset.rolling2), readImage(asset.rolling3), readImage(asset.rolling4)},
{readImage(asset.plunger1), readImage(asset.plunger2), readImage(asset.plunger3), readImage(asset.plunger4)},
{readImage(asset.squig1), readImage(asset.squig2), readImage(asset.squig3), readImage(asset.squig4)}
    }
    local bombExplosion = readImage(asset.alien_shot_exploding)
    self.rollingBomb = Bomb(vec2(0,0), bombTypes[1], bombExplosion, true, 1,16)
    self.plungerBomb = Bomb(vec2(0,0), bombTypes[2], bombExplosion, false, 1,16)
    self.squiggleBomb = Bomb(vec2(0,0), bombTypes[3], bombExplosion, false, 7,21)
    self.saucer = Saucer()
end

This demands saucer class:

Saucer = class()

function Saucer:init()
end

21: army creates saucer -- Tests:233: attempt to call a nil value (method 'isAlive')

Saucer needs an isAlive() method, returning true (for this test to pass so far).

The test passes. Of course, it shouldn’t. At creation time of the army, the saucer should not be alive.

        _:test("army creates saucer", function()
            local army = Army()
            _:expect(army:getSaucer():isAlive()).is(false)
            -- something happens
            _:expect(army:getSaucer():isAlive()).is(true)
        end)

This will fail on the false and we’ll fix it to fail on the true, because something hasn’t happened yet:

Saucer = class()

function Saucer:init()
    self.alive = false
end

function Saucer:isAlive()
    return self.alive
end

The flag seems sufficient, but now the decision as to how life is determined is hidden inside Saucer in a way that it is not hidden in our other moving objects. This is better, I think.

Now, what has to happen?

We think we know some of the rules. The invaders have to be at least one or two rows down. Watching old videos, it looks to me as if the invaders have to be two rows down before the saucer comes out. And it doesn’t come out immediately, so there are other rules.

We can TDD each of the rule conditions, or we can just assume that we’ll fill in setup on this test until everything is being handled under the covers. For now, let’s do that.

One condition is that the invaders are down two rows from where they start. Invaders have a known value, stepDown, which is presently vec2(0,-8). It’s also global. Let’s move it inside Army and then see how it gets used:

~~~lua function Army:init(player) self.player = player … self.bombCycle = 0 self.reverse = -1 self.stepDown = vec2(0,-8) end

function Army:adjustMotion() if self.overTheEdge then self.overTheEdge = false self.motion = self.motion*self.reverse + self.stepDown else self.motion.y = 0 – stop the down-stepping end end

In a marvelous bit of previous design, `overTheEdge` is set here:

~~~lua
function Army:invaderOverEdge(invader)
    self.overTheEdge = true
end

This suggests to me that we can almost … almost … tell the army it’s over the edge a couple of times and then it should bring the saucer to life. This isn’t quite true unless we were to make it count down the rows or something. After motion is set in adjustMotion, it has to be used, here:

function Army:update()
    self:updateBombCycle()
    self:possiblyDropBomb()
    local continue = true
    while(continue) do
        continue = self:nextInvader():update(self.motion, self)
    end
    self.rollingBomb:update(self)
    self.plungerBomb:update(self)
    self.squiggleBomb:update(self)
end

Let’s not be too fanatical here. Let’s update just the first invader, because we expect she’ll be the one used to check our level.

Here’s roughly what we want:

        _:test("army creates saucer", function()
            local army = Army()
            _:expect(army:getSaucer():isAlive()).is(false)
            army:overTheEdge()
            army.invaders[1]:update(army)
            _:expect(army:getSaucer():isAlive()).is(false)
            army:overTheEdge()
            army.invaders[1]:update(army)
            _:expect(army:getSaucer():isAlive()).is(true)
        end)

I want to know what the y coordinates are, because I intend to use them in the decision of when to trigger the saucer. So:

        _:test("army creates saucer", function()
            local army = Army()
            local y
            y = army.invaders[1].pos.y
            _:expect(y).is(0)
            _:expect(army:getSaucer():isAlive()).is(false)
            army:overTheEdge()
            army.invaders[1]:update(army)
            _:expect(army:getSaucer():isAlive()).is(false)
            army:overTheEdge()
            army.invaders[1]:update(army)
            y = army.invaders[1].pos.y
            _:expect(y).is(0)
            _:expect(army:getSaucer():isAlive()).is(true)
        end)

These two new checks on y will surely fail, and they’ll tell me the magical values I need for later. They’ll also reassure me that the invaders are really stepping down based on the calls I’m making.

Bad method name. Said overTheEdge, should have said invaderOverEdge:

        _:test("army creates saucer", function()
            local army = Army()
            local y
            y = army.invaders[1].pos.y
            _:expect(y).is(0)
            _:expect(army:getSaucer():isAlive()).is(false)
            army:invaderOverEdge()
            army.invaders[1]:update(army)
            _:expect(army:getSaucer():isAlive()).is(false)
            army:invaderOverEdge()
            army.invaders[1]:update(army)
            y = army.invaders[1].pos.y
            _:expect(y).is(0)
            _:expect(army:getSaucer():isAlive()).is(true)
        end)

I got a nice failure on my first assert:

21: army creates saucer  -- Actual: 105.0, Expected: 0

But then:

21: army creates saucer -- Invader:27: attempt to index a nil value (local 'army')

function Invader:update(motion, army)
    if not self.alive then return true end
    army:reportingForDuty()
    self.picture = (self.picture+1)%2
    self.pos = self.pos + motion
    Shield:checkForShieldDamage(self)
    if self:overEdge() then
        army:invaderOverEdge(self)
    end
    return false
end

I’m not calling the update with the right information. Let’s not reach so far down in the object hierarchy. Instead, let’s have Army do it.

Arrgh

Guess who forgot to commit the code after Saturday’s emergency fix? This programmer. Guess what happened when this programmer reverted the code. That bug came back.

We could save what we have in a branch, fix this, etc etc., but in fact our time is up. I’ve put that fix back in and am committing: fix runner global bugs.

What have we learned? Well, be sure you have a good commit to revert to, but I was certain I had. I was wrong.

Have tests good enough to run, and run them after a revert. That would have shown the problem and I might have noticed.

Aside from that, we’ve learned a lot about how to do the Saucer. I think tomorrow will be a better day.

No changes. Day wasted, except for some painful learning. So it goes. Tomorrow it’ll be better.

I wish I didn’t have to tell you about today, but I promised, warts and all. Today: warts.

See you next time!