We made a number of useful changes last time. Let’s harvest some value.

My friend and colleague GeePaw Hill has coined the notion of “harvesting the value of change”, which he feels is at the core of good software development (and probably life). Certainly what I hope to do this afternoon is to begin to harvest the value of the changes I’ve been making to the collection logic.

I think first I’d like to look at the universe’s draw method:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    if self.timeOfNextWave > 0 and self.currentTime >= self.timeOfNextWave then
        self:newWave()
    end
    if not self.attractMode and not self.saucer and self.currentTime - self.saucerTime > self.saucerInterval then
        self.saucerTime = currentTime
        Saucer()
    end
    self.frame64 = (self.frame64+1)%64
    self:checkBeat()
    --displayMode(FULLSCREEN_NO_BUTTONS)
    pushStyle()
    background(40, 40, 50)
    self.processorRatio = DeltaTime/0.0083333
    self:drawAsteroids()
    self:drawExplosions()
    checkButtons()
    drawButtons()
    if self.ship then self.ship:draw() end
    if self.ship then self.ship:move() end
    if self.saucer then self.saucer:draw() end
    if self.saucer then self.saucer:move() end
    self:drawMissiles()
    drawSplats()
    self:drawScore()
    popStyle()
    self:findCollisions()
    self:checkNewWave()
end

This has become downright messy, bit by bit, as methods like this tend to do. We are in reasonable condition to start cleaning it up.

With Codea, we only get one repeated event, draw. To operate the game, everything that happens has to happen inside that event, other than whatever happens in touch. The “right thing” for a game is to draw everything, move everything, check for collisions, and check for needing a new wave, in that order. Everything should move in the same time interval. If we move some things and not others, we might, in principle at least, see something on the screen that was not detected in the game.

Presently, we draw and move things i chunks. The function drawAsteroids does both drawing and moving:

function Universe:drawAsteroids()
    pushStyle()
    stroke(255)
    fill(0,0,0, 0)
    strokeWidth(2)
    rectMode(CENTER)
    for i,asteroid in pairs(self.objects) do
        if asteroid:is_a(Asteroid) then
            asteroid:draw()
            asteroid:move()
        end
    end
    popStyle()
end

The same is true for drawMissiles, although it is done a bit differently:

function Universe:drawMissiles()
    for k, missile in pairs(self.objects) do
        if missile:is_a(Missile) then missile:draw() end
    end
    for k, missile in pairs(self.objects) do
        if missile:is_a(Missile) then missile:move() end
    end
end

The drawExplosions function is also separate, but it doesn’t move explosions because they just say “BLAMMO” right now and have no motion. That will change one of these days of course.

Now all of these objects are in our objects collection, and we can process them all together. For asteroids, we need to move all the pushing and popping inside asteroid:draw, like this, from:

function Asteroid:draw()
    pushMatrix()
    pushStyle()
    translate(self.pos.x, self.pos.y)
    scale(self.scale)
    strokeWidth(1/self.scale)
    for i,l in ipairs(self.shape) do
        line(l.x, l.y, l.z, l.w)
    end
    popStyle()
    popMatrix()
end

To this:

function Asteroid:draw()
    pushMatrix()
    pushStyle()
    stroke(255)
    fill(0,0,0, 0)
    strokeWidth(2)
    rectMode(CENTER)
    translate(self.pos.x, self.pos.y)
    scale(self.scale)
    strokeWidth(1/self.scale)
    for i,l in ipairs(self.shape) do
        line(l.x, l.y, l.z, l.w)
    end
    popStyle()
    popMatrix()
end

Now we can remove that stuff from the drawAsteroids loop:

function Universe:drawAsteroids()
    for i,asteroid in pairs(self.objects) do
        if asteroid:is_a(Asteroid) then
            asteroid:draw()
            asteroid:move()
        end
    end
end

All should be well, and I’ll test to find out.

I did notice one little thing: When the new wave starts at the beginning of the game, the attract mode asteroids are not deleted. That’ll be left over from this morning: a defect has escaped the barn.

