Space Invaders 60

Touch to Start has me thinking that attract mode might be within reach. Things go Very Wrong.

NARRATOR (V.O.): This article is a long game of whack-a-mole. It ends with the problems all solved. Then finally Ron reverts the code. You’ll understand why.

Yesterday we worked on “Touch to Start”, so that players could play more than one game. The present implementation is pretty messy, if I correctly recall, but it has provided a few different game states. When the game is over, we go to touch to start mode, but the invaders are still marching. That was intentional, according to me. In that state, it shouldn’t take much to spawn a robotic player and drive it around shooting. That’s the original game’s basic attract mode, although there is another phase to that mode.

In my reflections before sleeping, and after starting to wake up, I was thinking I might draw a little picture of the various “modes” the game could be in, and do some design thinking about how a higher-level form of control “should” be implemented. Maybe I’ll still do that at some point, but now that I’m here, I think it would be more interesting to let the code guide us. We have a number of things tangled together managing the touch to start behavior, but there isn’t much to it. If we can see enough commonality to factor out an object, it seems to me that that object will automatically be a perfect center for a top level game controller.

So we’ll take a look and see.

Review of Touch to Start

Let’s start by seeing what we did. Maybe it’s not as messy as I remember. Maybe it’s worse.

We have pulled out starting the game from Main:setup:

function setup()
    runTests()
    parameter.boolean("Cheat", false)
    Line = image(208,1)
    for x = 1,208 do
        Line:set(x,1,255, 255, 255)
    end
    Runner = nil
end

function startGame()
    Runner = GameRunner()
    SoundPlayer = Sound()
    invaderNumber = 1
end

The GameRunner instance, Runner, can be nil now, or a GameRunner, and we use that as a flag in a number of places. In Main:touched, we do this:

function touched(touch)
    if Runner and Runner:livesRemaining() ~= 0 then
        Runner.player:touched(touch)
    else
        if touch.state == ENDED then
            testsVisible(false)
            startGame()
        end
    end
end

As long as there’s a Runner with lives available, we pass the touch on to that Runner. Failing that, when the touch ends, we turn off the visibility of the unit tests, and start a new game. Since the Runner starts nil, the starting screen is blank except for the unit tests and the touch to start words:

At the end of any game, any leftover invaders still run, there is no available gunner, and the touch message appears:

Control of what’s displayed looks like this:

function draw()
    pushMatrix()
    pushStyle()
    background(40, 40, 50)
    showTests()
    if Runner then Runner:draw() end
    if not Runner or Runner:livesRemaining() == 0 then
        drawTouchToStart()
    end
    popStyle()
    popMatrix()
    if Runner then Runner:update() end
end

Based on a search for access to Runner, I’m fairly sure that these are the only changes required to give us the touch to start behavior. That’s good news.

The not so good news is that we’re treating existence of a Runner as a flag for top level behavior, and that usage is spread around amid other unrelated code. A good question is “what are we going to do about it?”

NullRunner?

What if there was another class, NullRunner, that mimicked enough of GameRunner’s behavior to let us just replace Runner with a NullRunner and send all the messages to it? That might remove all this iffy behavior from Main.

A related thought comes to mind. Right now, Runner is initialized to nil. I believe it never gets set to nil again. I did that to get touch to start to appear at the beginning of the game. What if, instead, we create Runner in setup and set lives to zero? Maybe we have to call startGame as well.

It seems to me that that might start a “game over” game running and Touch to Start would come up. Then I could remove all those checks for Runner being nil.

Let’s try it.

Looking at the code, it seems clear that we should just call startGame from setup. But if we do, GameRunner will start a game, and create a LifeManager with three lives. Let’s give startGame and GameRunner:init a parameter, the number of lives to start with:

function setup()
    runTests()
    parameter.boolean("Cheat", false)
    Line = image(208,1)
    for x = 1,208 do
        Line:set(x,1,255, 255, 255)
    end
    startGame(0) -- <---
end

function startGame(numberOfLives) --- <---
    Runner = GameRunner(numberOfLives) -- <---
    SoundPlayer = Sound()
    invaderNumber = 1
end

