Spacewar! 34 - Fixing the double collisions.
A clean death
Presently, in our collision logic, two colliding objects are each sent a die
message. Furthermore, they are sent that message twice, because the collision loop processes everyone against everyone.
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
Our mission, should we decide to accept it, is to deal with the duplicate calls, and to give the colliding object a chance not to die. For example, a ship might die immediately upon falling into the sun (if there were one), but the sun would not die. Contrariwise, a missile might die upon hitting a ship but the ship might just accrue some damage, depending on its armor class and perhaps a saving throw.
We consider, briefly, looping i from 1 to N and j from i to N, which might solve the two-calls problem. We reject this idea because we’d be going further into the pairs
abstraction than we’d like.
Whatever message we send (die probably isn’t it), the target object needs to know what hit it. So it’ll need to be more like a:die(b)
. That would allow a to ask how much damage b can do to it. (Note that we are mixing two notions in our discussion here, the single-call issue and the damage issue. Deal with it: we are.)
One first step, Tozier suggests, is to change the die name to something closer to what we mean. I paused meaningfully right here because I’m thinking of the changes we’ll have to make. Fortunately, we have a search/replace function. Tozier says something that makes me think he means that we need more tests. Maybe we do.
Let’s change the name. a:collide(b)
? Here goes:
function Universe:interactAll()
for _, a in pairs(U.Drawn) do
for _, b in pairs(U.Drawn) do
if U:colliding(a,b) then
a:collide(b)
b:collide(a)
end
end
end
end
But let’s think about that. Should the Universe inform both parties that they have collided, as shown here? Or should it just inform the first of whichever pair it finds, leaving the rest up to the object? Or should it consider in some magical way which should be primary and which secondary?
Tozier asks whether, if we just did first:collide(second)
, and not the other, wouldn’t the other turn up later in the loop and would that solve our duplication problem? We decide to back up and try it in the die mode:
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
Brilliant! That seems to work with the game as now configured: the ship explodes only once. The missile dies. Observe:
MOVIE HERE
So that’s good. Why didn’t I see that? I was thinking of the double loop as generating the pairs, and in an earlier version they were being removed from the loop as the loop ran and well my brain just went into the ditch. This happens. (Please assume, if you wish, that I am faking stupidity for sake of the narrative. That would be kind.)
Tozier raises the issue that this algorithm is O(n^2) and that will be terribly inefficient and the sky may literally fail to fall owing to lag. All true. We don’t know a better way and anyway until a performance measurement shows we have the problem we’ll ignore it.
So now we can change our message name from die
to something better, and pass in the other guy so we can assess damage. We were using the message collide:
for a moment. I am offering hitBy
. We think we’ll go with that.
function Universe:interactAll()
for _, a in pairs(U.Drawn) do
for _, b in pairs(U.Drawn) do
if U:colliding(a,b) then
a:hitBy(b)
end
end
end
end
Of course, this won’t work because no one understands hitBy. We’ll rename the die
functions to hitBy
and give them a parameter, which we’ll ignore first time thru. The idea is to do a quick refactoring and leave the system running. Well, almost:
function Fragment:draw()
self.timeOut = self.timeOut - 1
if self.timeOut < 0 then self:die() end
pushMatrix()
pushStyle()
translate(self.pos:unpack())
rotate(self.heading)
self:drawFragment(self.type)
popStyle()
popMatrix()
end
...
function Fragment:die()
U:kill(self)
end
Objects use die
to kill themselves. So we need to add the function hitBy, not rename it:
function Fragment:hitBy(anObject)
self:die()
end
And so on. For now, we’re just wiring everything to go through hitBy
. Then we’ll add smarter behavior. Tozier points out that since Fragments cannot collide, the current collision logic will not call this function. That’s interesting. It seems to me that the official interface of objects in space includes hitBy
. We have a violation of the object’s protocol here: the Universe shouldn’t be making that decision. Write that on a napkin, will you, and remind me later.
Continuing …
function Missile:hitBy(anObject)
self:die()
end
function Ship:hitBy(anObject)
self:die()
end
What about Explosion? Explosion is odd. Originally we had a bit of scaffolding there, that said “Blammo!” for a while, so we could see that the logic was working. Now, however, Explosions don’t really need to exist at all. Here’s the code:
-- HAVE YOU PUSHED TO GITHUB TODAY?
Explosion = class()
function Explosion:init(ship)
for i = 1, 5 do
Fragment(i, ship)
end
U:addObject(self)
self.name = "blammo"
self.timeOut = 100
self.pos = ship.pos
self.cannotCollide = true
end
function Explosion:draw()
self.timeOut = self.timeOut - 1
if self.timeOut <= 0 then self:die() return end
pushMatrix()
pushStyle()
translate(self.pos:unpack())
text("Blammo!")
popStyle()
popMatrix()
end
function Explosion:draw()
end
function Explosion:die()
U:kill(self)
end
function Explosion:move()
end
Note that it has an empty draw
function. All it does is create the Fragments, set up a bunch of useless variables, then time out and die. We should remove Explosion class and move exploding into Ship. Is this the time to do that? Inspecting Explosion tells us that we can “just” move the Fragment loop to Ship, like this:
function Ship:die()
self:explode()
U:kill(self)
end
function Ship:explode()
for i = 1, 5 do
Fragment(i, self)
end
end
Tozier leaned toward writing the loop explicitly into die
. I didn’t, instead creating a function. Why did I do that, class?
“Mr Jeffries, was it about that thing you call Composed Function?”
Yes, Timmy, that’s right. Also that thing I call Intention. My intention was to create five fragments. The code to do that is of course, the for loop. But if I put that code into the die
function, that function includes a call and some explicit code. The Composed Function pattern suggests that functions should either contain explicit code or calls, but not both. This is more of a guideline than an actual rule.
Tozier desires to run the system without deleting the Explosion tab. Belt and suspenders? I don’t know but why not.
The system runs. Now he wants to read Explosion’s code. I suggest looking for references to it. We find none. We’ll remove the tab. (It’s all in GitHub anyway …) However, thanks T, we’ll commit and then delete the tab.
As we test after deleting the tab, we agree that we don’t like that the fragments all go the same speed and all disappear at the same time. Frill? Nicety? Who cares, we’re here to please ourselves.
By the way, the tests all still run. Woot! However, some of them may be calling die
when they should be calling hitBy
. Let’s look. In fact they are not: they are setting up some objects and then calling U:interact
, so that’s going through the right code. I love it when a plan comes together.
Had we found that we were calling die
, we’d have had to consider whether our tests were too tightly coupled to the code. Since the tests are staying stable, we can feel pretty good.
We did notice that there is a test, TestPairs
, that does some obscure thing about processing each pair of objects only once. We could delete that because we’re just accepting the N-squared. Or we could keep it because it has a “solution” to the pairs problem. I guess we’ll keep it.
Our options are: deal with damage other than fatal, or improve the look of the explosions. Let’s take the easy way out and improve the look of the explosions: it’s 10:44 and we must stop soon.
Fragment = class()
Fragment.types = {}
Fragment.types[1] = function()
line(0,7,-3,-8)
line(-3,-8, 1, -4)
line(1,-4, 0,7)
end
Fragment.types[2] = function()
line(-2,2, 3,6)
line(3,6, 3,-2)
line(3,-2, -4,-4)
line(-4,-4, -2,2)
end
Fragment.types[3] = function()
line(-4,1, 3,3)
line(3,3, 3,1)
line(3,1, -1,-3)
line(-1,-3, -5,-3)
line(-5,-3, -4,1)
end
Fragment.types[4] = function()
line(-5,2, -1,2)
line(-1,2, 3,6)
line(3,6, 6,-2)
line(6,-2, -6,-2)
line(-6,-2, -5,2)
end
function Fragment:init(fragmentNumber, ship)
self.type = fragmentNumber
self.cannotCollide = true
self.pos = ship.pos
local outwardVelocity = vec2(0,1):rotate(math.random()*math.pi*2)
self.vel = ship.vel + outwardVelocity
self.angularVelocity = math.random()*10-5
self.heading = 0
self.timeOut = 360
U:addObject(self)
end
function Fragment:move()
self.pos = U:clip_to_screen(self.pos + self.vel)
self.heading = self.heading + self.angularVelocity
end
function Fragment:draw()
self.timeOut = self.timeOut - 1
if self.timeOut < 0 then self:die() end
pushMatrix()
pushStyle()
translate(self.pos:unpack())
rotate(self.heading)
self:drawFragment(self.type)
popStyle()
popMatrix()
end
function Fragment:drawFragment(type)
strokeWidth(2)
if self.types[type] then self.types[type]() else
pushStyle()
fill(255)
text("?",0,0)
popStyle()
end
end
function Fragment:hitBy(anObject)
self:die()
end
function Fragment:die()
U:kill(self)
end
The relevant lines are:
local outwardVelocity = vec2(0,1):rotate(math.random()*math.pi*2)
Tozier suggests changing the 1 in vec2(0,1)
to a random number of some kind. I think we’ll try a velocity range of 0.5, centered around 1. We try that but want a bit more variation, so we get:
local speed = 1 + math.random()
local outwardVelocity = vec2(0,speed):rotate(math.random()*math.pi*2)
MOVIE HERE
Looks better. They still die at the same time. Tozier has a plan:
self.timeOut = math.random(200,300)
MOVIE HERE
Sweet. Let’s commit. Yay, worked!
We’ve not done damage yet. And you may have noticed the fifth Fragment renders as a question mark. That’s because I have a special fragment in mind. Stay tuned.
What else do we see? We improved the design a bit, I think, by removing the redundant Explosion class. We are set up to allow damage, by calling hitBy
rather than die
, which gives an object the chance to ask the object that hit it what damage it does. We have a place to stand, but haven’t stood there yet.
We have Fragments set to cannotCollide
, which is tested in the Universe:
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
(Explosion also used this flag.) My concern with the flag is that it exports too much decision logic to Universe. I’d rather see the objects deciding for themselves what to do. One reason we put that in was that when fragments were created near the ship, they tended to run into each other and disappear. That doesn’t look good. On the other hand, we think that it might be fun if you could get killed by a fragment from the guy you killed. So we might want to play with that. In any case, I think exporting that state is a design flaw. Law of Demeter or something.
Tozier suggests that Fragments could be hard to kill, so they wouldn’t kill each other right off. I’m thinking they might ignore each other. Anyway, that’s for another day. We remain a little concerned about the N-squared aspect of collision. That, too, is for another day.
Good session! See you next time!