Users have detected a problem. Users, do you hear me? Users!

Dave 1707 and Bri_G from the Codea forum report that my invaders start too far down the screen. I don’t recall whether I started them according to my reading of the original code or more whimsically, but Dave took a picture that’s pretty convincing:

dave pic

It’s easy to make the girls start higher up. I did it last night. There’s this code in the MarshallingCenter:

function MarshallingCenter:marshalArmy(army)
    local invaders = {}
    for row = 1,5 do
        for col = 11,1,-1 do
            local p = vec2(col*16, 185-row*16)
            table.insert(invaders, 1, Invader(p,self.vaders[row], self.scores[row], army))
        end
    end
    return invaders
end

It is intuitively obvious to the casual observer, I’m sure, that the rows of invaders will start at y position 169, and array themselves also at 153, 137, 121, and 105. Yeah, maybe not quite intuitively. Those do not seem like the sort of values that would appear in an assembly-language program. I’ll try to figure out from the original code where they started back then.

But wait, there’s more! You also get to decide when to allow saucers to start flying, which is after two steps downward, and the code is this:

function Army:runSaucer()
    if self.saucer:isRunning() then return false end
    if self:yPosition() > 89 then return false end
    if math.random(1,20) < 10 then return false end
    self.saucer:go(Player:instance():shotsFired())
    return true
end

Yes, that’s right, when the y coordinate is less than or equal to 89, the saucer can fly. 105 - 89, as all know, is 16, which is in fact two steps downward, because

local reverse = -1
local stepDown = vec2(0,-8)

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

The Y position of the saucer is, of course:

function Army:saucerTop()
    return 205
end

That number minus the invaders’ highest y coordinate, 169, is 36, or, in technical terms, not obviously related.

Truly, whoever wrote this must have a dizzying intellect. But there’s more:

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

This, at least, is tied to a name. The rest of the information on where the invaders go, how they move, and what happens near them … all that information is spread around the program in literal constants. There’s no coherent order, nothing organized about them at all. I am not at all certain that there aren’t magical things in still more places, tied to that 185 we saw at the beginning in mysterious unwritten ways.

This is not good design. While we move our invaders up a bit, let’s see about improving this aspect of the design.

In my defense, I’ve mentioned a few times that there were getting to be a lot of magic numbers up in this um program, but the prosecution is quick to point out that this only shows that I was aware of the crime, and that I still committed it. This sort of thing happens in real life, of course. Our mission is to improve the situation.

An early question should be to ask who should be the owner of this information. Most of it is used by the Army, or its subordinate MarshallingCenter, but some of the information is used by the Missile, which belongs to the Player and is managed there.

This makes me think that this information package should belong to the GameRunner, which knows the Player and the Army, and is already subject to being discovered via the global Runner.

We could add methods to GameRunner, or we could add famous member variables. Either would broaden its interface in a way that I don’t like. I’m leaning toward giving GameRunner an instance of a new object, something like UniversalConstants, with a number of methods or member variables.

I am trending toward preferring methods, partly because that’s what Smalltalk did and there is Smalltalk in my blood, partly because using methods allows for things that seem like constants to their users but that can change dynamically if the bigger picture calls for it.

Right now, though, I’m going to review the old Space Invaders code and see if I can glean some facts.

Some Facts

I think I’ve seen and reported this before. If so, here it is again.

;##-AlienStartTable
; Starting Y coordinates for aliens at beginning of rounds. The first round is initialized to $78 at 07EA.
; After that this table is used for 2nd, 3rd, 4th, 5th, 6th, 7th, 8th, and 9th. The 10th starts over at
; 1DA3 (60).
1DA3: 60                                    
1DA4: 50                                    
1DA5: 48                                    
1DA6: 48                                    
1DA7: 48                                    
1DA8: 40                                    
1DA9: 40                                    
1DAA: 40   
The first wave starts at Y=78 (hex). The next wave starts 16 pixels (one row of the rack) lower at Y=50. Then the rack holds at 48 for three rounds and then 40 for three rounds. The value Y=40 is just above the player's shields.