function touched(touch)
    if Runner and Runner:livesRemaining() ~= 0 then
        Runner.player:touched(touch)
    else
        if touch.state == ENDED then
            testsVisible(false)
            startGame(3) -- <---
        end
    end
end

And, of course:

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.player = Player()
    TheArmy = Army(self.player)
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

This may break some tests. I’ll look ahead of time for a change. There were two, and I set them to init with 3, which should ensure they get the same behavior as they expect.

Now to run. And it works just as I’d hoped. Invaders marching, Touch to Start on screen. It looks like the Game Over from the last game played. I think that’s perfect.

Now I can remove those checks for Runner existing. I’d better go carefully to be sure that none of them break anything:

function touched(touch)
    if Runner and Runner:livesRemaining() ~= 0 then
        Runner.player:touched(touch)
    else
        if touch.state == ENDED then
            testsVisible(false)
            startGame(3)
        end
    end
end

This if Runner can be removed. I’m tempted to always pass the touch to the runner, but for now we need it up here. So:

function touched(touch)
    if Runner:livesRemaining() ~= 0 then
        Runner.player:touched(touch)
    else
        if touch.state == ENDED then
            testsVisible(false)
            startGame(3)
        end
    end
end

That’s still rather invasive but it’s better. Now in draw:

function draw()
    pushMatrix()
    pushStyle()
    background(40, 40, 50)
    showTests()
    if Runner then Runner:draw() end
    if not Runner or Runner:livesRemaining() == 0 then
        drawTouchToStart()
    end
    popStyle()
    popMatrix()
    if Runner then Runner:update() end
end

This becomes:

function draw()
    pushMatrix()
    pushStyle()
    background(40, 40, 50)
    showTests()
    Runner:draw()
    if Runner:livesRemaining() == 0 then
        drawTouchToStart()
    end
    popStyle()
    popMatrix()
    Runner:update()
end

This seems to work just fine. No surprise, except that I’m always surprised when things work.

This is simpler than what we had yesterday, with the same behavior. It’s like we went around three sides of the block yesterday, only to discover that we could have gone straight to our current behavior by adding a test visibility feature, pulling out startGame and tossing in two quick checks on lives. Who knew?

And that’s the point. No one knew, at least no one at my house. I went through a couple of phases of how it looked, and it was only when I was faced with the game behavior that I knew what I wanted next.

And the rule is “make it work, make it right (and if need be, make it fast)”. Yesterday was “make it work”. Today, so far, has been “make it right”.

Time for chai run. I’ll BRB.

Chai Run Musings

I drive rather briskly,¹ so I don’t have a lot of spare bandwidth for thinking about programming. That’s a good thing for the squirrels² and such, and it might be good for the program. I don’t think about details, but impressions can form.

I’m a little disappointed about this morning’s results so far. I had rather hoped that some little object would pop out, one that could serve as a place to stand while building more robust behavior outside of regular game play. That was not to be: what we have here is better for the current need.

As frequent readers know, I try not to build “for the future”, but only to build what we need now, while building it to be as well-structured as is reasonable given our finite time and capacity. I developed that habit in part because the YAGNI³ principle served us well back on the original XP project, and since then, I’ve continued, because my programs don’t matter. That lets me push various practices to the limit just to see what happens.

Most of us fear that if we don’t get the design correct now, later on we will suffer. We’re afraid that we need to put in generality to deal with unknown demands of the future. The YAGNI principle says, no, just build what you need, and build it well, and when the future comes, you’l be as ready as you need to be.

There’s a reason why this is a good principle. I like to say “Modularity works”. A good modular design is flexible. Things are where they belong, things that should be apart are apart, and that means that if some new thing is needed, there will very likely be a place for it to exist. If our modularity were perfect, there would quite likely be just one place for our new capability–because we do everything once, in a single place.

Still, I had hoped for a place to stand to deal with attract mode, high score, and the like. So far, that place has not arisen. We won’t try to build the place: we’ll build the next things, and let the place be discovered.

What is the next thing?

There are lots of possible next things, but the one that I think might be fun to start on would be the part of attract mode that plays the game. When Space Invaders is not playing a game, it cycles through a few attract mode behaviors. One of those is a machine-played game.

