Spacewar! 31 - Moving into the universe.
Moving into the universe
I’m back on station at the Brighton Agile Roundtable, and the plan is to complete the move to Universe. Without having looked at the code since yesterday, my definition of complete will be that there will be no globals other than U the Universe, the U table will be gone from Universe, if it isn’t already, and there will be no global functions in Main.
Let’s take a look:
-- HAVE YOU PUSHED TO GITHUB TODAY?
-- S3 Spacewar
-- The Universe
U = Universe()
U.universe = U
function setup()
Ship(1)
Ship(2)
U.Running = true
end
function touched(touchid)
U.universe:touched(touchid)
end
function draw()
U.universe:draw()
end
function moveAll()
for _, obj in pairs(U.Drawn) do
obj:move()
end
end
function interactAll()
for _, a in pairs(U.universe.Drawn) do
for _, b in pairs(U.universe.Drawn) do
if colliding(a,b) then
a:die()
b:die()
end
end
end
end
function drawAll()
background(40, 40, 50)
for _, obj in pairs(U.universe.Drawn) do
obj:draw()
end
end
function addTouched(anObject)
U.universe:addTouched(anObject)
end
function clip_to_screen(vec)
return vec2(vec.x%WIDTH, vec.y%HEIGHT)
end
function colliding(obj1, obj2)
if obj1 == nil then return false end
if obj2 == nil then return false end
if obj1.cannotCollide then return false end
if obj2.cannotCollide then return false end
if obj1 == obj2 then return false end
local d = obj1.pos:dist(obj2.pos)
local result = d < U.MissileKillDistance
return result
end
Ah. And while we’re at it, let’s remove that Running flag. I was going to use that in testing and I think it’s unused. I’ll find all references to it and remove them, unless I find someone actually needing it … and I do not. Two sets of it, no accesses. Gone. Now for those functions. The colliding function looks like a good one to put on Universe, though one could imagine a superclass of the objects that could handle it. We don’t have one of those just now, and we’re on a mission. I’ll move colliding and change references.
function Universe:colliding(obj1, obj2)
if obj1 == nil then return false end
if obj2 == nil then return false end
if obj1.cannotCollide then return false end
if obj2.cannotCollide then return false end
if obj1 == obj2 then return false end
local d = obj1.pos:dist(obj2.pos)
local result = d < U.MissileKillDistance
return result
end
There’s just one call to colliding, up there in Main interactAll. There are two ways to go: one is to fix interactAll in place and then move it when its turn comes, or to move it now and use self. The former is safer and with no pair – and my history – I’d better play it safe:
function interactAll()
for _, a in pairs(U.universe.Drawn) do
for _, b in pairs(U.universe.Drawn) do
if Universe:colliding(a,b) then
a:die()
b:die()
end
end
end
end
We note those U.universe references. They should be able to be removed now but let’s run first. System works fine. But see below. Removing those U.universe.Drawn refs to U.Drawn and everything works fine. However …
To be sure these changes work, I have to run the program. I don’t really have tests that are good enough to give me confidence, which means that I have to run through some manual bullet-firing and such. This isn’t ideal. In addition, I’ve stopped relying on my tests enough that when I tried them just now, the Missile Kill tests no longer run. This is a Very Bad Sign. When we stop relying on our tests, they decay, and we are tempted never to run them or even to delete them. Do you want the 20th Century back? That’s how you get the 20th Century back!
Let’s fix that broken test and then see what else we might do.
-- HAVE YOU PUSHED TO GITHUB TODAY?
function testKill()
_:describe("Missiles Kill", function()
_:before(function()
dSave = U.Drawn
tSave = U.Touched
U.Drawn = {}
U.Touched = {}
end)
_:after(function()
U.Drawn = dSave
U.Touched = tSave
end)
_:test("All exist", function()
Ship(1)
Ship(2)
local found = {}
for _, obj in pairs(U.Drawn) do
found[obj] = obj.name
end
_:expect(found).has("Ship 1")
_:expect(found).has("Ship 2")
end)
_:test("Ship no longer kills self by firing missile", function()
local ship1 = Ship(1)
Ship(2)
Missile(ship1)
collisions()
local found = {}
for _, obj in pairs(U.Drawn) do
found[obj] = obj.name
end
_:expect(found).has("Ship 1")
_:expect(found).has("Ship 2")
end)
end)
end
The first failure is in “All exist”:
1: All exist -- Actual: table: 0x15632c9a0, Expected: Ship 1
1: All exist -- Actual: table: 0x15632c9a0, Expected: Ship 2
I’m not sure what this is. We’re doing this cute trick of tucking away the Drawn table and using our own. Do we have nothing in our own? Has it come unhooked during the switchover? A quick print shows me that our found table is empty, because the Drawn table is empty. Why? Because we changed how objects get added and removed. When things are created now, they add themselves to U.Adds, and they are only added to the Universe during draw:
function Universe:draw()
U.Tick = U.Tick + 1
self:removeDeadObjects()
self:addInNewObjects()
moveAll()
interactAll()
drawAll()
end
I’d rather not call draw from here. Let’s factor out the calls to removeDeadObjects and addInNewObjects and then call from the test:
function Universe:draw()
U.Tick = U.Tick + 1
self:updateUniverse()
moveAll()
interactAll()
drawAll()
end
function Universe:updateUniverse()
self:removeDeadObjects()
self:addInNewObjects()
end
-- TestKill
_:test("All exist", function()
Ship(1)
Ship(2)
U:updateUniverse()
local found = {}
for _, obj in pairs(U.Drawn) do
found[obj] = obj.name
end
_:expect(found).has("Ship 1")
_:expect(found).has("Ship 2")
end)
That fixes the first problem. The second test is going to need that but it also has an issue with collisions:
2: Ship no longer kills self by firing missile -- TestKill:35: attempt to call a nil value (global 'collisions')
We fix:
_:test("Ship no longer kills self by firing missile", function()
local ship1 = Ship(1)
Ship(2)
Missile(ship1)
U:updateUniverse()
interactAll()
local found = {}
for _, obj in pairs(U.Drawn) do
found[obj] = obj.name
end
_:expect(found).has("Ship 1")
_:expect(found).has("Ship 2")
end)
Here again, bad news. Note that we renamed the collisions function to interactAll days ago. Another sign that we’re not relying on the tests. Bad habit, but also signals that the tests aren’t helping us moment to moment.
What would improve the tests enough so that we’d want to run them? Well, my current “standard” manual test is to start the game, fire a missile that will wrap around and kill me, then rotate right and fire a couple to kill the other guy. Like this:
MOVIE HERE
Could we automate this in a test, at least to the point of firing and turning? This test removes the real Drawn table, but if we didn’t do that then anything we add in a test should fly. However, as long as a test is running, the Codea draw is not going to be called: Codea works by relinquishing control, not by interrupts. But how about this? What if we were to add another test button, and that one, after setting up some conditions, puts some kind of Checker object into U.Drawn, so the checker is called in each draw cycle, and it can do thing, check for ships being killed, stuff like that?
I think that would be a good thing. Should I do it now? I’m “in the middle” of a refactoring, to move things into Universe. On the other hand, I’m held back, at least in principle, by not having god enough tests. On the gripping hand, though, this test will be a fair amount of work before it’s as good as my quick manual test. So I am tempted to proceed with the refactoring.
But wait! We’re doing this refactoring right for a change. Things aren’t all moved over, but they are all hooked up so that the system works. And maybe we can devise a really simple first test.
What if our new test button just did this:
- Fire a missile from Ship1
- Rotate Ship1 90 degrees left
- Fire another missile
That should duplicate my manual test with perfect accuracy. I’d have to wait and see if the ships die and the “Blammo!” goes away but that’s OK.
Let’s try that. The tab can’t be named TestXXX though, because that will attract the attention of the testing framework. I’ll put it in Main for now:
-- Main
function setup()
parameter.action("Automate", function()
automate()
end)
Ship1 = Ship(1)
Ship(2)
end
function automate()
Missile(Ship1)
Ship1.heading = Ship1.heading + 90
Missile(Ship1)
end
Wow, that works a treat! Watch!
MOVIE HERE
That saves a little time, though the missile movement is slow. Speeding up time is tricky, though, because objects will move further. We don’t check to see whether the Missile’s path has intersected the Ship: we check on each draw whether in fact things are colliding. We could probably speed things up a bit but it would be even more of a hack than we have now. Nonetheless I think I’ll try it. What if we changed Missile velocity?
-- Main
function automate()
Missile(Ship1, vec2(0,10))
Ship1.heading = Ship1.heading + 90
Missile(Ship1, vec2(0,10))
end
-- Missile
function Missile:init(ship, testVel)
local myVel = testVel or vec2(0,1)
self.name = "missile" .. Missile.count
Missile.count = Missile.count + 1
self.pos = ship.pos + self:aWaysOutInFront(ship)
local velocity = myVel:rotate(math.rad(ship.heading))
self.vel = ship.vel + velocity
self.maxDistance = WIDTH*1.4
self.distance = 0
U:addObject(self)
end
I added that testVel, myVel stuff to Missile. That local statement:
local myVel = testVel or vec2(0,1)
Sets myVel to the value of testVel if it isn’t nil (as it isn’t in my test in Main), otherwise to vec2(0,1), which is what it always used to be. It works a treat! Watch this:
MOVIE HERE
Sweet! I love it. Now back to work. What else can we refactor? Let’s find all the references to U.universe and replace them as needed:
They’re all in Main, which looks like what’s just below. Note in particular that in the init, U.universe is set to U. We should be able to remove that set, and all the references to .universe and be better off. (Not done, but better off.)
-- HAVE YOU PUSHED TO GITHUB TODAY?
-- S3 Spacewar
-- The Universe
U = Universe()
U.universe = U
function setup()
parameter.action("Automate", function()
automate()
end)
Ship1 = Ship(1)
Ship(2)
end
function automate()
Missile(Ship1, vec2(0,10))
Ship1.heading = Ship1.heading + 90
Missile(Ship1, vec2(0,10))
end
function touched(touchid)
U.universe:touched(touchid)
end
function draw()
U.universe:draw()
end
function moveAll()
for _, obj in pairs(U.Drawn) do
obj:move()
end
end
function interactAll()
for _, a in pairs(U.Drawn) do
for _, b in pairs(U.Drawn) do
if Universe:colliding(a,b) then
a:die()
b:die()
end
end
end
end
function drawAll()
background(40, 40, 50)
for _, obj in pairs(U.universe.Drawn) do
obj:draw()
end
end
function addTouched(anObject)
U.universe:addTouched(anObject)
end
function clip_to_screen(vec)
return vec2(vec.x%WIDTH, vec.y%HEIGHT)
end
I noticed that Universe:colliding as well. That should be U.colliding. Fixing all that:
-- HAVE YOU PUSHED TO GITHUB TODAY?
-- S3 Spacewar
-- The Universe
U = Universe()
function setup()
parameter.action("Automate", function()
automate()
end)
Ship1 = Ship(1)
Ship(2)
end
function automate()
Missile(Ship1, vec2(0,10))
Ship1.heading = Ship1.heading + 90
Missile(Ship1, vec2(0,10))
end
function touched(touchid)
U:touched(touchid)
end
function draw()
U:draw()
end
function moveAll()
for _, obj in pairs(U.Drawn) do
obj:move()
end
end
function interactAll()
for _, a in pairs(U.Drawn) do
for _, b in pairs(U.Drawn) do
if U:colliding(a,b) then
a:die()
b:die()
end
end
end
end
function drawAll()
background(40, 40, 50)
for _, obj in pairs(U.Drawn) do
obj:draw()
end
end
function addTouched(anObject)
U:addTouched(anObject)
end
function clip_to_screen(vec)
return vec2(vec.x%WIDTH, vec.y%HEIGHT)
end
Test runs. Mmm, I really like that little test button. Can I remove the global addTouched? Let’s find references …
Button = class()
function Button:init(x, y)
self.name = string.format("Button %f %f", x, y)
self.pos = vec2(x,y)
self.radius = 50
self.pressed = false
self.capturedID = nil
self.cannotCollide = true
addTouched(self)
U:addObject(self)
end
If we refer that to U, we should be able to remove the global addTouched. And that works.
Time to commit a version. Past time, even. Hold on. Hm. Timed out the first time but then it pushed. All OK.
Things are going smoothly, and I do feel better with my tests running. Let’s move the clipToScreen.
function Universe:clip_to_screen(vec)
return vec2(vec.x%WIDTH, vec.y%HEIGHT)
end
function Missile:move()
self.distance = self.distance + self.vel:len()
if self.distance > self.maxDistance then
self:die()
return
end
self.pos = U:clip_to_screen(self.pos + self.vel)
end
function Ship:move()
local turn
local thrust
if self.controls.left.pressed and not self.controls.right.pressed then turn = Ship.TurnAmount
elseif self.controls.right.pressed and not self.controls.left.pressed then turn = -Ship.TurnAmount
else turn = 0 end
self.heading = self.heading + turn
if self.controls.thrust.pressed then thrust = Ship.ThrustAmount else thrust = vec2(0,0) end
self.vel = self:adjustedVelocity(self.vel, self.heading, thrust)
self.pos = U:clip_to_screen(self.pos + self.vel)
if self.controls.fire.pressed then self:fireMissile() end
end
Works just fine. What’s left in Main?
-- HAVE YOU PUSHED TO GITHUB TODAY?
-- S3 Spacewar
-- The Universe
U = Universe()
function setup()
parameter.action("Automate", function()
automate()
end)
Ship1 = Ship(1)
Ship(2)
end
function automate()
Missile(Ship1, vec2(0,10))
Ship1.heading = Ship1.heading + 90
Missile(Ship1, vec2(0,10))
end
function touched(touchid)
U:touched(touchid)
end
function draw()
U:draw()
end
function moveAll()
for _, obj in pairs(U.Drawn) do
obj:move()
end
end
function interactAll()
for _, a in pairs(U.Drawn) do
for _, b in pairs(U.Drawn) do
if U:colliding(a,b) then
a:die()
b:die()
end
end
end
end
function drawAll()
background(40, 40, 50)
for _, obj in pairs(U.Drawn) do
obj:draw()
end
end
We have moveAll, interactAll, drawAll. I’ll move one at a time, testing after each one. I’ll not trouble you with the intermediates unless something interesting happens.
Universe = class()
function Universe:init(x)
self.Touched = {}
self.Adds = {}
self.Tick = 0
self.Adds ={}
self.Drawn = {}
self.Removes = {}
self.Touched = {}
self.MissileKillDistance = 20
end
function Universe:draw()
U.Tick = U.Tick + 1
self:updateUniverse()
self:moveAll()
self:interactAll()
self:drawAll()
end
function Universe:touched(touch)
for _,obj in ipairs(self.Touched) do
obj:touched(touch)
end
end
-- helper functions
function Universe:addInNewObjects()
for _, obj in ipairs(self.Adds) do
self.Drawn[obj] = obj
end
self.Adds = {}
end
function Universe:addObject(anObject)
table.insert(self.Adds, anObject)
end
function Universe:addTouched(anObject)
table.insert(self.Touched, anObject)
end
function Universe:clip_to_screen(vec)
return vec2(vec.x%WIDTH, vec.y%HEIGHT)
end
function Universe:colliding(obj1, obj2)
if obj1 == nil then return false end
if obj2 == nil then return false end
if obj1.cannotCollide then return false end
if obj2.cannotCollide then return false end
if obj1 == obj2 then return false end
local d = obj1.pos:dist(obj2.pos)
local result = d < U.MissileKillDistance
return result
end
function Universe:drawAll()
background(40, 40, 50)
for _, obj in pairs(U.Drawn) do
obj:draw()
end
end
function Universe:interactAll()
for _, a in pairs(U.Drawn) do
for _, b in pairs(U.Drawn) do
if U:colliding(a,b) then
a:die()
b:die()
end
end
end
end
function Universe:kill(anObject)
table.insert(self.Removes, anObject)
end
function Universe:moveAll()
for _, obj in pairs(U.Drawn) do
obj:move()
end
end
function Universe:removeDeadObjects()
for _, obj in ipairs(self.Removes) do
self.Drawn[obj] = nil
end
self.Removes = {}
end
function Universe:updateUniverse()
self:removeDeadObjects()
self:addInNewObjects()
end
Everything was inside Universe, except that I had to edit the TestKill to refer to U:interactAll
_:test("Ship no longer kills self by firing missile", function()
local ship1 = Ship(1)
Ship(2)
Missile(ship1)
U:updateUniverse()
U:interactAll()
local found = {}
for _, obj in pairs(U.Drawn) do
found[obj] = obj.name
end
_:expect(found).has("Ship 1")
_:expect(found).has("Ship 2")
end)
And we’re done. We have removed the U table and are running on U the Universe. Code is pushed. Now let’s talk about what happened?
Good thing, or bad thing?
This is clearly a better design than the U table and all the stuff in Main. Clear to me, at least. But what about the Shoulds? 1 We “should” have seen the need for a full Universe object instead of the U table. Well, since I’m an OO kind of guy, yes, in fact I did see that. But I wanted to play with a simpler object, just a classless table with functions built into it. So I don’t mind the exploration, and it seems possible to me that we could have extended that table, perhaps adding a self parameter to the functions, and kept it as a table but with object syntax. I’m not at all sure that’s a good idea but it may be worth exploring that kind of thing one day.
Anyway, we could have seen and done the Universe object sooner. And we didn’t. What did it cost us, and what did we learn?
I feel sure that it took longer to refactor from U-table to U-universe than it would have if we had started with U-universe, and the development of new functionality would be about the same. So we “wasted” some time. Maybe we “should” have designed more. Yeah, well, maybe. But I set out to learn as I program, and I imagine you are reading this to see what happens when things don’t go perfectly. What happened was that we refactored out way to a better design. Most of the time, we kept the system running, over three or four or more commits, while we were refactoring U->U. We could have stretched that refactoring out over a longer time interval, adding features as needed, and it probably wouldn’t have been much worse.
We had some stumbles and setbacks. If I set my mind to it, I’m sure I could find some really dumb mistakes that cost me time. You probably noticed more.
Back when Adventures in C# was on the market, Chet ran into someone who had read the book. The person said “I’d be reading along and Ron would say he was going to try something, and I’d say ‘That’s not gonna work’ and a few pages later Ron says ‘That didn’t work’”. My point exactly. That book, and this material, is about what really happens when you program. It’s about recognizing that mistakes will be made (quite likely by me, maybe by you) and that the thing is to find them as soon as we can, and to adjust how we work to avoid them, find them sooner, fix them faster.
So what have we learned here? We took some big bites, and probably selected a poor place to start a time or two. In fact, it might have been “better” to have moved all the Main functions to the U table, one at a time. Then we might have left it there, or we might have promoted U to a class. In retrospect, I think that might have gone more smoothly. Great, keep that in mind for next time. I’m not allowed to go back on my own time line, and I assure you that if I were, I’d be buying Microsoft and Apple stock and selling them on the right days, not fiddling around with that code. But we’ll keep it in mind for the future.
I note that I often take bigger bites, and then I’m the one who gets bitten. That’s a lesson I need to learn and re-learn. I wonder if there’s something I could do to help. When working with a pair, it’s easier to keep big bites in check, because the pair generally recognizes trouble before the coder. What can I do by myself? Hmm. I wonder if setting a timer to ding every ten minutes or something, and if I’m not done with something good, at least whap myself upside the head and ask what’s got me in trouble this time. Might be fun to try.
And the tests. That little Automate button helped: it speeds up my manual test by a factor of at least ten. It’s not great, but it’s a foothold for something even better. And maybe I should automatically run the CodeaUnit tests, and make their breaking into a bigger deal. I’d maybe have to enhance the framework a bit but what’s the harm in that?
Oh, that run it and fix it pattern that I did for a while? It worked better in Smalltalk than it did in Codea. I think in Codea, the thing is to use the search / replace to look for code that needs to be modified. We used to do that, too, in Smalltalk. You could ask for senders of a method, get one or more editors open on all of them, modify them, put them away, very smoothly. This isn’t the same as that but it’s still pretty smooth. So let’s remember to look for references to changed code, not just wait for the system to explode. That will almost certainly result in a defect that doesn’t get found until serious testing, or until release. That would be bad.
And again, the tests. Must bear down on tests.
Bottom line, though, in a few sessions of less than a full day’s work, we’ve moved our design to a much better place, and we mostly managed to do it without breaking anything.
I’m pleased with where we are, and not too bruised from getting there. Time for lunch! See you next time!
-
Someone said “Don’t should all over yourself”, and they were right. Nonetheless, it’s good to look back and learn. ↩