Round 7 is as hard as the game gets. By that time the player's score is above 3000, which means the aliens reload their shots at the fastest rate. The aliens never start out lower than Y=40. If you can manage the game at level 7 you can play forever.

Hex 78 is decimal 120. That reminds me that Halloween == Christmas, because Oct 31 = Dec 25. This may explain the displays in the stores around this season.

So the lowest alien should start at 120. Our starts at 105, which is off by 15, or just about one row. This jibes with Dave and Bri’s observation that we’re about one row off. The fact that the starting Y position varies tells me that making the constant access via methods is probably better.

I’m going to build a UniversalConstants class and create one in GameRunner:

function GameRunner:init(numberOfLives)
    self.constants = UniversalConstants()
    self.lm = LifeManager(numberOfLives or 3, self.spawn, self.gameOver)
    Shield:createShields()
    Player:newInstance()
    TheArmy = Army()
    self.sounder = Sound()
    self.line = image(208,1)
    for x = 1,208 do
        self.line:set(x,1,255, 255, 255)
    end
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

UniversalConstants = class()

function UniversalConstants:init()
end

Now we could write tests for this class but I think it’s so trivial that there’s not much point. I may, of course, regret saying that. If I begin to feel regret, I’ll write tests.

Let’s apply our new, currently empty class, in the MarshallingCenter, where we began.

function MarshallingCenter:marshalArmy(army)
    local invaders = {}
    for row = 1,5 do
        for col = 11,1,-1 do
            local p = vec2(col*16, 185-row*16)
            table.insert(invaders, 1, Invader(p,self.vaders[row], self.scores[row], army))
        end
    end
    return invaders
end

Now the fact of interest is that the bottom row of invaders should start at Y = 120. That fact is well buried in our code here. Recall that it presently sets invaders to 169, 153, 137, 121, and 105. Our code here is drawing top down, and it needs to do that because I want the bottom left invader to be #1 in the array.

By golly, I see a useful test here! And a quick look tells me that we have a test to adjust:

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

This code tests my bombPosition code but that’s just fine. Those constants in the expect(y) calls are the very values we want for our rows when the new constants work.

We might prefer to do this in hex: the arithmetic is easier. How about this:

            y = lowestY(1)
            _:expect(y).is(0x78)
            army:directKillInvader(1)
            _:expect(lowestY(1)).is(0x88)
            _:expect(lowestY(2)).is(0x78)
            army:directKillInvader(1 + 11)
            _:expect(lowestY(1)).is(0x98)
            army:directKillInvader(1 + 2*11)
            _:expect(lowestY(1)).is(0xa8)
            army:directKillInvader(1 + 3*11)
            _:expect(lowestY(1)).is(0xb8)
            army:directKillInvader(1 + 4*11)
            _:expect(lowestY(1)).is(nil)

This test will now fail:

20: finding bottom-most invader  -- Actual: 105.0, Expected: 120

And so on. Now let’s patch in what we know from the original source:

function MarshallingCenter:marshalArmy(army)
    local bottomY = 0x78 -- <---
    local startY = bottomY + 5*16  -- <---
    local invaders = {}
    for row = 1,5 do
        for col = 11,1,-1 do
            local p = vec2(col*16, startY-row*16) -- <---
            table.insert(invaders, 1, Invader(p,self.vaders[row], self.scores[row], army))
        end
    end
    return invaders
end

Whee, the test runs again. Now let’s use our new universal constants. I’m thinking the syntax is about like this:

function MarshallingCenter:marshalArmy(army)
    local bottomY = Runner:constants():invadersBottomY()
    local startY = bottomY + 5*16
    ...

Now presently the member variable in GameRunner is constants, so we need to rename that or the access method. I vote for changing the member variable, and we’ll add the access method:

function GameRunner:init(numberOfLives)
    self.const = UniversalConstants()
    self.lm = LifeManager(numberOfLives or 3, self.spawn, self.gameOver)
    Shield:createShields()
    ...