The invaders are on the screen, marching. A Gunner appears, and moves around shooting at the invaders, just like a human player would. Sooner or later, the invaders bomb the player, and the next cycle of attract mode begins.

So I want a game-controlled player to come on screen when we’re in attract mode. We don’t have an attract mode indicator yet, but we do have the fact that there are no lives left.

In due time, what we’ll want is for GAME OVER to come up, leaving the screen as is for a while, and then after some delay, the game’s gunner comes up and starts shooting. We’ll treat the delay separately from the gunner appearing.

We now have a GameRunner running all the time. So let’s look and see what it does when lives are zero, and see what we can gin up.

In draw, we do this:

function GameRunner:draw()
    pushMatrix()
    pushStyle()
    noSmooth()
    stroke(255)
    fill(255)
    self:scaleAndTranslate()
    TheArmy:draw()
    self.player:draw() -- <---
    Shield:drawShields()
    self:drawStatus()
    popStyle()
    popMatrix()
end

The Player does this:

function Player:draw()
    pushMatrix()
    pushStyle()
    self.missile:draw()
    tint(0,255,0)
    self:drawingStrategy()
    popStyle()
    popMatrix()
end

And the drawing strategies are these:

function Player:drawNothing()
end

function Player:drawPlayer()
    sprite(asset.play, self.pos.x, self.pos.y)
end