We (well, I) removed the init of asteroids in the new wave code, and didn’t replace it with anything. I believe we can initialize the entire objects collection. I think it should go here:

function Universe:startGame(currentTime)
    self.currentTime = currentTime
    self.saucerTime = currentTime
    self.attractMode = false
    self.objects = {} -- <---
    createButtons()
    Ship()
    self.waveSize = nil
    self.lastBeatTime = self.currentTime
    self:newWave()
end

That works but found another defect when the saucer finally hit the ship. (It’s cleaning out far too many asteroids, by the way. We’ll have to look into that.) The error is this:

Universe:196: attempt to index a nil value (local 'ship')
stack traceback:
	Universe:196: in method 'checkMissileHitShip'
	Universe:189: in method 'findCollisions'
	Universe:78: in method 'draw'
	Main:12: in function 'draw'

And the code is:

function Universe:checkMissileHitShip(missile, ship)
    if not missile:is_a(Missile) then return end
    if  missile.pos:dist(ship.pos) < ship:killDist() then -- 196
        self:deleteShip(ship)
        missile:die()
    end
end

Hm! Where’s the call to this?

function Universe:findCollisions()
    -- we clone the asteroids collection to allow live editing
    for i,a in pairs(clone(self.objects)) do
        self:checkMissileCollisions(a)
        if self.ship then self:checkShipCollision(a) end
    end
    if self.ship then
        for i,m in pairs(clone(self.objects)) do
            self:checkMissileHitShip(m, self.ship)
        end
    end
end

How could ship be nil? We check before entering the loop. I conclude that we must have killed the ship and then detected the collision again. “Conclude” is too strong. I’m guessing. I think I want more information.

Ah. Suppose the ship fires two missiles, and one hits the ship. Then we check the second missile. There is no ship, but we are inside the check, so we check and the checkMissileHitShip function explodes.

We can protect it for now:

function Universe:checkMissileHitShip(missile, ship)
    if not missile:is_a(Missile) then return end
    if not ship then return end
    if  missile.pos:dist(ship.pos) < ship:killDist() then
        self:deleteShip(ship)
        missile:die()
    end
end

It’s irritating having to check things like this, and I believe when we get all the checking unwound, we’ll be nearly OK. I say “nearly” because while I hope we can just check everything against everything, there are still issues like this to be had when we’re iterating over the collection twice, once inside the other.

We’ll burn that bridge when we come to it. For now, we’re into cleaning up draw. Everything seems to be working, so commit: “move asteroid drawing details to asteroid. fix problem with two missiles against ship”.

Now back to draw:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    if self.timeOfNextWave > 0 and self.currentTime >= self.timeOfNextWave then
        self:newWave()
    end
    if not self.attractMode and not self.saucer and self.currentTime - self.saucerTime > self.saucerInterval then
        self.saucerTime = currentTime
        Saucer()
    end
    self.frame64 = (self.frame64+1)%64
    self:checkBeat()
    --displayMode(FULLSCREEN_NO_BUTTONS)
    pushStyle()
    background(40, 40, 50)
    self.processorRatio = DeltaTime/0.0083333
    self:drawAsteroids()
    self:drawExplosions()
    checkButtons()
    drawButtons()
    if self.ship then self.ship:draw() end
    if self.ship then self.ship:move() end
    if self.saucer then self.saucer:draw() end
    if self.saucer then self.saucer:move() end
    self:drawMissiles()
    drawSplats()
    self:drawScore()
    popStyle()
    self:findCollisions()
    self:checkNewWave()
end