function GameRunner:constants()
    return self.const
end

I think this should walk back accessing invadersBottomY, but it doesn’t, because the test doesn’t have Runner set up. I get this message, which breaks the tests entirely:

MarshallingCenter:19: attempt to index a nil value (global 'Runner')
stack traceback:
	MarshallingCenter:19: in method 'marshalArmy'
	Army:23: in method 'marshalTroops'
	Army:5: in field 'init'
	... false
    end

    setmetatable(c, mt)
    return c
end:24: in global 'Army'
	GameRunner:12: in field 'init'
	... false
    end

    setmetatable(c, mt)
    return c
end:24: in global 'GameRunner'
	Tests:9: in field '_before'
	CodeaUnit:44: in method 'test'
	Tests:19: in local 'allTests'
	CodeaUnit:16: in method 'describe'
	Tests:6: in function 'testCodeaUnitFunctionality'
	[string "testCodeaUnitFunctionality()"]:1: in main chunk
	CodeaUnit:139: in field 'execute'
	Main:66: in function 'runTests'
	Main:5: in function 'setup'

Oh neat. We’re creating the army, which wants the universal constants, so it calls upon Runner, but Runner isn’t set up yet because we’re still creating it

I see two possibilities: pass the GameRunner to Army, or make the universal constants a global all on its own. The former is better design. Let’s do that:

function GameRunner:init(numberOfLives)
    self.const = UniversalConstants()
    self.lm = LifeManager(numberOfLives or 3, self.spawn, self.gameOver)
    Shield:createShields()
    Player:newInstance()
    TheArmy = Army(self)
    self.sounder = Sound()
    self.line = image(208,1)
    for x = 1,208 do
        self.line:set(x,1,255, 255, 255)
    end
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

Now … we see this oddity:

function Army:init(player)
    self:initMemberVariables(player)
    self:marshalTroops()
    self:defineBombs()
end

function Army:initMemberVariables()
    self.weaponsAreFree = false
    self.armySize = 55
    self.invaderCount = 0
    self.invaderNumber = 1
    self.overTheEdge = false
    self.motion = vec2(2,0)
    self.bombDropCycleLimit = 0x30
    self.bombCycle = 0
    self.saucer = Saucer(self)
    self.score = 0
end

Army already gets a parameter, player, but it’s not really used. I’ll look and see if someone is even passing it. One test does, but since it’s clearly not doing anything, I just remove the parameter. Now we can rename it and use it:

function Army:init(runner)
    self:initMemberVariables()
    self:marshalTroops(runner)
    self:defineBombs()
end

And …

function Army:marshalTroops(runner)
    self.invaders = MarshallingCenter(runner):marshalArmy(self)
end

Finally, we get the desired error:

MarshallingCenter:20: attempt to call a nil value (method 'invadersBottomY')
stack traceback:
	MarshallingCenter:20: in method 'marshalArmy'
	Army:23: in method 'marshalTroops'
	Army:5: in field 'init'
	... false
    end

And we implement the method:

function UniversalConstants:invadersBottomY()
    return 0x78
end

Now we have a lot of calls to create Army to fix, in the tests. They all need a Runner. We have one, so I’ll just provide it, but I’m not happy.

Why am I not happy? Because I can no longer test the Army independently. I need to give it a GameRunner, all for the sake of one constant.

I apologize to my friends, and tip my hat to my foes, but this calls for Singleton. It also calls for a shorter name for the class.

~~~luaConstants = class()

local singleton

function Constants:instance() if not singleton then singleton = Constants() end return singleton end

function Constants:init() end

function Constants:invadersBottomY() return 0x78 end ~~~

We’ll back out the use of Runner in the setup of Army.

