Space Invaders 26
It’s my ball and if I want to play with it you’re not the boss of me.
Today, I feel like improving some of this code. It’s not particularly urgent but that’s what I want to do today, and you can’t stop me nanner nanner.
My basic plan is to get some more objects up in this batch. Of procedural code. Shields, missiles, gunner. And maybe some new things. We’ll see how it goes.
Gunner tab is 58 lines long, Shield tab is 24. We’ll start with Shield.
But first a word from our sponsor, Small Steps Taken Often.
I was thinking as I drove back from *$ this morning that while these example programs are very tiny, they are very much like the many much larger programs that I’ve worked on and that we’ve all worked on. They are made up of fairly small chunks.
Now if you just suddenly thought of some 1300 line monster method or function that you had to deal with, just hold up there bunkie, because even that monster was made up of smaller bits. It just had them all crammed together. If you were really unlucky, it even had them shuffled together, but the small chunks were in there.
They have to be, because our minds are not capable of holding a big 1300 line monster all at once. We can always inspect that monster carefully and find a little bit that can be understood, described, and pulled out into a function or method on its own.
And that means that refactoring, even in a large system, can always be done in small steps. True, it won’t be done until everyone uses all the small chunks we’ve pulled out, but we can put in those references to the small chunk one at a time.
Sometimes it seems impossible. Sometimes we want to give up. Sometimes we even should give up. But before you give up on refactoring something, ask yourself something like this:
Suppose you had to give up on a stage and announce to the world that this code, right here, was a mess and could not be refactored and you’d stake your life and reputation on that fact. And to prove it, you were going to submit that code to me, to Chet Hendrickson, to James Shore, to J B Rainsberger, to Ward Cunningham, to Kent Beck and then to a half a dozen actually smart people, and they would fail, do you hear me fail to refactor that code. And if through some strange impossible chance, they did refactor it, you would be exposed, there on that stage, as the one person who couldn’t refactor that code. Would you still say it couldn’t be refactored?
I can tell you my answer: I might say I don’t know how to refactor it. I might say I think it’s not worth refactoring. I might say a lot of things. But I’ve never seen code that people like those listed above just couldn’t refactor. And I never expect to.
Anyway, point is, it’s always the same, large and small, and it’s always possible to take small steps to make it better.
And that’s why I think these little game exercises have value in “real life”, because real life is a lot like this.
With that said, let’s do some darn work.
Shield Tab
-- Shields
-- RJ 20200819
function createShields()
local img = readImage(asset.shield)
local posX = 34
local posY = 48
Shields = {}
for s = 1,4 do
local entry = {img=img:copy(), pos=vec2(posX,posY), count=0, alive=true}
table.insert(Shields,entry)
posX = posX + 22 + 23
end
end
function drawShields()
pushStyle()
tint(0,255,0)
for i,shield in ipairs(Shields) do
sprite(shield.img, shield.pos.x, shield.pos.y)
end
popStyle()
end
This is interesting, because all this code is about a collection of shields, each one being a table entry containing an image, a position, a count, and an alive flag. I kind of wonder what the count and alive flags are for.
We see that the Shields are stored in a global table called Shields, so let’s see who else may look at this collection.
Main calls these two methods, but never refers to the collection. Bombs check to see if they’ve hit a shield:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local pos = shield.pos
local img = shield.img
local hit = self:damageShield(img,pos)
if hit then return true end
end
return false
end
One test creates an empty Shields collection, so that the code above can run.
What about references to the count and alive thingies? Are those just copied and pasted from somewhere else?
The only reference to .count are on Gunner. I suspect we could drop that from the Shield. And .alive is referenced only in Gunner and Invader. Those two fields should be able to disappear.
When I remove those and run the program, everything is fine, but I notice something odd:
The bombs are making black marks on the shields. Those used to be red. When did that happen? Looking back at yesterday’s photos, I see red marks. Weird.
Revert. Still happens. This doesn’t matter, because we are going to do something far more exotic for shield damage, but I’m curious just what is going on.
Wouldn’t you just know, you get all preachy and righteous and bang something instantly happens to show how human and fallible you are. Life is great.
A quick review of the code convinces me that we’re setting the hits to red. But remember that tint command, where we tint the white shield green? I bet that killed out the red.
The description of tint says that the color is “multiplied” by the color that’s already there. If red*green is black, that explains it. When I turn off the tint, the red comes back. Well then, that’s interesting. For now, let’s leave in the tint and carry on.
Where we we? Oh, right, we re-remove count and alive from the shield tables. What about that code in Bomb? Look at the whole thread:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local pos = shield.pos
local img = shield.img
local hit = self:damageShield(img,pos)
if hit then return true end
end
return false
end
function Bomb:damageShield(img, 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
function Bomb:applyDamage(img, hit, imagePos)
local relativePos = hit-imagePos
for x = -1,1 do
for y = -1,1 do
img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
All this code is here because there is nobody else to talk to about whether a bomb has hit a shield, so the bomb has to do it all itself, including knowing an awful lot about shields’ shape and size.
I honestly don’t know how this will sort out, but I feel sure that having a shield object will allow us to make it better. And I’m tempted to make another class to manage the collection of them. We’ll see about that. First, the Shield class:
Shield = class()
function Shield:init(img, pos)
self.img = img
self.pos = pos
end
function createShields()
local img = readImage(asset.shield)
local posX = 34
local posY = 48
Shields = {}
for s = 1,4 do
local entry = Shield(img:copy(), vec2(posX,posY))
table.insert(Shields,entry)
posX = posX + 22 + 23
end
end
I just created a Shield instead of a table there in the loop. But now I can tell the object to draw itself, which is no big trick:
function Shield:draw()
tint(0,255,0)
sprite(self.img, self.pos.x, self.pos.y)
end
function drawShields()
pushStyle()
for i,shield in ipairs(Shields) do
Shield:draw()
end
popStyle()
end
Well, it wouldn’t be a big deal if I had lower-cased Shield:
function drawShields()
pushStyle()
for i,shield in ipairs(Shields) do
shield:draw()
end
popStyle()
end
And we’re back in business. Commit: initial Shield class.
Now we can look at the bomb stuff. To refresh my memory and yours:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local pos = shield.pos
local img = shield.img
local hit = self:damageShield(img,pos)
if hit then return true end
end
return false
end
function Bomb:damageShield(img, 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
function Bomb:applyDamage(img, hit, imagePos)
local relativePos = hit-imagePos
for x = -1,1 do
for y = -1,1 do
img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
(I notice the rectanglesIntersect functions are also in the Bomb tab, and that should be addressed. But we’re on a mission right now.)
It’s not as clear as it might be what’s going on here. Let me rename a bit:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = self:damageShield(shield.img,shield.pos)
if hit then return true end
end
return false
end
function Bomb:damageShield(shieldImg, shieldPos)
local hit = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(shieldImg, hit, shieldPos)
return true
end
function Bomb:applyDamage(shieldImg, hit, shieldPos)
local relativePos = hit-shieldPos
for x = -1,1 do
for y = -1,1 do
shieldImg:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
It’s easier to see now that almost all this code is about the shield and almost none about the bomb. Major feature envy here. Let’s start at the bottom with applyDamage:
Meh! Can’t. Don’t have the shield object at this point. We’ll have to start top down. We’ll change this:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = self:damageShield(shield.img,shield.pos)
if hit then return true end
end
return false
end
We’ll pass down the shield object:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = self:damageShield(shield)
if hit then return true end
end
return false
end
That requires us to change this:
function Bomb:damageShield(shieldImg, shieldPos)
local hit = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(shieldImg, hit, shieldPos)
return true
end
To this:
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hit = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(shield, hit)
return true
end
We had to create the temp shieldPos, but we’ll see about getting rid of it shortly. First I’d like to get this stuff moved to where it needs to be.
Rename hit to hitPos for “clarity”. (Yes, “position” would be better. Our local standard uses “pos” and we all understand it.)
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(shield, hitPos)
return true
end
Now we need to convert this:
function Bomb:applyDamage(shieldImg, hit, shieldPos)
local relativePos = hit-shieldPos
for x = -1,1 do
for y = -1,1 do
shieldImg:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
We convert to this:
function Bomb:applyDamage(shield, hitPos)
local relativePos = hitPos - shieldPos
for x = -1,1 do
for y = -1,1 do
shield.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
I expect this to work. It doesn’t. The bombs pass right through the shields. What did I miss, and why aren’t there tests for this?
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hit == nil then return false end
self:applyDamage(shield, hitPos)
return true
end
Failed to rename hit in one of its locations. Renames are tricky in Codea, because there’s no refactoring browser, and not much of a useful replace operator. Fixing that, I see
Bomb:64: bad argument #2 to '__sub' (vec2)
stack traceback:
[C]: in metamethod '__sub'
Bomb:64: in method 'applyDamage'
Bomb:59: in method 'damageShield'
Bomb:49: in method 'killedShield'
Bomb:32: in method 'update'
Army:51: in method 'doUpdate'
Army:41: in method 'update'
Main:34: in function 'draw'
OK …
function Bomb:applyDamage(shield, hitPos)
local relativePos = hitPos - shieldPos
for x = -1,1 do
for y = -1,1 do
shield.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
Another rename fail:
function Bomb:applyDamage(shield, hitPos)
local relativePos = hitPos - shield.pos
for x = -1,1 do
for y = -1,1 do
shield.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
And we’re good.
I really wish I had tests, but in fact running the program is turning up the problems just as quickly as tests would: it just feels a bit more slimy.
I should be able to write a bomb damaging shield test, and for my sins I’ll do it now.
Wow, this is really hard. The basic setup is easy enough, something like this:
_:test("Bomb damages shield", function()
local bomb = Bomb(vec2(100,100))
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(???)
end)
If this works, Bomb:applyDamage will be called and will damage the image in the shield. How can we detect that in some reasonable way?
I had thought to intercept the call to applyDamage, like this:
_:test("Bomb damages shield", function()
local bomb = Bomb(vec2(100,100))
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb.applyDamage = Bomb.interceptDamage()
bomb:damageShield(shield)
_:expect(pos).is(vec2(100,100))
end)
function Bomb:interceptDamage(shield, hitPos)
print("intercepted", shield, hitPos)
end
I wasn’t sure how I was going to get information out of the intercepted function, but it turns out that the print there prints nil nil for shield and hitPos. So somehow, my interception is being called but the parameters aren’t there.
This was deep in the bag of tricks anyway, so fiddling with it much more violates the rule of holes, or the rule of deep bags, or something.
What about this. I’ll insert this line:
function Bomb:applyDamage(shield, hitPos)
local relativePos = hitPos - shield.pos
for x = -1,1 do
for y = -1,1 do
shield.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
shield.damagedAt = relativePos
end
And now I should be able to assert on that value:
_:test("Bomb damages shield", function()
local bomb = Bomb(vec2(100,100))
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(shield.damagedAt).is(vec2(100,100))
end)
This fails as I expect, because I’ve put in the world coordinates not the relative. The failure is this:
12: Bomb damages shield -- Actual: (11.000000, 8.000000), Expected: (100.000000, 100.000000)
And that seems to me to be right, because I drew a picture before I selected the numbers 89 and 92 and 89+11 is 100 and 92+8 is 100.
So I sort of have a test.
_:test("Bomb damages shield", function()
local bomb = Bomb(vec2(100,100))
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(shield.damagedAt).is(vec2(11,8))
end)
I did have to put special code in to make this testable. That’s troubling and needs to go on the list to be addressed. It might be that I’ll not have a better idea. We’ll find out.
I think the intercept should have worked, but it didn’t. Maybe I’ll chase that yak to shave it later, because intercepting things might be kind of useful.
I think now I’m ready to move behavior into Shield class, but first, it’s time for lunch.
Long morning, but I think I’m on solid ground. Let’s commit: refactor bomb-shield interaction, add bomb shield test.
See you in a while …
Friday Afternoon
I left you at about 1100 Friday and it is almost 1600 now. Despite the odds that it’s a rathole, I’ve decided to take another run at intercepting the applyDamage call, in order to test it.
Here’s the test now:
_:test("Bomb damages shield", function()
local bomb = Bomb(vec2(100,100))
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(shield.damagedAt).is(vec2(11,8))
end)
I’ve patched the applyDamage to return the relative position used in that function.
Here’s Plan C:
_:test("Bomb damages shield", function()
local apply = function(object, shield, pos)
print("apply", object, shield, pos)
end
local bomb = Bomb(vec2(100,100))
bomb.applyDamage = apply
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(shield.damagedAt).is(vec2(11,8))
end)
I define a function local to the test and replace the applyDamage in bomb with it. The result is that it prints the information I expect, and that the test itself fails, because my intercepting function doesn’t return the right stuff.
apply table: 0x281b859c0 table: 0x281b96040 (100.000000, 100.000000)
12: Bomb damages shield -- Actual: nil, Expected: (11.000000, 8.000000)
Now let’s make my interceptor call the original:
_:test("Bomb damages shield", function()
local bomb = Bomb(vec2(100,100))
local original = bomb.applyDamage
local apply = function(object, shield, pos)
print("apply", object, shield, pos)
return original(object, shield, pos)
end
bomb.applyDamage = apply
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(shield.damagedAt).is(vec2(11,8))
end)
Here, I’ve saved out the original applyDamage, called the saved function from within my interceptor, and then called the original. (I wanted to say object:original(shield,pos) but that doesn’t work, probably because of the syntactic sugar. Anyway, this does the print and the test passes. Now I can fetch out whatever information I want, and plunk it into the test. (However, I probably can’t get at everything I want yet.)
_:test("Bomb damages shield", function()
local hitAt
local bomb = Bomb(vec2(100,100))
local original = bomb.applyDamage
local apply = function(object, shield, pos)
hitAt = pos
return original(object, shield, pos)
end
bomb.applyDamage = apply
local shield = Shield(readImage(asset.shield), vec2(89,92))
bomb:damageShield(shield)
_:expect(hitAt).is(vec2(100,100))
end)
Notice that I changed the expect to get the world position, not the relative position we had before.
This is OK with me for now, I just wanted to find out how to make interception work. It’s still pretty darn deep in the bag.
Now I can remove that special save from this:
function Bomb:applyDamage(shield, hitPos)
local relativePos = hitPos - shield.pos
for x = -1,1 do
for y = -1,1 do
shield.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
shield.damagedAt = relativePos -- < remove this
end
Anyway, this was just a side project done in my off time, to see how to do an interception. I’ve learned a way to do it and am not entirely clear on why the first way didn’t work. Don’t care.
However …
We’re here to refactor
All this code in Bomb belongs in Shield in my view. Let’s start moving it.
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hitPos == nil then return false end
self:applyDamage(shield, hitPos)
return true
end
function Bomb:applyDamage(shield, hitPos)
local relativePos = hitPos - shield.pos
for x = -1,1 do
for y = -1,1 do
shield.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
This becomes:
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hitPos == nil then return false end
shield:applyDamage(hitPos)
return true
end
Together with this:
function Shield:applyDamage(hitPos)
local relativePos = hitPos - self.pos
for x = -1,1 do
for y = -1,1 do
self.img:set(relativePos.x + x, relativePos.y + y, 255,0,0)
end
end
end
This will break my deep in the bag of tricks test, but I expect the game to run. And yes, it does run. Now let’s move another method, or maybe two. We have this:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = self:damageShield(shield)
if hit then return true end
end
return false
end
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hitPos == nil then return false end
shield:applyDamage(hitPos)
return true
end
We see now that we have some seriously magical numbers in the damageShield method, and we can’t just push it over to Shield as it stands. What if we do this instead, up in killedShield …
You know what? There’s an interesting question here. I’m glad we dug into this. Whose job it is to damage the actual shield bitmap? On the one hand, the bomb presumably knows how much damage it can do, but on the other hand, we don’t want to just be handing out our bitmap to other objects to fiddle with it.
I think I’ll settle this in the direction we have been heading: the shield will damage itself. We might let it call back to the bomb for information. Let’s see where this goes …
We’ve got this in Bomb:
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hitPos == nil then return false end
shield:applyDamage(hitPos)
return true
end
To move that to Shield, we’ll have to pass in the bomb as a parameter, because we need both positions. We’ll do that, then see what we can do to clean it up.
function Shield:damage(bomb)
local hitPos = rectanglesIntersectAt(bomb.pos,3,4, self.pos,22,16)
if hitPos == nil then return false end
self:applyDamage(hitPos)
return true
end
We’ll call that:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = shield:damage(self)
if hit then return true end
end
return false
end
This works. (The test is still borked. I feel like it served its real purpose, learning a deep trick, but let’s leave it around and see what happens.
Now we do know that we’re calling the damage function in a loop, and we’d like to terminate that loop early. On the other hand, we can’t possibly hit more than one shield. So what if, in the event of a collision, we called back to the bomb saying you’ve exploded? I think that’s better, but if that’s the case, we could have done that from the bomb side just as well.
I’ll revert that last change. Sure wish I had committed the first bit.
We’re back to this:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = self:damageShield(shield)
if hit then return true end
end
return false
end
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hitPos == nil then return false end
shield:applyDamage(hitPos)
return true
end
Now that call to rectanglesIntersectAt uses information about the shield that the bomb should not know. (And if we put it in Shield the reverse would be true.
What we really want to know is whether the bomb’s rectangle and the shield’s rectangle overlap, and if so, where. What we will want to know in the near future is whether the two objects have any non-zero bits in common.
Let’s commit the current working code and go after that. Commit: refactor applyDamage from Bomb to Shield. But first, I’ll ignore that test.
Bitmaps Intersect
I have an online party to go to in a few minutes, but let’s at least try a cut at this. Let’s see … we have two bitmaps of unknown shape (but kind of rectangular) who may be overlapping. In the one, the shield, we really only want to consider overlap if the other, the bomb, covers a colored bit in the shield. If the shield bit is black, no harm.
The same may not be true the other way. We may wish to treat the bombs as only intersecting if one of their colored bits hits a colored bit, or we may wish to check all the bits, colored or not.
What if the bomb, which is smaller, would generate its bits one by one and inquire of the shield whether it had hit anything?
We’ll surely want to check for rectangle overlap before we do this, we can’t go searching all the bits on the screen. But let’s try to build the basic bit checker stuff with some TDD.
This should be amusing.
I decided I needed an object, so wrote this test:
_:test("Images intersect", function()
local white = color(255)
local b1 = Bitmap(2,2)
b1.bits:set(0,0,white)
b1.bits:set(1,1,white)
_:expect(b1:hasBit(1,1)).is(true)
end)
And this class:
Bitmap = class()
function Bitmap:init(w,h)
self.bits = image(w,h)
end
function Bitmap:hasBit(x,y)
local col = self.bits:get(x,y)
return col ~= color(0)
end
That actually works. Nearly amazing. But I really don’t think we want to call our hasBit function a million times in a loop. On the other hand, make it work then make it right then make it fast. What do I actually want to ask of this bitmap?
I want to ask it whether in some rectangle in its local space, it has any non-black bits, I guess I’ll just write that:
_:test("Bitmap rectangle", function()
local white = color(255)
local b1 = Bitmap(2,2)
b1.bits:set(0,0,white)
b1.bits:set(1,1,white)
_:expect(b1:hasBitsIn(1,1,1,1)).is(true)
_:expect(b1:hasBitsIn(1,0,1,1)).is(false)
end)
I’m on the hairy edge of the party and tired to boot, but maybe I can make this work pretty readily:
function Bitmap:hasBitsIn(x1,y1,x2,y2)
for x = x1,x2 do
for y = y1,y2 do
if self:hasBit(x,y) then return true end
end
end
return false
end
I had high hopes for that but it gets an error, trying to fetch a pixel out of bounds. But that’s OK, because our Bitmap knows its own coordinates. So we can do:
function Bitmap:hasBitsIn(x1,y1,x2,y2)
for x = x1,x2 do
for y = y1,y2 do
if self:inBounds(x,y) and self:hasBit(x,y) then return true end
end
end
return false
end
And we’re left with the trivial concern of doing inBounds:
Oops, my bad I had misremembered that images start at 1,1 not 0,0. Slight revision and:
function Bitmap:hasBitsIn(x1,y1,x2,y2)
for x = x1,x2 do
for y = y1,y2 do
if self:inBounds(x,y) and self:hasBit(x,y) then return true end
end
end
return false
end
function Bitmap:inBounds(x,y)
if x < 1 or y < 1 then return false end
return x <= self.maxX and y <= self.maxY
end
That passes the first test but the second comes back true. And it should, because the rectangle (1,0,1,1) does overlap us. I need to fix that test:
No. It is too much. It’s party time and my brain is fried. I’ll revert this and come back tomorrow.
It’s tempting to save it, I’m sure it’s nearly good But it’ll be better tomorrow.
See you then …
Tomorrow + 1
Saturday did come, but I’d left this iPad unplugged, to let the battery discharge a bit, and I must have left the game running, because it was totally flat when I came back to work on this.
You’re wondering why I didn’t just plug it in. Well, when working, I put the iPad on my keyboard tray, and the wire won’t reach. Yes, I could move it, but it was a good excuse not to do any work yesterday.
Now it’s Sunday, and I won’t do much, since that’s our day for a leisurely breakfast and some TV and reading. But I think there’s time to do a bit.
It is too much
I’ve come to think that I’m making too big a deal of this tiny problem, generalizing, deciding how to be all “tell, don’t ask” and so on. My new plan is to assess where we are, and then see about programming directly to what we actually need, not what seems “right”. Then, if it isn’t right, we’ll make it right.1
Let’s have a butcher’s at the code now. Here’s what’s running in the game:
function Bomb:killedShield()
for i,shield in ipairs(Shields) do
local hit = self:damageShield(shield)
if hit then return true end
end
return false
end
function Bomb:damageShield(shield)
local shieldPos = shield.pos
local hitPos = rectanglesIntersectAt(self.pos,3,4, shieldPos,22,16)
if hitPos == nil then return false end
shield:applyDamage(hitPos)
return true
end
function rectanglesIntersectAt(bl1,w1,h1, bl2,w2,h2)
if rectanglesIntersect(bl1,w1,h1, bl2,w2,h2) then
return bl1
else
return nil
end
end
function rectanglesIntersect(bl1,w1,h1, bl2,w2,h2)
local tr1 = bl1 + vec2(w1,h1) - vec2(1,1)
local tr2 = bl2 + vec2(w2,h2) - vec2(1,1)
if bl1.y > tr2.y or bl2.y > tr1.y then return false end
if bl1.x > tr2.x or bl2.x > tr1.x then return false end
return true
end
Friday I started trying to TDD a kind of bitmap object that could check for collisions, but it wasn’t going well and I was on a 20 minute deadline. Let’s try to rethink about this.
We want to detect whether an incoming bomb has actually struck a shield. The bomb is a 3x4 rectangle containing some fancy sprite work. The shield lives inside a 22x16 rectangle, but our plan is that bombs will damage the shield, removing some of the shield’s bits to show damage. A subsequent bomb does not hit the shield unless it actually hits a visible bit in the shield.
Now, it’s true that “make it fast” is last in our little mantra, but there is a natural progression to think about. The bomb might not yet be low enough to hit any shield. It might not be within the x borders of any shield. It might be too low to hit a shield. And then finally, it might be within the shield’s bounds but not hitting any visible bits in the image. It makes sense logically to make these checks in order.
I confess that I’m at least a bit tempted not to make the checks and to go right to checking the bits. We’ll have to write that code anyway, and it is both necessary and sufficient for our purpose. It just seems kind of stupid not to do the other checks first.
Well, the borders of stupid are where all the learning is, so what the heck, let’s do it. That being the case, let’s try again to TDD it.
Here’s a cut at a test. It’s a bit ambitious but we’ll see how it goes:
_:test("rectangle covers visible bits", function()
local white = color(255)
local img = image(22,16) -- lower left quadrant white
for x=1,11 do
for y = 1,8 do
img:set(x,y,white)
end
end
local shield = Shield(img,vec2(100,100))
local hitNo = shield:hitBits(vec2(112,109),3,4)
local hitYes = shield:hitBits(vec2(100,100),3,4)
_:expect(hitNo).is(false)
_:expect(hitYes).is(true)
end)
This creates an image that’s white in the lower left quadrant, and creates a shield with that image, because I’m planning to TDD a method, hitBits, on Shield. The test then calls hitBits twice, once with a rectangle that’s outside the white, and once with a rectangle that’s inside. We’ll see what this test drives.
13: rectangle covers visible bits -- Tests:140: attempt to call a nil value (method 'hitBits')
No surprise here, the method doesn’t exist yet.
function Shield:hitBits(pos,w,h)
return false
end
“Fake it ‘til you make it”, another Kent Beck gift. The first check should succeed, and the second fail.
13: rectangle covers visible bits -- Actual: false, Expected: true
So far so good. Now what?
Well, we know the dimensions of the bomb, w, h, so we can ask the Shield whether it has a bit at each location in the bitmap. How about this:
function Shield:hitBits(pos,w,h)
for x = pos.x,pos.x+w do
for y = pos.y,pos.y+h do
local has = self:hasBit(x,y)
if has then return true end
end
end
return false
end
I’m positing a new function hasBit that accepts real coordinates and answers whether we have a visible bit at that position. Now to write that …
function Shield:hasBit(x,y)
local xx = x - self.pos.x
local yy = y - self.pos.y
if xx < 1 or xx > 22 or yy < 1 or yy > 16 then return false end
local r,g,b = self.img:get(xx,yy)
return r + b + g > 0
end
I compute the local values of x and y by subtracting our position. Then if the result is inside the image’s dimensions, I fetch the bit.
But wait. Is the other loop right? It’s going, e.g. from x to x + w. That should be w-1 I think. I’ll change it now but we need to write some tests to check that we’ve covered just the right bits.
Both the tests now pass. Let’s spatter some shots around and do some more checking.
I’ll add them one at a time. Arguably I should write a new test for each of these, but I’ll just extend this one.
local hit = shield:hitBits(vec2(97,100),3,4)
_:expect(hit).is(false)
This is just far enough off to the left to miss, if the code is correct. And it does. Now I’ll remove the -1 and it should fail.
And it does not fail. Ah. I see another issue.
When we subtract the rectangle position from our position, we get a zero-based value. These images, bless their hearts, and one-based. So …
function Shield:hasBit(x,y)
local xx = x - self.pos.x + 1
local yy = y - self.pos.y + 1
if xx < 1 or xx > 22 or yy < 1 or yy > 16 then return false end
local r,g,b = self.img:get(xx,yy)
return r + b + g > 0
end
Now, without the subtract - 1 in the outer method, I expect the 97 test to fail and not the others. But I’m scared, that’s more defects than I’m comfortable with.
But it fails as intended. However, the output is tricky to understand, as often happens when we put multiple expectations in a single test. I can’t be sure which test failed. We’ll address that in a moment, but first I’ll put back the - 1s.
OK, tests all good. I’ve made myself nervous, though, so I want more tests. And I’ll do them a bit more nearly correctly,
_:test("rectangle covers visible bits", function()
local shield = testShield()
local hitNo = shield:hitBits(vec2(112,109),3,4)
local hitYes = shield:hitBits(vec2(100,100),3,4)
_:expect(hitNo).is(false)
_:expect(hitYes).is(true)
local hit = shield:hitBits(vec2(97,100),3,4)
_:expect(hit).is(false)
end)
function testShield()
local white = color(255)
local img = image(22,16) -- lower left quadrant white
for x=1,11 do
for y = 1,8 do
img:set(x,y,white)
end
end
return Shield(img,vec2(100,100))
end
Pull out the creation of the test shield. Let’s go the whole way and break up our first test, then do more.
_:test("rectangle high right hits no visible bits", function()
local shield = testShield()
local hitNo = shield:hitBits(vec2(112,109),3,4)
_:expect(hitNo).is(false)
end)
_:test("rectangle at origin hits visible bits", function()
local shield = testShield()
local hitYes = shield:hitBits(vec2(100,100),3,4)
_:expect(hitYes).is(true)
end)
_:test("rectangle off to left misses visible bits", function()
local shield = testShield()
local hit = shield:hitBits(vec2(97,100),3,4)
_:expect(hit).is(false)
end)
Now for a few more tests just to increase certainty. I’ll do one that just barely hits the top of my image:
_:test("rectangle just above hits visible bits", function()
local shield = testShield()
local hit = shield:hitBits(vec2(105,108),3,4)
_:expect(hit).is(true)
end)
I figure this just ticks us in the middle of the top row of visible bits. However, it fails:
16: rectangle just above hits visible bits -- Actual: false, Expected: true
My bad. 108 is above the first bits. This zero-based graphics and one-based bitmap thing will drive me crazy soon. The image goes from 1-8 putting bits into 100-107. So 108 is outside. Change the test to 107 … and it runs.
OK, I’m about convinced that this baby is working, and it’s about time for breakfast. But what about our very first test:
_:test("rectangle high right hits no visible bits", function()
local shield = testShield()
local hitNo = shield:hitBits(vec2(112,109),3,4)
_:expect(hitNo).is(false)
end)
That can be tightened up, can’t it? The 109 can clearly be 108. Still works.
We might want to do some more tests. For now, some quick remarks and then breakfast. I’ll continue this article tomorrow (Monday) if today goes as I expect.
Remarks
The difference between zero-based graphics and one-based bitmaps has bitten me quite a few times in this exercise. We have a kind of clash of the representations. Lua arrays are 1-based, so it makes sense that images would be as well, as they are just arrays of color. But the screen on the iPad, like most screens, is zero-based. So when we need to convert between a screen coordinate and an image coordinate, we get all these off-by-one adjustments, and they’re hard to keep track of, especially in a head like mine.
What we could do, at least in principle, is create a new abstraction and use it. For example, instead of using Codea images directly, we could have an Image class that was 1-based, doing the 1-adjustments all inside that class.
Doing that should mean that most of our address adjustment would go away, at the cost of a function dispatch on every bit access. That might be too high a price to pay, but if we didn’t access the bits very often, it could be acceptable.
We could even imagine folding in operations larger than one bit in side, and doing the translation inside, much as we’ve done here in shield.
And there might be some “in-between” solution that we could put into Shield, to make these operations less error-prone.
For now, this morning, all I’ve got is the problem, but in the case in hand, our bitmap checking, I think we’ve resolved it.
One more thing
This went very smoothly, despite some worries about plus or minus one. The TDD helped by letting me focus on specific cases that helped me see whether the code was right (or nearly right, those darn +/- 1s).
But the other thing that helped was the one or two previous attempts, and my willingness to say Oh $%$# it and toss them out. That’s a very powerful tactic, for me at least. When it’s not going well, notice that it’s not going well, take a break, and start over.
Enough for Sunday. My tea is ready.
See you tomorrow!
(No zipped code today, I forgot to save it and I’m on Monday already. Next time.)
-
That reminds me to mention this. I almost certainly learned “Make it work, make it right, make it fast” from Kent Beck. Kent mentioned on Twitter yesterday that he learned it from his father. Wherever you heard it, it’s a good way to think. ↩