function Player:drawExplosion()
    local select = (self.count//6)%2 + 1
    sprite(self.explosions[select], self.pos.x, self.pos.y)
end

When all the lives are gone, we’ll have set drawingStrategy to drawNothing.

We need to look at update as well. Player update is called on the 60ths clock, and it goes like this:

function Player:update()
    self.missile:update()
    if self.alive then
        self.pos = self.pos + self.gunMove + vec2(self:effectOfGravity(),0)
        self.pos.x = math.max(math.min(self.pos.x,208),0)
    end
    self:manageCount()
end

Just musing. If the game-driven Player is alive, we could somehow turn off response to touch and instead drive position by computation. Better yet, maybe it has more than two states, alive true and false, but a third state zombie or something.

The manageCount looks like this:

function Player:manageCount()
    if self.count <= 0 then return end
    self.count = self.count - 1
    self:manageExplosion()
    self:manageSpawning()
end

We don’t really need to look at the details here, the names tell us what we need to know, but they are illustrative:

function Player:manageSpawning()
    if self.count <= 0 then
        Runner:requestSpawn()
    end
end

function Player:manageExplosion()
    if self:explosionOver() then self.drawingStrategy = self.drawNothing end
end

Nice bitty little methods. We note that when the explosion is over we start drawing nothing, and when the count finally drops to zero we try to spawn.

Once the count has gone to zero, manageCount does nothing. If we successfully spawn a new player, this happens:

function Player:spawn(pos)
    self.alive = true
    self.drawingStrategy = self.drawPlayer
    self:setPos(pos)
end

Sooner or later, if the new player explodes, count will get set and we’ll start managing the deal throes again

I want to look at one more thing, touch handling.

function Player:touched(touch)
    local fireTouch = WIDTH-195
    local moveLeft = 97
    local moveRight = 195
    local moveStep = 1.0
    local x = touch.pos.x
    if touch.state == ENDED then
        self.gunMove = vec2(0,0)
        if x > fireTouch then
            self:fireMissile()
        end
    end
    if touch.state == BEGAN or touch.state == CHANGED then
        if x < moveLeft then
            self.gunMove = vec2(-moveStep,0)
        elseif x > moveLeft and x < moveRight then
            self.gunMove = vec2(moveStep,0)
        end
    end
end

This whole function either fires a missile, or sets a value into self.gunMove, which is used, as we saw, to move the gunner. It seems to me that we should condition all this behavior on the gunner being alive. Right now, changing those values is harmless, but if we’re driving the gunner, we don’t want the touch controls active at this level.

But it seems to me that we need those three states, dead, alive, zombie. Hm. This is interesting. I want to change the implementation of whether the player is alive, from a boolean member variable to a more powerful structure. To do that, let’s first build a new function, isAlive:

function Player:isAlive()
    return self.alive
end

Then let’s find all the accesses to the flag and convert them to use this function.

There are unfortunately lots of objects with an alive flag. Once we have a poor idea, we like to use it a lot. But all I need to worry about are the ones on Player. Plus I have an idea which I’ll get to after I find and fix the ones I see.

The first one I see is this:

function Player:init(pos)
    self:setPos(pos)
    self.count = 210 -- count down to first player
    self.missile = Missile()
    self.gunMove = vec2(0,0)
    self.explosions = { readImage(asset.playx1), readImage(asset.playx2) }
    self.missileCount = 0
    self.alive = false
    self.drawingStrategy = self.drawNothing
end

Since I’m working to get rid of that flag:

function Player:init(pos)
    self:setPos(pos)
    self.count = 210 -- count down to first player
    self.missile = Missile()
    self.gunMove = vec2(0,0)
    self.explosions = { readImage(asset.playx1), readImage(asset.playx2) }
    self.missileCount = 0
    self:die()
    self.drawingStrategy = self.drawNothing
end

function Player:die()
    self.alive = false
end

My plan is to wind up with just a very few accesses to alive so that I can then implement my new scheme.

function Player:spawn(pos)
    self.alive = true
    self.drawingStrategy = self.drawPlayer
    self:setPos(pos)
end

This becomes:

function Player:spawn(pos)
    self:live()
    self.drawingStrategy = self.drawPlayer
    self:setPos(pos)
end

function Player:live()
    self.alive = true
end

So far this seems oddly unnecessary. We’ll see. I have a plan and I’m going to finish it. I’m confident that if something simpler arises, we can switch to it.

NARRATOR (V.O.): I’m not sure whether Ron was already off-track at this point. Perhaps he was still OK here.

function Player:explode()
    if Cheat then return end
    Runner:playerExploding()
    self.alive = false
    self.count = 240
    self.drawingStrategy = self.drawExplosion
    SoundPlayer:play("explosion")
end

At last, reuse!

function Player:explode()
    if Cheat then return end
    Runner:playerExploding()
    self:die()
    self.count = 240
    self.drawingStrategy = self.drawExplosion
    SoundPlayer:play("explosion")
end

Then

function Player:update()
    self.missile:update()
    if self.alive then
        self.pos = self.pos + self.gunMove + vec2(self:effectOfGravity(),0)
        self.pos.x = math.max(math.min(self.pos.x,208),0)
    end
    self:manageCount()
end

That becomes:

function Player:update()
    self.missile:update()
    if self:isAlive() then
        self.pos = self.pos + self.gunMove + vec2(self:effectOfGravity(),0)
        self.pos.x = math.max(math.min(self.pos.x,208),0)
    end
    self:manageCount()
end

Similarly:

function Player:fireMissile()
    if self:isAlive() and self.missile.v == 0 then
        self:unconditionallyFireMissile()
    end
end

And that’s all there is in Player. I’ll do the same for the tests. Arrgh, there are a million Gunner.alive = true lines in there. I’ll see whether Codea replace will help me. Failing that I’ll paste the function into a real editor and do it. Woot! It did it.

There should be no functional change at this point. I’ll test.

NARRATOR (V.O.): Here is where the trouble really begins. This is a bug from two days ago, catching Ron unawares.

Tests all run, game mostly works … except when I shoot the saucer I get this:

Saucer:82: attempt to index a nil value (global 'Gunner')
stack traceback:
	Saucer:82: in method 'score'
	Saucer:71: in method 'die'
	Saucer:62: in method 'killedBy'
	Army:242: in method 'checkForKill'
	Army:222: in method 'processMissile'
	Missile:26: in method 'draw'
	Player:69: in method 'draw'
	GameRunner:24: in method 'draw'
	Main:36: in function 'draw'

I’m wondering how Gunner got to be nil here. The function is just:

function Saucer:score()
    return self.scores[Gunner:shotsFired()%15 + 1]
end

So clearly Gunner has turned up nil here. Where does that ever happen?

Arrgh! There’s not supposed to be a global Gunner at all. The GameRunner knows the gunner. So the Saucer needs to ask Runner for shots fired, not Gunner I wonder if that’s a typo. I’ll fix that. But that makes me wonder about those tests as well. We’ll look at those shortly.

NARRATOR (V.O.): Ron is getting more confused, but not realizing it yet. Our boy is in for big trouble if this doesn’t turn around.

Hm, I thought we had implemented shotsFired on runner. I guess not. So far it’s only on the Player. For now, I’ll put it on Runner so that saucer scoring can work, but I don’t entirely like it. And I see that yesterday, I implemented that (actually the night before) and then immediately broke it. I do have a broken unit test now but I expect that to recover when I do this:

function GameRunner:shotsFired()
    return self.player:shotsFired()
end

Whoa that broke a bunch more tests. Ah, the saucer scoring tests all broke. No real surprise there, that test (the one I just “fixed” isn’t using Gunner properly). We could promote the Gunner back to a global but I think that would be expedient but not good. First I want to see if the game can kill the saucer now.

It can. However, in my first game I saw that odd behavior where the saucer doesn’t appear at all. I restarted the program and the next time it did appear. There may be a lurking problem.

Now let’s look at those tests.

This is getting frustrating. A wiser person might revert, but the yak I’m shaving was really there before we started this exercise. Might still be the thing to do but I’m gonna shave it.

NARRATOR (V.O.): Too right. This would have been a perfect time to revert. Whatever happened after that would have been better than this. He’s off the rails.

Most of the expect statements here are failing:

        _:test("Saucer Score depends on shots fired", function()
            local army = Army()
            Gunner = Player()
            local s = army.saucer
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(50)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(50)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(150)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(50)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(300)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(50)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(150)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)
            _:expect(s:score()).is(100)
        end)