function MarshallingCenter:init()
    local vader11 = readImage(asset.inv11)
    local vader12 = readImage(asset.inv12)
    local vader21 = readImage(asset.inv21)
    local vader22 = readImage(asset.inv22)
    local vader31 = readImage(asset.inv31)
    local vader32 = readImage(asset.inv32)
    self.vaders = {
    {vader31,vader32},
    {vader21,vader22}, {vader21,vader22},
    {vader11,vader12}, {vader11,vader12}
    }
    self.scores = {30,20,20,10,10}
end

function MarshallingCenter:marshalArmy(army)
    local bottomY = Constants:instance():invadersBottomY()
    local startY = bottomY + 5*16
    local invaders = {}
    for row = 1,5 do
        for col = 11,1,-1 do
            local p = vec2(col*16, startY-row*16)
            table.insert(invaders, 1, Invader(p,self.vaders[row], self.scores[row], army))
        end
    end
    return invaders
end

And here:

function Army:init()
    self:initMemberVariables()
    self:marshalTroops()
    self:defineBombs()
end

And here:

function GameRunner:init(numberOfLives)
    self.const = UniversalConstants()
    self.lm = LifeManager(numberOfLives or 3, self.spawn, self.gameOver)
    Shield:createShields()
    Player:newInstance()
    TheArmy = Army() -- <---
    self.sounder = Sound()
    self.line = image(208,1)
    for x = 1,208 do
        self.line:set(x,1,255, 255, 255)
    end
    self:resetTimeToUpdate()
    self.weaponsTime = 0
end

Now I rather expect things to work, though my pessimistic side tells me something may blow up.

Oh right, there I was defining the wrong thing right there in GameRunner, which need know nothing of constants.

Now, however, the tests are green and the game plays. Let’s go after those other magic constants now. The saucer doesn’t run after two steps down any more, because of this:

function Army:runSaucer()
    if self.saucer:isRunning() then return false end
    if self:yPosition() > 89 then return false end
    if math.random(1,20) < 10 then return false end
    self.saucer:go(Player:instance():shotsFired())
    return true
end

Let’s rewrite that this way:

function Army:runSaucer()
    if self.saucer:isRunning() then return false end
    if self:yPosition() > Constants:instance():saucerFiringLimit() then return false end
    if math.random(1,20) < 10 then return false end
    self.saucer:go(Player:instance():shotsFired())
    return true
end

No, let’s call it saucerRunningLimit(), saucers run, they don’t fire.

function Constants:saucerRunningLimit()
    return self:invadersBottomY() - 2*self:saucerDownStep()
end

function Constants:saucerDownStep()
    return 8
end

The tests run and so does the game. What else did we have? I wrote that saucerDownStep for two reasons. First, to make it clear what the code means, but also because I know we use a similar value when we move the Army:

local reverse = -1
local stepDown = vec2(0,-8)

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

We would prefer not to recalculate this on every move, so let’s posit the value in a member variable and then init it. We’ll probably find that motion needs to be moved to Constants also.

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

We have this:

function Army:initMemberVariables()
    self.weaponsAreFree = false
    self.armySize = 55
    self.invaderCount = 0
    self.invaderNumber = 1
    self.overTheEdge = false
    self.motion = vec2(2,0)
    self.bombDropCycleLimit = 0x30
    self.bombCycle = 0
    self.saucer = Saucer(self)
    self.score = 0
end

We change to this:

function Army:initMemberVariables()
    self.weaponsAreFree = false
    self.armySize = 55
    self.invaderCount = 0
    self.invaderNumber = 1
    self.overTheEdge = false
    self.motion = Constants:instance():saucerSideStepVector()
    self.stepDown = Constance:instance():saucerDownStepVector()
    self.bombDropCycleLimit = 0x30
    self.bombCycle = 0
    self.saucer = Saucer(self)
    self.score = 0
end

And:

function Constants: saucerSideStepVector()
    return vec2(2,0)
end

function Constants:saucerDownStepVector()
    return vec2(0, -self:saucerDownStep())
end

I expect this all to work. Let’s see why it doesn’t.

Who the hell wrote “Constance”??? I haven’t thought of her for decades. Anyway with that fixed, everything works. The missiles seem to explode well above the saucer but we should fix them to refer to the constants.

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