Reorder things a bit:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    if self.timeOfNextWave > 0 and self.currentTime >= self.timeOfNextWave then
        self:newWave()
    end
    if not self.attractMode and not self.saucer and self.currentTime - self.saucerTime > self.saucerInterval then
        self.saucerTime = currentTime
        Saucer()
    end
    self.frame64 = (self.frame64+1)%64
    self:checkBeat()
    --displayMode(FULLSCREEN_NO_BUTTONS)
    pushStyle()
    background(40, 40, 50)
    self.processorRatio = DeltaTime/0.0083333
    checkButtons()
    drawButtons()
    self:drawAsteroids()
    self:drawExplosions()
    if self.ship then self.ship:draw() end
    if self.saucer then self.saucer:draw() end
    self:drawMissiles()
    drawSplats()
    self:drawScore()
    popStyle()
    if self.ship then self.ship:move() end
    if self.saucer then self.saucer:move() end
    self:findCollisions()
    self:checkNewWave()
end

Now all the drawing is done in one clump. That says to me that if we do this:

  • remove all the explicit drawing
  • draw everything
  • move everything
  • remove the separate ship and saucer move code

… everything should “just work”. I’m going to test and commit this simple reordering first, because there’s every chance we’ll need a quick revert here.

We seem to be good to go. Commit: “group drawing together in draw”.

Now replace all that drawing with a call to a new method: drawEverything:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    if self.timeOfNextWave > 0 and self.currentTime >= self.timeOfNextWave then
        self:newWave()
    end
    if not self.attractMode and not self.saucer and self.currentTime - self.saucerTime > self.saucerInterval then
        self.saucerTime = currentTime
        Saucer()
    end
    self.frame64 = (self.frame64+1)%64
    self:checkBeat()
    --displayMode(FULLSCREEN_NO_BUTTONS)
    pushStyle()
    background(40, 40, 50)
    self.processorRatio = DeltaTime/0.0083333
    checkButtons()
    drawButtons()
    self:drawEverything()
    drawSplats()
    self:drawScore()
    popStyle()
    if self.ship then self.ship:move() end
    if self.saucer then self.saucer:move() end
    self:findCollisions()
    self:checkNewWave()
end


function Universe:drawEverything()
    for k,o in pairs(self.objects) do
        o:draw()
    end
end

This is probably going to be interesting. In particular, things won’t move. Right. Nothing moves except the saucer and ship. I left those two calls in. Remove them and add a move:

...
    self:drawEverything()
    self:moveEverything()
    drawSplats()
    self:drawScore()
....

This works until an asteroid hits the ship. Then we get this:

Universe:85: attempt to call a nil value (method 'move')
stack traceback:
	Universe:85: in method 'moveEverything'
	Universe:69: in method 'draw'
	Main:12: in function 'draw'

Surely that has happened because the ship got deleted? I wonder what was trying to move, the ship or something else. And why? We should be free of collision damage here.

Well the code is this:

function Universe:moveEverything()
    for k,o in pairs(self.objects) do
        o:move()
    end
end

And BLAMMO is on the screen! I’ll bet the Explosion has no move method! Sure enough, it doesn’t. We’ll add it. Everyone has to respond to the key methods, even if they do nothing:

function Explosion:move()
end

Sure enough, everything works. Commit: “common draw and move loops for all but Splats and score”.

Nice. Universe:draw is shorter now and things are done in the right order. Let’s see what else we might do to clean it up:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    if self.timeOfNextWave > 0 and self.currentTime >= self.timeOfNextWave then
        self:newWave()
    end
    if not self.attractMode and not self.saucer and self.currentTime - self.saucerTime > self.saucerInterval then
        self.saucerTime = currentTime
        Saucer()
    end
    self.frame64 = (self.frame64+1)%64
    self:checkBeat()
    --displayMode(FULLSCREEN_NO_BUTTONS)
    pushStyle()
    background(40, 40, 50)
    self.processorRatio = DeltaTime/0.0083333
    checkButtons()
    drawButtons()
    self:drawEverything()
    self:moveEverything()
    drawSplats()
    self:drawScore()
    popStyle()
    self:findCollisions()
    self:checkNewWave()
end

We could and should pull out all that housekeeping about new waves, attract mode, and the like. Some of it will be straightforward, and we’ll start with that because we’re not fools:

Huh. The new wave stuff can be moved into checkNewWave:

function Universe:checkNewWave()
    local count = self:asteroidCount()
    if count == 0 then
        if self.timeOfNextWave == 0 then
            self.timeOfNextWave = self.currentTime + self.timeBetweenWaves
        end
    end
    if self.timeOfNextWave > 0 and self.currentTime >= self.timeOfNextWave then
        self:newWave()
    end
end

That works just the same as always. I suspect we need to wait a bit longer and make the saucer go away, because sometimes it kills asteroids as soon as the new wave starts. Anyway it’s not a new problem, I believe.

Now let’s move the saucer stuff out into checkSaucer:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    self.frame64 = (self.frame64+1)%64
    self:checkBeat()
    self:checkSaucer()
    --displayMode(FULLSCREEN_NO_BUTTONS)
    pushStyle()
    background(40, 40, 50)
    self.processorRatio = DeltaTime/0.0083333
    checkButtons()
    drawButtons()
    self:drawEverything()
    self:moveEverything()
    drawSplats()
    self:drawScore()
    popStyle()
    self:findCollisions()
    self:checkNewWave()
end

function Universe:checkSaucer()
    if self.attractMode or self.saucer then return end
    if self.currentTime - self.saucerTime > self.saucerInterval then
        self.saucerTime = currentTime
        Saucer()
    end
end

Things work. Should have committed once already. Commit: “centralize checkWave and create checkSaucer”.

What else? We have no reason to push and pop style. Remove those. Well, almost, a check shows they’re needed in drawScore instead:

function Universe:drawScore()
    local s= "000000"..tostring(self.score)
    s = string.sub(s,-5)
    pushStyle()
    fontSize(100)
    text(s, 200, HEIGHT-60)
    popStyle()
end

I decide to reorder draw a bit:

function Universe:draw(currentTime)
    self.currentTime = currentTime
    self.processorRatio = DeltaTime/0.0083333
    self.frame64 = (self.frame64+1)%64
    --displayMode(FULLSCREEN_NO_BUTTONS)
    background(40, 40, 50)
    checkButtons()
    drawButtons()
    self:drawEverything()
    self:moveEverything()
    drawSplats()
    self:drawScore()
    self:findCollisions()
    self:checkBeat()
    self:checkSaucer()
    self:checkNewWave()
end

We can improve this a bit more:

function Universe:draw(currentTime)
    self:adjustTimeValues(currentTime)
    --displayMode(FULLSCREEN_NO_BUTTONS)
    background(40, 40, 50)
    checkButtons()
    drawButtons()
    self:drawEverything()
    self:moveEverything()
    drawSplats()
    self:drawScore()
    self:findCollisions()
    self:checkBeat()
    self:checkSaucer()
    self:checkNewWave()
end

function Universe:adjustTimeValues(currentTime)
    self.currentTime = currentTime
    self.processorRatio = DeltaTime/0.0083333
    self.frame64 = (self.frame64+1)%64
end

I think this’ll do it for now. We’ve managed to clean up draw substantially, by a combination of some simple reordering, and by using our new all-objects collection.

There remains more to be done, for sure, but by my lights, it’s better than it was. Commit: “light draw refactoring”.

Let’s sum up.

Summing Up

I feel like I’ve redeemed myself from this morning, though truth to tell, this morning wasn’t bad. We got some very serious refactoring done, broke little or nothing, and set ourselves up to harvest more value this afternoon.

This afternoon, we saw the draw method drop from about 30 lines to about 15, and a consolidation of a lot of other code. We removed at least three specialized draw functions, for another 15 or 20 lines.

We consolidated a number of addX/deleteX functions, incuding “Explosion”, which I think I forgot to mention. So we deleted a number of lines there.

Things are a lot more systematic, the system is overall smaller and simpler, and at almost no time did our fingers leave our hands.

We did encounter a few defects, including a couple that escaped this morning. I think I’ve seen most everything happen that has to happen, and everything is working, but I have some concerns about our adding things to the objects collection.

All that’s for another day. See you next time!

Zipped Code