That’s because we created our own Gunner. We should use the one in the GameRunner. But do we even have a GameRunner at this point? I don’t think so. This is irritating and I think there’s a lesson here to be considered.

A Lesson?

We’ve just connected a bottom-most object, the Saucer, all the way back to the top of the system, by letting it talk to Runner to get the number of shots fired by the gunner. That makes a big loop in the references: Runner knows Army, Army knows Saucer, and Saucer knows Runner. Runner knows Player and player knows shots fired. This means that for Saucer to know its score, it has to know shots fired, the Runner, Player, Army, and Saucer all have to exist.

NARRATOR (V.O.): Realizing there’s a dependency loop was a sure sign that Ron is on the wrong track. Even he should have known to stop and think at this point.

That’s no fun. I don’t mind creating a couple of objects to get a test to work, but this is too much. Probably the test will run if I create a GameRunner, but I hate it. We’ll try that first, using before and after:

    _:describe("Invaders Test Suite", function()

        _:before(function()
            --Missile = {v=0, p=vec2(0,0)}
            --createBombTypes()
            Runner = GameRunner(3)
            SoundPlayer = Sound(false)
            Line = image(1,1)
        end)

        _:after(function()
            GameRunner = nil
        end)

I’m not sure about nilling the GameRunner, we just set things up so that should never happen. We’ll see.

That doesn’t work, Something in CodeaUnit can’t find GameRunner. I’m not going after that yak. I’ll fix it in the test.

        _:test("Saucer Score depends on shots fired", function()
            Runner = GameRunner(3) -- can't be local
            local Gunner = Runner.player
            local s = Runner.army.saucer
            _:expect(s:score()).is(100)
            Gunner:unconditionallyFireMissile(true)

It fails in a new way. [EXPLETIVE DELETED].

32: Saucer Score depends on shots fired -- Tests:374: attempt to index a nil value (field 'army')

NARRATOR (V.O.): Will he never learn? When the program makes you mad, it’s time to back off, calm down, and, probably, revert.

GameRunner creates a global TheArmy, not a member variable. We’ll first set a member variable, then deal with the global when the tests run.

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.player = Player()
    self.army = Army(self.player)
    TheArmy = self.army
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

Tests are green. Now references to TheArmy? In GameRunner there are lots. We can change them all to self.army.

Army has this oddity:

function Army:addToScore(added)
    self.score = self.score + added
    TheArmy:adjustTiming(self.score)
end

Why can’t that say self? If they’re identical it must be possible. Changed.

Missile wants to know

function Missile:handleOffScreen()
    if self.pos.y > TheArmy:saucerTop() then
        self.explodeCount = 15
    end
end

Let him ask the runner. The runner knows everyone. But won’t that give us another pernicious loop in who knows whom?

Strictly speaking, we might say we should pass down to all objects pointers to the objects they need to talk with. Then in our tests we could plug in fakes if we need to. But that’s a lot of passing things around and there’s essentially no legitimate reason why the missiles should know the army or anything about it. But they do need to know their range. That would be a legitimate thing for them to be told. If that were the case, the Player should know the range. It would be told in init:

function Player:init(pos, missileRange)
    self:setPos(pos)
    self.count = 210 -- count down to first player
    self.missile = Missile(missileRange or 205)
    self.gunMove = vec2(0,0)
    self.explosions = { readImage(asset.playx1), readImage(asset.playx2) }
    self.missileCount = 0
    self:die()
    self.drawingStrategy = self.drawNothing
end

I’m defaulting it to 205 because that’s the right answer. So sue me. Now missile:

Missile = class()
    function Missile:init(range)
    self.range = range or 205
    self.v = 0
    self.p = vec2(0,0)
    self.explosion = readImage(asset.player_shot_exploding)
    self.explodeCount = 0
end


function Missile:handleOffScreen()
    if self.pos.y > self.range then
        self.explodeCount = 15
    end
end

Now to pass it in before we forget:

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.army = Army(self.player)
    self.player = Player(self.army:saucerTop())
    TheArmy = self.army
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

Nota Bene

I’ve been juggling a lot of balls for way too long. A wise man would revert and start over when fresh. But I think we’re nearly there. Let’s find out.

32: Saucer Score depends on shots fired -- Player:131: bad argument #-2 to '__add' (vec2)

What even is that?

function Player:unconditionallyFireMissile(silent)
    self.missile.pos = self.pos + vec2(7,5) -- line 131
    self.missile.v = 1
    self.missileCount = self.missileCount + 1
    if not silent then SoundPlayer:play("shoot") end
end

That means the Player pos is nil. How was it ever not nil?

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.army = Army(self.player)
    self.player = Player(self.army:saucerTop())
    TheArmy = self.army
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

Anyway this needs to set it:

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.army = Army(self.player)
    self.player = Player(vec2(0,0),self.army:saucerTop())
    TheArmy = self.army
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

I’m just bashing and looking for bigger hammers. This better unwind soon. I really should revert but I just can’t make myself do it. Sunk cost fallacy. But we’re nearly there, I just know it. One more test fails:

NARRATOR (V.O.): He knows he’s in trouble but he just keeps on going. Gotta give him credit for guts if not brains.

19: rolling shot finds correct column -- Army:142: attempt to index a nil value (field 'player')

Army expects a player. I bet that test didn’t provide one. (We’re trying to get away from TheArmy global.)

        _:test("rolling shot finds correct column", function()
            local army = Army(Gunner)
            local Gunner = Player(army:saucerTop())
            local bomb = army.rollingBomb
            army:setPosition(vec2(10,180))

Yes, well, it can’t know the right gunner here, can it?

Can I just reverse those inits?

        _:test("rolling shot finds correct column", function()
            local Gunner = Player(Army:saucerTop())
            local army = Army(Gunner)
            local bomb = army.rollingBomb

Yes. Tests running as intended. Testing the game. Arrgh: firing a shot I get:

Army:142: attempt to index a nil value (field 'player')
stack traceback:
	Army:142: in function <Army:141>
	(...tail calls...)
	Army:112: in method 'dropBomb'
	Army:74: in method 'dropRandomBomb'
	Army:62: in method 'possiblyDropBomb'
	Army:155: in method 'update'
	GameRunner:88: in method 'update60ths'
	GameRunner:76: in method 'update'
	Main:42: in function 'draw'

This has nothing to do with firing. Run again and wait. Right, it’s trying to drop a bomb, probably the rolling one:

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

We should have a player unless someone creates an army without one. And:

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.army = Army(self.player)
    self.player = Player(vec2(0,0),self.army:saucerTop())
    TheArmy = self.army
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

Can’t use it if you haven’t created it. These two lines depend on each other as written. However:

function GameRunner:init(numberOfLives)
    self.lm = LifeManager(numberOfLives, self.spawn, self.gameOver)
    Shield:createShields()
    self.player = Player(vec2(0,0),Army:saucerTop())
    self.army = Army(self.player)
    TheArmy = self.army
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

The saucerTop method is effectively a class method, so this will work. My hammer is very heavy at this point. I’m going to have to have a stern talk with myself, whether this ever works or not.

A test fails:

19: rolling shot finds correct column -- Bomb:95: attempt to call a nil value (method 'targetedColumn')

What even IS THIS??? I’m thrashing. No rational person would do anything but revert, scrap this article and start tomorrow pretending this never happened.

NARRATOR (V.O.): Best idea of the day. Even now he could scrap this and pretend it never happened. But will he? No.

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

For that to fail, army must have been nil on input. Who’s calling nextColumn?

function Army:dropBomb(aBomb)
    if aBomb.alive then return end
    local col = aBomb:nextColumn(self)
    local bombPos = self:bombPosition(col)
    if bombPos ~= nil then
        aBomb.pos = bombPos
        self.bombCycle = 0
        aBomb.alive = true
    end
end

That can’t be nil unless we called it on the class. Who calls dropBomb?

OK, that was bizarre. When I went to copy targetedColumn above, I apparently cut it instead. Putting it back makes the game run but two tests fail. My bad, I had an assert to check for something. Now I expect things to work, finally.

Tests are green except for that ignored one, which is fixed, by the way and can be removed.

        _:ignore("Lives hack initialized to 4", function()
            
        end)
        

That was fixed with LifeManager. Tests are green, game plays.

I’d commit the code but I’ve forgotten what the heck we were doing before the yak invasion started.

Let’s see. We set out to put in a rudimentary attract mode, which mostly involved just starting the game with zero gunners. When we set out to refactor the alive flag to allow for a new “zombie” state. We never got to the new state because, first, we crashed trying to to kill a saucer, which I believe was an old bug.

I should have committed at that point, saving the refactoring of alive status. But I chased the bug. That turned out to be bizarre. I think much of the trouble was due to trying to unwind the connections between the objects. Some of them know too much about others.

You know what? Having beaten this thing, thus feeling that I’m not as stupid as I’ve been feeling.

I’m going to revert.

Yes. I’m going to trash all of today’s work, including the good bits, and start over tomorrow. Whatever I’ve done here has been hammering, not clear thinking. I don’t know what’s wrong with the code base right now, and if I’m very lucky it might even be OK. But I have no right to assume that.

I’m reverting, now. A test run shows me that the saucer bug was there when we started. I’m not going to try to fix it. Someone has taken my keys and offered to drive me home.

NARRATOR (V.O.): At last he does the right thing. We’ll see down the road whether he has learned a lesson or just collapsed in a heap, weeping.

Done. Let’s sum up. We need to talk.

Summing Up

There was a defect in saucer-killing from the get-go, and I didn’t know it was there. The release from Wednesday had a fatal error, another error not detected by the tests.

The defect had to do with removing one of the globals, in this case Gunner. Somehow, and I’m not clear just how, I started chasing my tail, fixing one thing only to have another pop up. It was a perfect game of whack-a-mole.

By any reasonable standard, I should have stopped, reverted, and proceeded more carefully, first fixing the bug, then working on the new idea for attract mode. Instead, I kept bashing my head against the problem. After a while, I was quite ready to give up except that I wanted to see if I could ever fix the thing or whether I would somehow just explode.

Once it was working, it didn’t even hurt to revert: it was the obvious thing to do. No one could have worked that long, blindly, and left good things behind.

In part, I wanted to show that even with the best of will, and the best good habits, I can fall into the swamp with the best of them. And in part, I just wanted to conquer the moles even if it would have taken far less time to stop.

NARRATOR (V.O.): Mostly he was just too stupid to do the right thing even when he knew what it was. Sure, he’ll say it was to help you learn from his mistakes. I’m not buying it.

Certainly for what we got, stopping would have been better, because I reverted to be sure we got nothing bad along with any good.

Tomorrow is another day. Or maybe later today, but probably tomorrow.

That was … fun. See you next time!