This should really be a constant of its own name:

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

We also use saucerTop in starting the saucer, I reckon:

function Saucer:go(shotsFired)
    self.startPos = vec2(  8,185)
    self.stopPos  = vec2(208,185)
    self.step     = vec2(  2,  0)
    if shotsFired%2==1 then
        self.step = -self.step
        self.startPos,self.stopPos = self.stopPos,self.startPos
    end
    self.alive = true
    self.pos = self.startPos
    self.exploding = 0
    Runner:soundPlayer():play("ufoLo")
end

No, we have more magic numbers. And I found this:

function Missile:explosionColor()
    return self.pos.y > TheArmy:saucerTop() and color(255,0,0) or color(255)
end

Well, in for a penny on the saucerTop. First I’ll just move the functionality:

function Constants:missileTop()
    return 205
end

I found all the references to saucerTop and changed them. I expect things to work. Curiously enough, they do.

We’ve been a long time without a commit. Could have committed and shipped at any point along here where we were green. Bad habit on my part, to go heads down and keep plugging along even when I could ship it.

Let’s do commit, just to try to reverse the habit. Commit: moved invaders up, adding Constants class.

Now those magic numbers in the Saucer. And, by the way, I noticed the 8 here:

function Saucer:go(shotsFired)
    self.startPos = vec2(  8,185) -- <---
    self.stopPos  = vec2(208,185)
    self.step     = vec2(  2,  0)
    if shotsFired%2==1 then
        self.step = -self.step
        self.startPos,self.stopPos = self.stopPos,self.startPos
    end
    self.alive = true
    self.pos = self.startPos
    self.exploding = 0
    Runner:soundPlayer():play("ufoLo")
end

The saucer is 16 wide, as we see in the magic number here:

function Saucer:isHit(missile)
    if not self.alive then return false end
    return rectanglesIntersect(missile.pos,1,4, self.pos+vec2(4,0),16,8)
end

So presently it vanishes at x = 208, that is, just before it starts to go outside the screen limit of x = 224. Maybe it should go all the way. And certainly on the left it should at least go down to zero. I’ve noticed that if my player is off full left, I can’t hit the saucer because it vanishes too soon.

Let me see what happens if I init these to -16 and 224.

In landscape mode, where I have more screen width than there is game width, the saucer stays visible until it disappears all at once. In portrait mode, where the screen width is equal to the game width, it just derives off screen. I think I like that.

I also have seven tests failing, who are doubtless looking at some magic values:

25: Saucer(even) runs left to right and dies  -- Actual: (-16.000000, 185.000000), Expected: (8.000000, 185.000000)

And a couple of similar ones. Plus:

23: Saucer starts right, goes left, stops left if shots odd  -- Actual: (-16.000000, 185.000000), Expected: (8.000000, 185.000000)

This raises an interesting question. As it’s written right now, the start and stop positions are the same. So the saucer will start off screen and finish off screen (in portrait mode) and start outside the screen bounds in landscape. I wonder if I can tell what the original game does. I doubt that it starts off screen.

The original game says this:

; Setup saucer direction for next trip
0462: 7E              LD      A,(HL)              ; Shot counter
0463: E6 01           AND     $01                 ; Lowest bit set?
0465: 01 29 02        LD      BC,$0229            ; Xr delta of 2 starting at Xr=29
0468: C2 6E 04        JP      NZ,$046E            ; Yes ... use 2/29
046B: 01 E0 FE        LD      BC,$FEE0            ; No ... Xr delta of -2 starting at Xr=E0
046E: 21 8A 20        LD      HL,$208A            ; Saucer descriptor
0471: 71              LD      (HL),C              ; Store Xr coordinate
0472: 23              INC     HL                  ; Point to ...
0473: 23              INC     HL                  ; ... delta Xr
0474: 70              LD      (HL),B              ; Store delta Xr
0475: C9              RET                         ; Done

