Space Invaders 19
I had no plan. Now I have a plan. Check code clarity, then work on shields. Things go … oddly
I really haven’t the slightest plan for today. I’ll surely start with a look at the code to see what needs cleaning up. Maybe this is a good time to set up something a bit more sensible for the gunner’s missiles. I noticed in watching recorded gameplay of the original that it appears that the player can only have one missile in flight at a time, so that’s interesting.
We could bring in the graphical elements and animation. That will be mostly tedious, I think, but we’ll surely have some fine opportunities for refactoring. Maybe we’ll discover a general animation object or something.
Shields will be interesting, especially since the invader bombs nibble away at them. Doing those might also be a start at importing the graphical elements.
First, a quick code review, see what that may call for, then shields, time permitting. We have a plan.
Code Review
Well, of course out of the box, we have the little bit of witchery-hackery that I did to make the gunner blob turn red when it’s hit:
function explode()
Gunner.alive = false
Gunner.count = 120
end
function setupGunner()
Gunner = {pos=vec2(112,10),alive=true,count=0,explode=explode}
GunMove = vec2(0,0)
end
function drawGunner()
pushMatrix()
pushStyle()
if Gunner.alive then
stroke(255)
fill(255)
else
stroke(255,0,0)
fill(255,0,0)
Gunner.count = Gunner.count - 1
if Gunner.count <= 0 then Gunner.alive = true end
end
rectMode(CENTER)
rect(Gunner.pos.x, Gunner.pos.y, 16,8)
stroke(255,0,0)
fill(255,0,0)
point(Gunner.pos.x, Gunner.pos.y)
point(Gunner.pos.x-8, Gunner.pos.y)
point(Gunner.pos.x+8, Gunner.pos.y)
point(Gunner.pos.x, Gunner.pos.y+4)
point(Gunner.pos.x, Gunner.pos.y-4)
popStyle()
popMatrix()
Gunner.pos = Gunner.pos + GunMove
end
I think we’ll let that ride, since we’ll be making changes when we add the gunner sprite and the real gunner explosion. Who knows, it might even be approximately the right structure.
Recall that we move only one invader per cycle. The code for fetching the next invader looks like this:
function Army:nextInvader()
if self.invaderNumber > #self.invaders then
if self.overTheEdge then
self.motion = self.motion*-1 - vec2(0,2)
self.overTheEdge = false
else
self.motion.y = 0
end
self.invaderNumber = 1
end
local inv = self.invaders[self.invaderNumber]
self.invaderNumber = self.invaderNumber + 1
return inv
end
That’s not very clear, is it? What we mostly need to do is go to the next invader, which is the three lines at the bottom. However, if we’ve just done the last invader (#invaders) we want to start over at 1. But if we’ve gone over the edge, we want to set the invader motion to be the reverse of what it is, plus two pixels down, to make them advance. Otherwise, when we’ve completed a cycle through all the invaders, we want to set the downward motion (self.motion.y) to zero, so they’ll only step down once per cycle.
Just to see if we can make this better, let’s refactor a bit.
We have no useful tests for this behavior, just a few that handle the basics. We’ll have to be careful, and we’ll look for opportunities to write new tests. I’m not optimistic about that last part.
Looking at that method, the stuff at the top could be considered to be loop handling, or end of cycle work. Let’s extract it all to a method:
function Army:nextInvader()
self:handleCycleEnd()
local inv = self.invaders[self.invaderNumber]
self.invaderNumber = self.invaderNumber + 1
return inv
end
function Army:handleCycleEnd()
if self.invaderNumber > #self.invaders then
if self.overTheEdge then
self.motion = self.motion*-1 - vec2(0,2)
self.overTheEdge = false
else
self.motion.y = 0
end
self.invaderNumber = 1
end
end
That’s easy and safe. And it works. Commit: extract Army:handleCycleEnd.
We’re not done, but we’re completely committed and our slightly improved code is now ready to ship. Let’s look at the handle method for more improvement.
First, I move the setting of the invaderNumber up to the top:
function Army:handleCycleEnd()
if self.invaderNumber > #self.invaders then
self.invaderNumber = 1
if self.overTheEdge then
self.motion = self.motion*-1 - vec2(0,2)
self.overTheEdge = false
else
self.motion.y = 0
end
end
end
That changes nothing. Could commit again but not yet. What does that innerif in there do?
if self.overTheEdge then
self.motion = self.motion*-1 - vec2(0,2)
self.overTheEdge = false
else
self.motion.y = 0
end
Well, like it says, if we’re over the edge move down, otherwise be sure we move only sideways. We could call it something … how about adjustMotion?
function Army:handleCycleEnd()
if self.invaderNumber > #self.invaders then
self.invaderNumber = 1
self:adjustMotion()
end
end
function Army:adjustMotion()
if self.overTheEdge then
self.motion = self.motion*-1 - vec2(0,2)
self.overTheEdge = false
else
self.motion.y = 0
end
end
Now it’s arguably more clear in the handler. Maybe a better name would make it more clear. The code in adjustMotion is no more clear, but it is now isolated.
Let’s go fully crazy here, just to show what’s possible and maybe learn when not to go wild:
First move the flag handling up:
function Army:adjustMotion()
if self.overTheEdge then
self.overTheEdge = false
self.motion = self.motion*-1 - vec2(0,2)
else
self.motion.y = 0
end
end
Then a couple of names:
local reverse = -1
local stepDown = vec2(0,-2)
function Army:adjustMotion()
if self.overTheEdge then
self.overTheEdge = false
self.motion = self.motion*reverse + stepDown
else
self.motion.y = 0
end
end
I think that makes the first motion adjustment more clear. It costs us a couple of variable slots, since Lua doesn’t have compile-time constants.
What about the second adjustment? We could define a function, I suppose. Or we could just add a comment as a signal that we have given up on making a single line more clear in a useful way.
Here’s where we are now:
function Army:nextInvader()
self:handleCycleEnd()
local inv = self.invaders[self.invaderNumber]
self.invaderNumber = self.invaderNumber + 1
return inv
end
function Army:handleCycleEnd()
if self.invaderNumber > #self.invaders then
self.invaderNumber = 1
self:adjustMotion()
end
end
local reverse = -1
local stepDown = vec2(0,-2)
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
Everything continues to work. I’ll commit this as “refactoring nextInvader for clarity” and then we need to have a little talk.
A Little Talk
Honestly, I think this new expression of nextInvader is far more clear than the old one. Many more of the lines express their intention. There are things I’d improve if I knew how, including
- We have the overTheEdge flag, which communicates a past event to the future, and when the future handles it, it clears the flag.
- That costs us a member variable, and a deferred change, and we have to remember to reset the flag. In a larger system, I might consider implementing messages from the past to the future. Might be fun to do it here.
- We have the resetting of
motion.y, which is pretty close to the metal. - That wasn’t worth wrapping as a function in my view. But it’s an implementation detail without a wrapper that says why it is there, and that should always be a red flag.
- We’re calling three functions to do what used to be done in one.
- My general practice is not to worry about function calls. They are generally pretty fast, so fast that you’d have trouble finding out how long they take. That said, this code runs for every invader move. On the other hand, that’s only once per 60th of a second. I prefer the added clarity, and YMMV.
There is also the question of whether this is in fact more clear. By my lights it is, because I’ve been trained by others, and by myself, to scan code for meaning. So if I scan the first method:
function Army:nextInvader()
self:handleCycleEnd()
local inv = self.invaders[self.invaderNumber]
self.invaderNumber = self.invaderNumber + 1
return inv
end
I’m like “yep, makes sense”, and I move on unless I’m interested in what goes on at the end of a cycle. In its prior form, I had to figure out more stuff and couldn’t just scan and move on.
However, working this way comes with some requirements. First, I think there’s something about abstraction. This relates to “programming by intention”, where we figure out a thing we want to do, and type its name and move on. There’s a kind of act of faith going on there. In writing, we promise we’ll build that thing. In reading, we trust that the function or method does what it says.
I suspect that programmers don’t move automatically from writing long procedures to writing these short tiny one-notion methods. And if they’ve not moved there, and they see code like this, they feel the need to traverse down the tree. Even if they aren’t really wondering right now how the end of cycle works, they feel obligated to pop down there, read it, pop back up.
When that’s the way you work, this style is painful. You’d rather have everything right there rather than skip around.
The person experienced with this style doesn’t skip around. She just drills down until she finds the bit she cares about, and then stops.
Of course I think my way is better, because the code says what it’s going to do, and then says how it’s going to do it, and I prefer that. But it is a preference, and perhaps not a law of nature.
OK, that was more of a big talk. What about shields? It’s only 0920 and my car isn’t back from service yet, so let’s see about shields.
Shields
There are four shields in the original game. They are 22x16, and each one can be damaged separately. So unlike an ordinary sprite, the bitmaps for the shields can change. So that’s interesting.
They appear to be separated by 23 columns, if I’m reading the code correctly. 4x22 + 3x23 is 157. Out of 224 pixels, that leaves 67, or 33 on one side and 34 on the other.
I think I’ll begin with a simple drawShield function in draw, to check out the spacing. I don’t see the vertical position of the shields in the original source, so for now I’ll just put them somewhere.
function drawShields()
local posX = 34 + 11 -- centered, 22 wide
local posY = 100
for i = 1,4 do
rect(posX,posY, 22,16)
posX = posX + 22 + 23
end
end
That gives us this picture, which isn’t half bad:
They’re too high, of course, but that’s easily corrected. I’m having difficulty decoding the assembler for the real game, so a lot of the numbers aren’t clear to me yet.
Interruption
Yesterday the dealer picked up my car to take it to service, and this morning they brought it back. It just arrived so I had a bit of a social distancing chat with the porter as he unloaded. This way of getting the car serviced is rather good. However, now that I’m interrupted, I think maybe I’ll pop out for a quick chai. I’ve been trapped at home for over 24 hours.
What I’ll be thinking about along the way is how much to write about Dave1707’s neat way of doing an explosion in a shield. Then a bit more work.
You can take a break now too, but I’ll be back in the next paragraph.
Shield Improvements
Back. Car works. Had to reset a menu item or two, they flashed one of the computers. Probably more changes to come as I notice things. But we’re here for the shields.
First thing, let’s convert the shields to bitmaps:
function setup()
runTests()
createShields() -- <---
TheArmy = Army()
setupGunner()
Missile = {v=0, p=vec2(0,0)}
invaderNumber = 1
end
OK, done. Well, not quite, I guess we have to make the bitmaps:
function createShields()
local img = image(22,16)
for row = 1,16 do
for col = 1,22 do
img:set(col,row,0,255,0)
end
end
Shields = {img}
for s = 2,4 do
table.insert(Shields,img:copy())
end
end
function drawShields()
local posX = 34+11 -- centered, 22 wide
local posY = 100
for i = 1,4 do
sprite(Shields[i], posX, posY)
posX = posX + 22+ 23
end
end
I want an array of 4 bitmaps, each 22 by 16, set all to – I decided on green, just because the ones in the game are green, because they’re behind green cellophane. I create one such bitmap, put it in the array Shields, then add three copies.
In draw, I just changed the rect call to sprite and it looks good:
Let’s commit: green bitmap shields.
Damage
It would be nice to do at least some elementary damage to the shields when the missiles hit them. That’s tricky right now, because where they are is rather a mystery as far as the code is concerned: I’ve not recorded that anywhere, I just draw them. Let’s defer that, until we see what the code for checking them would like to do.
In Bomb:update we have this:
function Bomb:update(army, increment)
self.pos.y = self.pos.y - (increment or 1)
if self:killsGunner() then
army:deleteBomb(self)
Gunner:explode()
end
if self.pos.y < 0 then
army:deleteBomb(self)
end
end
This looks like a good place to check for hitting a shield, something like this:
function Bomb:update(army, increment)
self.pos.y = self.pos.y - (increment or 1)
if self:killedShield() then
army:deleteBomb(self)
return
end
if self:killsGunner() then
army:deleteBomb(self)
Gunner:explode()
return
end
if self.pos.y < 0 then
army:deleteBomb(self)
return
end
end
You’ll note that I added in some explicit returns, as the cases really shouldn’t overlap. Using elseif might be preferable but I’m here to implement, not refactor. So I have posited a function killedShield that returns true if we hit the shield, and deletes the bomb. (I suspect we could do that inside the inner function, and I’l try to remember to think about that later. For now, we’ll match the existing pattern, which is unfortunately more “ask don’t tell” than the preferred “tell don’t ask”.
Anyway … my first cut is this:
function Bomb:killedShield()
for s = 1,4 do
local pos = ShieldPos[i]
local img = Shields[i]
return self:damageShield(img,pos)
end
end
Here, I imagine there’s a table ShieldPos with the positions of the shields, and that there’s a function damageShield that returns true if we have damaged a shield. That will stop our looking and also tell our caller to delete the bomb (which we could have done ourselves, probably.)
We trying to make it work. We’ll make it right after it works, unless it gets so messy that we give up.
But we’re doing that “by intention” thing, naming smaller and smaller operations until finally we’re done.
Now for damageShield. That needs to do a very similar rectangle intersection thing like we did in Invader:isHit:
function Invader:isHit(missile)
if not self.alive then return false end
local missileTop = missile.pos.y + 4
local invaderBottom = self.pos.y - 4
if missileTop < invaderBottom then return false end
local missileLeft = missile.pos.x-1
local invaderRight = self.pos.x + 4
if missileLeft > invaderRight then return false end
local missileRight = missile.pos.x + 1
local invaderLeft = self.pos.x - 4
return missileRight > invaderLeft
end
I’ll paste that (yucch) and then edit it into rough submission.
Well, no. Clearly too messy. Using the isHit as a guide, I’ll do something a bit more reasonable:
I came up with this:
function Bomb:damageShield(pos,img)
local hit = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(img,hit)
return true
end
The idea is that I’ve pushed off the intersection problem one more time, and that function either returns nil, if there is no intersection, or a point in the intersection. For now I don’t care what the point is. If we do get a point, we damage the bitmap around that point. Both those functions remain to be defined. But now …
Wow. I wrote this which can’t possibly be right:
function rectanglesIntersectAt(pos1,w1,h1, pos2,w2,h2)
local bottom1 = pos1 - h1//2
local top2 = pos2 + h2//2
if bottom1 > top2 then return nil end
-- we're low enough
local top1 = pos1 + h1//2
local bottom2 = pos2 - h2//2
if bottom2 > top1 then return nil end
-- we're on the right level
local r1 = pos1 + w1//2
local l2 = pos2 - w2//2
if r1 < l2 then return nil end
-- we're not too far left
local l1 = pos1 - w1//2
local r2 = pos2 + w2//2
if l1 > r2 then return nil end
return pos1
end
That really needed to be TDD’d but I was on a roll.
Let’s do explode.
OK, that has not gone well. Here’s what I finally did for explode:
function Bomb:applyDamage(img, hit, imagePos)
--[[
local offset = imagePos - hit
for x = -3,3 do
for y = -3,3 do
img:set(offset.x + x, offset.y+y, 255,0,0)
end
end
]]--
for x = 1,22 do
for y = 1,16 do
img:set(x,y,255,0,0)
end
end
end
My original code had no effect, but when a bomb struck the leftmost shield, I saw that the bomb disappeared, which told me the collision was being detected. Bombs drop right through the other three shields, so there’s something wrong. You can see in this picture that the leftmost shield is red, and there’s a bomb below one of the others, showing that it was flat missed.
What happened? Well, a lot. I wrote a lot of code speculatively, and it had a number of interesting bugs in it, such as using vectors where I meant a single coordinate in the rectangle checker:
function rectanglesIntersectAt(pos1,w1,h1, pos2,w2,h2)
local bottom1 = pos1.y - h1//2
local top2 = pos2.y + h2//2
if bottom1 > top2 then return nil end
-- we're low enough
local top1 = pos1.y + h1//2
local bottom2 = pos2.y - h2//2
if bottom2 > top1 then return nil end
-- we're on the right level
local r1 = pos1.x + w1//2
local l2 = pos2.x - w2//2
if r1 < l2 then return nil end
-- we're not too far left
local l1 = pos1.x - w1//2
local r2 = pos2.x + w2//2
if l1 > r2 then return nil end
return pos1
end
I think that might be right now, but of course I have only faith to go on, since I didn’t test it, much less TDD it.
My guess is that a little quick debugging will tell me what’s wrong with the other shields, and I can’t resist doing it. But this is wrong, wrong, wrong.
This print:
function Bomb:damageShield(img, shieldPos)
print("bomb,shield", self.pos, shieldPos)
local hit = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(img, hit, shieldPos)
return true
end
Shows the bomb position changing, and the shield position not changing. Better check where that’s created:
function createShields()
local img = image(22,16)
for row = 1,16 do
for col = 1,22 do
img:set(col,row,0,255,0)
end
end
Shields = {img}
for s = 2,4 do
table.insert(Shields,img:copy())
end
ShieldPos = {}
local posX = 34+11
local posY = 100
for i = 1,4 do
table.insert(ShieldPos, vec2(posX,posY))
posX = posX + 22 + 23
end
end
That sure seems to vary. I’ll print it to be sure.
for i,p in ipairs(ShieldPos) do
print(i,p)
end
They are correct as far as I can tell. Certainly different anyway. So why are they the same later on?
Duh:
function Bomb:killedShield()
for i = 1,4 do
local pos = ShieldPos[i]
local img = Shields[i]
return self:damageShield(img,pos)
end
end
This function returns the first result unconditionally. Let’s be about fixing that:
function Bomb:killedShield()
for i = 1,4 do
local pos = ShieldPos[i]
local img = Shields[i]
local hit = self:damageShield(img,pos)
if hit then return true end
end
return false
end
That makes it work. Here’s a pic of all four shields turned red by being hit by bombs. You need to take my word for it that the bombs don’t fly on through. Or not. Anyway, they don’t.
It’s 1156 and I’m going to commit: shields red when hit, and break for lunch,
But this is wrong, wrong, wrong.
Wrong, wrong, wrong
Yes, it works. It’s not even so far off from right as to make it impossible to move forward. It’s even somewhat well-factored, so that new implementations will probably be doable.
But I was way out on a limb and someone else was holding my beer. Now I’m a decent programmer on most days, and writing fifty or sixty lines of code and making it work is clearly within my capability, since I just did it. But it didn’t have to go that way and I could have been in either for a long and painful debugging session, or a throw-away-and-rewrite experience.
I got lucky. I want to say that I got skillful, and of course I did, but in my book it was too risky. What would have been better?
Well, the central issue was of course the rectangle intersecting function. That came out right (I think) because I had just done something like it and had it fresh in mind. That should have been done with TDD, I think. I’m kind of willing to accept it because I expect to go to a different form of check, but no. Something that complicated deserves testing even if it’s temporary.
Mind you, I’m not beating myself up here. I’m reflecting on my standards, versus what I’ve done, and observing that they are not aligned. It’s as if I had committed to give up french fries and then ordered a big package of them with lunch. (That, too, may have happened.)
So what caused me to just go ahead and code up that rectangle intersector? Well, I had an example to look at, and I remembered the basic trick:
If my right is left of his left, we don’t intersect, etc.
So I was sure I could do it and it was right to hand. And I had done some testing on the other one, and it was a pain because one needs lots of examples and the calculations seem very tedious and tricky.
In the end, I just “didn’t want to”.
I would prefer not to.
– Bartleby
But I had a lot of balls in the air and I chose to juggle them all. I could have done something simple with the damage, but I tried to do something clever (which I have since backed out). Basically instead of drilling down one path at a time until the path was done, I was drilling on multiple paths.
Something about pre-order post-order end-order I suppose.
Still, all’s well that ends well, and we do have a working demonstration of bombs hitting shields and doing damage, at least if you consider turning red to be damage.
A decent morning’s work in all, but I feel that I was staking on very thin ice. Don’t try this at home.
Let’s sum up.
Summing Up
We have a rather hacked-in version of shields, including detection of collisions and triggering of damage. We can surely extend that version further, even in the condition it’s in now, because at least the various functions are broken out and can be enhanced fairly independently.
We certainly need to do something more clever than have two tables, one with a position and one with a shield image, that just happen to be in correspondence. Probably I’ll make a table pos->image or something like that.
The rectangulator whatchamajigger is a free-range utility function, which might want to be better organized. At it should be usable in the missile as well as in the bomb, so that’ll be a bit of reuse and space saving.
All in all, it’s decent progress. I took a leap into space assuming that I could fly, and either I was able to fly or I landed in a soft pile of hay. So I’ve lived to program another day.
Tomorrow, most likely. See you then!