Space Invaders 48
Better scaling, then more with the saucer, I guess. Plus whatever opportunities we spot.
I managed to get Codea to run on my iPhone and started up the game. The scaling worked, with tiny little alien invaderettes. But the screen positioning was off. Here’s why:
function GameRunner:draw()
pushMatrix()
pushStyle()
noSmooth()
rectMode(CORNER)
spriteMode(CORNER)
stroke(255)
fill(255)
scale(math.min(HEIGHT/256, WIDTH/224))
translate(WIDTH/8-112,0)
TheArmy:draw()
self.player:draw()
drawShields()
drawStatus()
popStyle()
popMatrix()
end
The translate is assuming that the scale is 4, and it isn’t. The fix is this:
function GameRunner:draw()
pushMatrix()
pushStyle()
noSmooth()
rectMode(CORNER)
spriteMode(CORNER)
stroke(255)
fill(255)
local sc = math.min(HEIGHT/256, WIDTH/224)
scale(sc)
translate(WIDTH/(2*sc)-112,0)
TheArmy:draw()
self.player:draw()
drawShields()
drawStatus()
popStyle()
popMatrix()
end
Commit: fix translate in screen scaling.
One might legitimately look at that code and wonder “why”. Remember that Kent Beck’s four rules of simple code are:
- Runs all the tests;
- Expresses all the programmer’s ideas;
- Contains no duplication; 4 Minimizes entities.
This certainly doesn’t express the scaling idea very well, and since it has taken me three tries to get it right, one could argue that my ideas on the subject aren’t very clear. Shall we try to make it more clear? Sure, why not? This does have to be done dynamically, lest the player rotate his screen during play, but this shouldn’t cost much, so let’s try. First, extract the relevant code to a function:
function GameRunner:draw()
pushMatrix()
pushStyle()
noSmooth()
rectMode(CORNER)
spriteMode(CORNER)
stroke(255)
fill(255)
self:setUpScale()
TheArmy:draw()
self.player:draw()
drawShields()
drawStatus()
popStyle()
popMatrix()
end
function GameRunner:setUpScale()
local sc = math.min(HEIGHT/256, WIDTH/224)
scale(sc)
translate(WIDTH/(2*sc)-112,0)
end
This has no visible effect. Arguably it’s a tiny bit more clear but it’s not great. And you know what? That CORNER stuff belongs in there, it’s part of why we do what we do. And the name should be setUpScaleAndOrigin.
function GameRunner:draw()
pushMatrix()
pushStyle()
noSmooth()
stroke(255)
fill(255)
self:setUpScaleAndOrigin()
TheArmy:draw()
self.player:draw()
drawShields()
drawStatus()
popStyle()
popMatrix()
end
function GameRunner:setUpScaleAndOrigin()
rectMode(CORNER)
spriteMode(CORNER)
local sc = math.min(HEIGHT/256, WIDTH/224)
scale(sc)
translate(WIDTH/(2*sc)-112,0)
end
Still no change in behavior. Now let’s expand this out and see if we can use Explaining Variable Name to advantage.
OK, this got weird. There’s such a thing as too much explaining:
function GameRunner:setUpScaleAndOrigin()
rectMode(CORNER)
spriteMode(CORNER)
local gameMaxY = 256
local gameMaxX = 224
local maxVerticalScale = HEIGHT/gameMaxY
local maxHorizontalScale = WIDTH/gameMaxX
local maxAcceptableScale = math.min(maxVerticalScale, maxHorizontalScale)
scale(maxAcceptableScale)
local centerX, centerY = WIDTH/2/maxAcceptableScale, HEIGHT/2/maxAcceptableScale
translate(centerX, centerY)
local cornerX, cornerY = -gameMaxX/2, -gameMaxY/2
translate(cornerX, cornerY)
end
But there are three things going on here. Let’s express those. We’ll go in three steps:
function GameRunner:setUpScaleAndOrigin()
rectMode(CORNER)
spriteMode(CORNER)
self:scaleToMaxAvailableSpace()
local centerX, centerY = WIDTH/2/maxAcceptableScale, HEIGHT/2/maxAcceptableScale
translate(centerX, centerY)
local cornerX, cornerY = -gameMaxX/2, -gameMaxY/2
translate(cornerX, cornerY)
end
function GameRunner:scaleToMaxAvailableSpace()
local gameMaxY = 256
local gameMaxX = 224
local verticalScale = HEIGHT/gameMaxY
local horizontalScale = WIDTH/gameMaxX
local maxAcceptableScale = math.min(verticalScale, horizontalScale)
scale(maxAcceptableScale)
end
This can’t work as written, because we no longer have maxAcceptableScale available.
At this moment I can see at least three things to do:
- Observe that this isn’t really making things better, and back it out;
- Pull out a
bestScalefunction and use it twice (duplication); - Change the currently pulled-out function to do it all, and refactor internally.
Case 3 looks a lot like hey, wait, this function is the same as the other one was.
Case 2 leaves us with three levels of functions, at least, just to set the scaling.
Case 1 is the answer. Back this clear out. Then look again to see if we can make it a bit more clear.
Revert.
function GameRunner:draw()
pushMatrix()
pushStyle()
noSmooth()
rectMode(CORNER)
spriteMode(CORNER)
stroke(255)
fill(255)
local sc = math.min(HEIGHT/256, WIDTH/224)
scale(sc)
translate(WIDTH/(2*sc)-112,0)
TheArmy:draw()
self.player:draw()
drawShields()
drawStatus()
popStyle()
popMatrix()
end
I think I will do this much:
function GameRunner:draw()
pushMatrix()
pushStyle()
noSmooth()
stroke(255)
fill(255)
self:scaleAndTranslate()
TheArmy:draw()
self.player:draw()
drawShields()
drawStatus()
popStyle()
popMatrix()
end
function GameRunner:scaleAndTranslate()
local gameScaleX = 256
local gameScaleY = 224
rectMode(CORNER)
spriteMode(CORNER)
local sc = math.min(HEIGHT/gameScaleX, WIDTH/gameScaleY)
scale(sc)
translate(WIDTH/(2*sc)-gameScaleX/2,0)
end
That isolates the weird code better, makes it just slightly more communicative, and salves my conscience. Commit: refactor GameRunner draw.
The big question here is: was that worth it? And, if not, doesn’t this prove we shouldn’t mess with existing code?
The benefits of this little exercise include:
- We exercised our skill in naming and in extract refactoring and in reordering for clarity. We’re just a bit stronger.
- We experienced going too far, felt it, and backed away.
- We reduced our fear of wasting time and reverting code just a bit.
- We left the code at least a bit more clear than it was.
The costs? A half hour or less of time, especially if we weren’t writing an article at the same time.
My own take is that we should almost always be willing to burn a half hour to make the code space better. We are going to be changing code as long as we are programmers. Gaining comfort with doing it is always worth it. And if we always leave it at least a bit better, we put off the inevitable heat death of the universe just a bit.
YMMV, of course. I’m here to press against the usual limits. You’ve probably got a job to do. That said, if I had a job to do, this is how I’d do it, because I’ve never written too many tests or kept the code space looking too pretty. I could … but I never have. Well, hardly ever. And besides, the product is dead.
But what about that saucer?
Saucer
I’ve been reading the assembly code and comments for Space Invaders, and haven’t really been able to grok just exactly what it’s doing about how often the saucer runs and such. So there’s more study to do, and maybe in the end we’ll just make a determination of our own. For now, I think we’ll see about shooting it down and scarfing up its score.
The Missile checks for kills by asking Army:
function Missile:draw()
if self.v == 0 then return end
pushStyle()
if self.explodeCount > 0 then
tint(self:explosionColor())
sprite(self.explosion, self.pos.x - 4, self.pos.y)
self.explodeCount = self.explodeCount - 1
if self.explodeCount == 0 then
self.v = 0
end
else
rect(self.pos.x, self.pos.y, 2,4)
end
popStyle()
if self.v > 0 then TheArmy:processMissile(self) end
end
Army does this:
function Army:processMissile(aMissile)
if aMissile.v > 0 and aMissile.pos.y <= self:saucerTop() then
self:checkForKill(aMissile)
end
end
function Army:checkForKill(missile)
for i, invader in ipairs(self.invaders) do
if invader:killedBy(missile) then
missile.v = 0
return
end
end
if self.rollingBomb:killedBy(missile) then
missile.v = 0
self.rollingBomb:explode(self)
elseif self.plungerBomb:killedBy(missile) then
missile.v = 0
self.plungerBomb:explode(self)
elseif self.squiggleBomb:killedBy(missile) then
missile.v = 0
self.squiggleBomb:explode(self)
end
end
I have my doubts about that saucerTop() but we’ll just have to be sure it’s high enough. Seems we can just add another check to the if nest there in checkForKill:
function Army:checkForKill(missile)
for i, invader in ipairs(self.invaders) do
if invader:killedBy(missile) then
missile.v = 0
return
end
end
if self.rollingBomb:killedBy(missile) then
missile.v = 0
self.rollingBomb:explode(self)
elseif self.plungerBomb:killedBy(missile) then
missile.v = 0
self.plungerBomb:explode(self)
elseif self.squiggleBomb:killedBy(missile) then
missile.v = 0
self.squiggleBomb:explode(self)
elseif self.saucer:killedBy(missile) then
missile.v = 0
self.saucer:explode(self)
end
end
I’m not entirely happy with slamming 0 into missile.v but we’re not here for that now. So the saucer needs to know killedBy and explode, though I wonder why they don’t all just explode if the check says they’re killed. Again, we’re not here for that, but when we refactor let’s think about it.
The Bomb:killedBy looks like this:
function Bomb:killedBy(missile)
return rectanglesIntersect(self.pos,3,4, missile.pos,3,4)
end
We can surely emulate that, but on second thought, copying how invaders do it is better:
function Invader:killedBy(missile)
if not self.alive then return false end
if self:isHit(missile) then
self.alive = false
addToScore(self.score)
self.exploding =15
SoundPlayer:play("killed")
return true
else
return false
end
end
function Invader:isHit(missile)
if not self.alive then return false end
return rectanglesIntersect(missile.pos,2,4, self.pos+vec2(2,0),12,8)
end
Yeah, this is more robust. it deals with adjusting for the fact that the invader’s rectangle is 16x8 but the invader herself is only 12x8. The saucer is 24x8 with 4 spaces on each side. We’ll copy this and hammer it:
function Saucer:killedBy(missile)
if not self.alive then return false end
if self:isHit(missile) then
self.alive = false
addToScore(self:score())
self.exploding =15
SoundPlayer:play("killed")
return true
else
return false
end
end
function Saucer:isHit(missile)
if not self.alive then return false end
return rectanglesIntersect(missile.pos,2,4, self.pos+vec2(4,0),16,8)
end
function Saucer:score()
return 66
end
I really should have written a test for this, especially since it’s going to be just about impossible for me to successfully shoot down one of these guys unless I really turn on some serious cheat codes.
I’ll write some tests, I promise. But first, I’m going to turn on some cheat codes which I plan to invent right now.
function setup()
runTests()
parameter.boolean("Cheat", false)
That’ll give me a switch labelled Cheat and a global Cheat that will be true if it’s set.
Now, let’s see. I would like for nothing to kill the player:
function Player:explode()
if Cheat then return end
self.alive = false
self.count = 240
SoundPlayer:play("explosion")
end
Now let’s play a bit and see if we can shoot down enough aliens to hit the saucer.
SaucerTop is too low. I bump that up by 32 pixels and, after too long an attempt, even cheating, I get my expected failure, attempt to explode, which isn’t implemented yet.
I promised to test, didn’t I? But now I know I don’t need them, because it works. Well, I could go back and erase the promise, but instead let’s actually try some edge cases.
_:test("Missile vs Saucer", function()
local saucer = Saucer()
saucer.pos = vec2(100,100)
local missile = Missile()
missile.pos = vec2(100,100)
_:expect(saucer:killedBy(missile)).is(false)
end)
This is monstrously invasive, and I’ve discovered some feature envy in checking how to do this. We’re not here for that, we’re here to test the saucer-missile relationship.
I’m just going to add some more assertions here and check that it works. Basically what we’ve implemented is that positions 4-19 (zero-based) from the left should count as hits, top to bottom 0-7. Let’s see how that works:
I suffer a bit of confusion because I’ve not set the missile and saucer to alive, which means no collisions ever.
_:test("Missile vs Saucer", function()
local saucer = Saucer()
saucer.pos = vec2(100,100)
local missile = Missile()
missile.alive = true
saucer.alive = true
missile.pos = vec2(100,100)
_:expect(saucer:killedBy(missile)).is(false)
missile.pos = vec2(103,100)
_:expect(saucer:killedBy(missile)).is(false)
missile.pos = vec2(100,107)
_:expect(saucer:killedBy(missile)).is(false)
missile.pos = vec2(103,107)
_:expect(saucer:killedBy(missile)).is(false)
missile.pos = vec2(100,100)
_:expect(saucer:killedBy(missile)).is(false)
missile.pos = vec2(112,104)
_:expect(saucer:killedBy(missile)).is(true)
end)
This errors trying to compute score. For now, hammer the test to init a Score global.
Now I’m surprised because this one comes out true:
missile.pos = vec2(103,100)
_:expect(saucer:killedBy(missile)).is(false)
Why? Because we treat the missile as having width 2, not width 1. I can change the test or change the code. I think we’ll make it hard to hit the saucer:
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
All the missile vs saucer tests run now. I’ll add a few more checks because why not.
Well, one reason why not is that the saucer killedBy logic is killing the saucer so I’d have to init it back to alive every time. Instead, we should be calling isHit, that’s all we’re testing.
This is tedious and boring. I’m stopping with this:
_:test("Missile vs Saucer", function()
local saucer = Saucer()
saucer.pos = vec2(100,100)
local missile = Missile()
missile.alive = true
saucer.alive = true
missile.pos = vec2(100,100)
_:expect(saucer:isHit(missile)).is(false)
missile.pos = vec2(103,100)
_:expect(saucer:isHit(missile)).is(false)
missile.pos = vec2(100,107)
_:expect(saucer:isHit(missile)).is(false)
missile.pos = vec2(103,107)
_:expect(saucer:isHit(missile)).is(false)
missile.pos = vec2(100,100)
_:expect(saucer:isHit(missile)).is(false)
missile.pos = vec2(104,100)
_:expect(saucer:isHit(missile)).is(true)
missile.pos = vec2(119,104)
_:expect(saucer:isHit(missile)).is(true)
missile.pos = vec2(120,104)
_:expect(saucer:isHit(missile)).is(false)
end)
I could imagine an array of values that should hit and another of values that should not, and a loop. I think this is good enough.
We do need at least a minimal Saucer:explode(). No, actually we don’t. The saucer manages its own exploding, and presently doesn’t do much. Let’s do make sure that’ll work:
function Saucer:killedBy(missile)
if not self.alive then return false end
if self:isHit(missile) then
self.alive = false
addToScore(self:score())
self.exploding =15
SoundPlayer:play("killed")
return true
else
return false
end
end
That’ll do for now. The saucer should vanish now if we hit it. I’ve removed the call to explode from the player. A quick run to be sure it does vanish.
It does and it even makes the invader-killed sound. However, I’ve noticed that saucerTop goes down as the invaders go down. The idea I had was that the saucer came lower and lower, but it does not. So:
function Army:saucerTop()
return self.invaders[#self.invaders].pos.y + 24 + 32
end
That can be a nice constant. Since the saucer flies at y = 185, let’s make this 20 above that, or 205.
That works as anticipated. Commit: missile kills saucer, no explosion yet.
Those changes touched 6 tabs. That’s rather a lot. Anyway, I’m tired and hungry, so let’s sum up.
Summing Up
Again, things went fairly smoothly. Overall, a successful session.
The refactoring of the scaling and translation was interesting (to me, anyway). There is some fundamental idea there that may still not be brought out. Basically the game is coded to run on a screen that is 224 wide by 256 high, because that makes the various bitmaps work at their original scale, and keeps any numbers that we import from the original game the same.
So scaling’s job is to fit a 224x256 rectangle maximally into whatever screen size the machine has, and to center it on X, and set it to the bottom on Y. You could imagine a diagram showing that. The code doesn’t really say that, even now.
On the other hand, the whole thing can be done in three lines. They are somewhat obscure lines, with some magic values in them. But it’s only three.
Is it worth expanding those few lines by a factor of three or more to “make it more clear”? In this case, perhaps not I do think it’s well worth knowing how to approach making three obscure lines clear, and to be willing to try to do it, because if we made each three lines of our program clear, we’d be happier campers overall. And in today’s machines we needn’t be so concerned about saving memory. We can’t be blindly stupid about it, but we can be fairly casual.
So I found the exercise worthwhile, working my brain to think of ways to make the code express what I knew … and clarifying what I knew in the process.
The copy-paste approach to saucer-missile relations worked and the changes were easy enough. We’re glad we have that rectangle intersection function. But it’s odd, isn’t it, a free-hanging function like that? We might want to think about where it would be better off.
We now have substantial duplication in the collision logic. Multiple calls to explode, feature envy setting missile v to zero to kill it, and probably more.
Next time, unless I change my mind, we’ll see about the sound for the saucer, its explosion, and improving the general collision code. I keep thinking there should be a “Collider” object to do the job for everyone. I could be quite wrong, but I bet I try it to find out.
Tun in next time! Thanks for reading, all three of you!