So the saucer starts at 0x29 and 0xE0, according to this. E0 is 224. I believe that. But 0x29 is 41, and I’m not at all sure that I believe that. That would start the saucer way in on the left.

I just viewed an invaders game video and in fact the saucer does start well in on the left side, and at the far right. So that’s interesting. It does seem to go all the way to the edge in both directions. I also notice that it is much slower than my saucer. Probably it doesn’t run on every tick.

I’m going to leave ours working as it is now, and just move the magic numbers into Constants. However, there’s no reason to do the call to Constants in go, we can save some time doing it in init. First I do this, which is safe:

function Saucer:init(army, optionalShotsFired)
    self.army = army
    self.startPos = vec2(  -16,185)
    self.stopPos  = vec2(224,185)
    self.exploding = 0
    self.scores = {100,50,50,100,150,100,100,50,300,100,100,100,50,150,100}
    if optionalShotsFired == nil then
        self.alive = false
    else
        self:go(optionalShotsFired)
    end
end

function Saucer:go(shotsFired)
    self.step     = vec2(  2,  0)
    if shotsFired%2==1 then
        self.step = -self.step
        self.startPos,self.stopPos = self.stopPos,self.startPos
    end
    self.alive = true
    self.pos = self.startPos
    self.exploding = 0
    Runner:soundPlayer():play("ufoLo")
end

Then to move those values to constants:

function saucerStartStopVectors()
    return vec2(-16,185), vec2(224,185)
end

Might as well keep them together as they are logically linked.

function Saucer:init(army, optionalShotsFired)
    self.army = army
    self.startPos, self.setPos = Constants:instance():saucerStartStopVectors()
    self.exploding = 0
    self.scores = {100,50,50,100,150,100,100,50,300,100,100,100,50,150,100}
    if optionalShotsFired == nil then
        self.alive = false
    else
        self:go(optionalShotsFired)
    end
end

I expect this to work, assuming against all evidence that I’ve not made a mistake. I have, though. Forgot to name the method:

function Constants:saucerStartStopVectors()
    return vec2(-16,185), vec2(224,185)
end

We still have those half-dozen tests failing:

23: Saucer starts right, goes left, stops left if shots odd  -- Actual: nil, Expected: (208.000000, 185.000000)

And so on. The nil return surprises me. The game does it too, when it gets around to starting the saucer. Better double check this:

    self.startPos, self.setPos = Constants:instance():saucerStartStopVectors()

Um “setPos”? What’s that??

    self.startPos, self.stopPos = Constants:instance():saucerStartStopVectors()

That will help, though the tests will still fail due to wrong expectations.

23: Saucer starts right, goes left, stops left if shots odd  -- Actual: (224.000000, 185.000000), Expected: (208.000000, 185.000000)

And so on. Before I fix this, I think I should fix the magical 185 in those vectors.

We want the saucer to run one row above the highest invader row, I believe. Since the bottom row is at 0x78, the highest is at 0x78 + 0x40, which is 0xb8 or 184. We want 0x78 + 0x50 then, or 200.

Naively:

function Constants:saucerStartStopVectors()
    local saucerY = self:invadersBottomY() + 0x50 -- 5 rows
    return vec2(-16,saucerY), vec2(224,saucerY)
end

That looks OK, although it runs across the test results. We’ll move those in due time. I note, however, that the saucer only runs once. Let’s see how it handles turning off its alive flag when it’s not shot down.

function Saucer:update()
    if not self.alive then return end
    local newPos = self.pos + self.step
    if self.startPos.x < self.stopPos.x and newPos.x <= self.stopPos.x then
        self.pos = newPos
    elseif self.startPos.x > self.stopPos.x and newPos.x >= self.stopPos.x then
        self.pos = newPos
    else
        self.alive = false
    end
end

I confess that that code isn’t clear to me at all. I guess what it’s saying is that depending on which is less, start or finish, we only need to check against the one value. I’m not fond of that now that one of the values can be negative.

The position is OK if newPos is strictly between start and stop, innit? Let’s say that:

function Saucer:update()
    if not self.alive then return end
    local newPos = self.pos + self.step
    if self:between(self.startPos.x, newPos.x, self.stopPos.x) then
        self.pos = newPos
    else
        self.alive = false
    end
end

function Saucer:between(a, b, c)
    local lo = math.min(a,c)
    local hi = math.max(a,c)
    return a <= b and b <= c
end

Let’s see what that does. OK, that’s worse for some reason. The saucer sound plays a lot but the saucer doesn’t appear, then after a while it appears a couple of times.

Oh. I didn’t use hi and lo. What a fool.

function Saucer:between(a, b, c)
    local lo = math.min(a,c)
    local hi = math.max(a,c)
    return lo <= b and b <= hi
end

There’s a better way to do that but first let’s get it to work. Still no good. I wish I had a recent commit to revert to. That should teach me, but it won’t. Let’s set the start and stop values back to what they were and observe.

First to this:

function Constants:saucerStartStopVectors()
    return vec2(-16,185), vec2(224,185)
end

And to this:

function Saucer:update()
    if not self.alive then return end
    local newPos = self.pos + self.step
    if self.startPos.x < self.stopPos.x and newPos.x <= self.stopPos.x then
        self.pos = newPos
    elseif self.startPos.x > self.stopPos.x and newPos.x >= self.stopPos.x then
        self.pos = newPos
    else
        self.alive = false
    end
end

Now it flies once right to left and then nothing. Revert to the earlier constants:

function Constants:saucerStartStopVectors()
    return vec2(8,185), vec2(208,185)
end

Even that is acting strange.

It turns out that I’ve only been working on saucer’s magic numbers since the last commit. That makes a revert less painful. Here goes.

Saucer behavior is back to normal, and the tests are green. Let’s proceed more slowly. First, move the saucer starting data into Constants, as it is:

Oh. I think I see what happened. I moved the startPos and stopPos code up to the init, calling it “safe”. It isn’t. Those two values are linked to step:

function Saucer:go(shotsFired)
    self.startPos = vec2(  8,185)
    self.stopPos  = vec2(208,185)
    self.step     = vec2(  2,  0)
    if shotsFired%2==1 then
        self.step = -self.step
        self.startPos,self.stopPos = self.stopPos,self.startPos
    end
    self.alive = true
    self.pos = self.startPos
    self.exploding = 0
    Runner:soundPlayer():play("ufoLo")
end

When the saucer reverses, all three values reverse, but next time we re-initialize step, which can allow us to try to step the wrong way. No good can come of that.

We’ll move all three to Constants:

However, I just noticed a bunch of mis-named constants:

function Constants:saucerRunningLimit()
    return self:invadersBottomY() - 2*self:saucerDownStep()
end

function Constants:saucerDownStep()
    return 8
end

function Constants: saucerSideStepVector()
    return vec2(2,0)
end

function Constants:saucerDownStepVector()
    return vec2(0, -self:saucerDownStep())
end

I don’t think these have anything to do with the saucer.

function Army:initMemberVariables()
    self.weaponsAreFree = false
    self.armySize = 55
    self.invaderCount = 0
    self.invaderNumber = 1
    self.overTheEdge = false
    self.motion = Constants:instance():saucerSideStepVector()
    self.stepDown = Constants:instance():saucerDownStepVector()
    self.bombDropCycleLimit = 0x30
    self.bombCycle = 0
    self.saucer = Saucer(self)
    self.score = 0
end

These are the invader motion values, not the saucer ones. We’ll rename.

function Army:initMemberVariables()
    self.weaponsAreFree = false
    self.armySize = 55
    self.invaderCount = 0
    self.invaderNumber = 1
    self.overTheEdge = false
    self.motion = Constants:instance():invaderSideStepVector()
    self.stepDown = Constants:instance():invaderDownStepVector()
    self.bombDropCycleLimit = 0x30
    self.bombCycle = 0
    self.saucer = Saucer(self)
    self.score = 0
end


function Constants:invaderDownStep()
    return 8
end

function Constants: invaderSideStepVector()
    return vec2(2,0)
end

function Constants:invaderDownStepVector()
    return vec2(0, -self:saucerDownStep())
end

There’s also this:

function Constants:saucerRunningLimit()
    return self:invadersBottomY() - 2*self:invaderDownStep()
end

That’s correct, used here:

function Army:runSaucer()
    if self.saucer:isRunning() then return false end
    if self:yPosition() > Constants:instance():saucerRunningLimit() then return false end
    if math.random(1,20) < 10 then return false end
    self.saucer:go(Player:instance():shotsFired())
    return true
end

Now if I’ve done this correctly we should still be good. But no:

function Constants:invaderDownStepVector()
    return vec2(0, -self:invaderDownStep())
end

OK, we’re good. Commit (dammit): rename misnamed constants.

Now let’s carefully move the saucer-starting constants, as they are, into Constants. This time I’m going to return all three linked values together:

function Constants:saucerStartInfo()
    return vec2(8,185), vec2(208,185), vec2(2,0)
end

And use them:

function Saucer:go(shotsFired)
    self.startPos, self.stopPos, self.step = Constants:instance():saucerStartInfo()
    if shotsFired%2==1 then
        self.step = -self.step
        self.startPos,self.stopPos = self.stopPos,self.startPos
    end
    self.alive = true
    self.pos = self.startPos
    self.exploding = 0
    Runner:soundPlayer():play("ufoLo")
end

I expect this to make zero change. And all is well. Now let’s adjust the Y:

function Constants:saucerStartInfo()
    local saucerY = self:invadersBottomY() + 0x50
    return vec2(8,saucerY), vec2(208,saucerY), vec2(2,0)
end

I expect this to change only how high the saucer runs. Everything is still fine. Commit: fixed saucer Y position. Used new Constants.

It has been a long enough day here. I paused at lunch time, came back in late afternoon. It’s nearly time for supper. Let’s sum up and compile this baby.

Summing Up

Not much to see here, other than a simple mistake confusing me for too long. Had I been more aware that had a good revert point, I hope I’d have reverted sooner, but my habits aren’t great in that regard. As usual, once I did, things went more smoothly, although I’ve not as yet gone as far as I tried to go with the previous try.

We now have the invaders starting higher, as requested by Bri_G and Dave1707. I’m not certain of the saucer Y value, because I can’t find it in the assembly code, but it’ll do for now.

Next time there are things to do with the saucer, including:

  • The saucer doesn’t fly if there are fewer than 8 invaders. (Or maybe 8 or fewer.)
  • Make saucer go full-screen side to side
  • When saucer explodes, the score value should appear beside it. (That may explain why it starts at x = 29, it occurs to me: to leave room for the display. I’ve not been able to find a video showing a saucer hit.)

There’s also this:

  • Allow multiple racks of invaders. The program crashes now if you kill all of them. (I didn’t know that, because I’ve never accomplished it, but I knew it didn’t do anything good.)

As usual, I’ll decide about what interests me.

Now let’s talk about this Constants thing.

I’ve chosen to make all the constant access use a singleton, and a method call. That makes the syntax this:

Constants:instance():yourConstantHere()

If we used a global and defined member values instead of methods, we could say:

Constants.yourConstantHere

That would be more compact and faster by one table hash lookup and maybe a bit more. It would be difficult to measure the time, but the syntax advantage might be nice.

We could compromise and put the constants in a global, allowing something like this:

Constants:yourConstantHere()

Even that is nicer. And we know I love my singletons too much. Kill your darlings, they say. Maybe we’ll do that.

Be that as it may, I think we’ll be happier when we get all the magic values at least centralized under Constants. So I’m counting that as a win.

See you next time!

OOPS! I forgot to fix the Y value in the tests for saucer position. Fixed in the zip file. I’ll save you reading the was 185 is 200 lines.

Invaders.zip