Spacewar! 35 - Reflection
This has been fun. Our little program is in good shape and has been advancing more or less smoothly. There have been no really deep bugs, though a few things have been confusing. I’ve been working “against type” by having fewer tests than my rantings considered teachings would suggest. On the other hand, I’ve been trying throughout to do the simplest thing that could possibly work, because I like to discover and display what happens when you do that. Usually, nothing bad happens. Naturally you discover that you need more structure, more design. But you also discover that you learn rapidly, rarely write anything you don’t really need, and survive quite nicely.
Nonetheless, things have not gone as smoothly as they might. I want to think about what we have to learn and how we might change our direction.
Collisions
The collision logic has been at the center of a lot of our troubles. Originally, I did the simplest thing I could think of. I wrote a loop over all objects, containing a loop over all objects. I checked to see if the two were colliding, and if they were, I told them both to die.
The question of whether they were colliding was a bit tricky. An object could not collide with itself. Some objects couldn’t collide at all, like the Buttons. I decided that when an object died, it would just nil out its entry in the object table, so nil objects had to be treated as not colliding. It resulted in code like this:
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
It seemed reasonable at the time. But a lot of trouble resulted, including:
- Setting dead objects to nil in the tables seemed to cause problems in the looping
- I went down a messy path trying to ensure that each pair of objects was considered only once
- I tried two or three different ways of representing the collection of objects and ways of updating it
- All the drama of object death, such as explosions, happened twice, once as missile hit ship, one as ship hit missile
The resolution, at the moment, is that objects register to be removed but are not removed during the collision logic, and as we go through the loop we only message the guy in the outer loop, trusting that the guy he’s colliding with will turn up again when he gets his turn in the outer loop:
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
So each object will see the message hitBy
only once, not twice. It’s mostly better. But why did we have this much trouble? Where did my thinking go wrong? More important, what can I do about that kind of problem? I can’t work smarter: I’m working as smart as I can. I can, however, adjust my habits a bit. What are some possibilities?
More design?
I could do more (conventional) design than I’ve been doing. Draw more diagrams, work out more details before coding, stuff like that. In a word, no. Oh, it might be worth trying, just to show how wrong it goes, but an essential aspect of my way of working is to limit separate up-front design as much as I can. Besides, it seems to me that it was the “thinking” that has gone wrong here, leading to misunderstandings of what was going on.
More tests?
Well, that’s easy to say. If the code doesn’t do what we thought it did, and we don’t know it right away, then we’re rather clearly running low on tests. In my case, I mean automated tests, checks as Michael Bolton would have it. The manual tests, exploratory tests are already problematical and the game is quite simple now. I even built that little button that does the fire, turn, fire thing, with missiles traveling much faster, just to make manual testing easier. Still, it’s almost impossible to duplicate something from test to test, and when we get gravity it will only get worse.
And yet, I’m not sure how we might have tested this collision stuff better. The colliding
function, with all its nil checks and such, has always worked and I don’t see that testing it would have helped. On the other hand, colliding
now has all kinds of nil checks and such in it that are clear indications that weird things have been going on.
What about the interactAll
loop, in its various incarnations? There, we might have done better. Maybe we could have tested:
- Does setting a key-value pair to nil inside a loop ensure that the item won’t be produced?
- Does the item get produced but sent through as nil next time?
- Can we reliably consider each pair only once? (We did do some testing for this.)
- The loop is O(N^2). Is that a problem?
It’s hard to justify the idea that testing would have made this better. Nonetheless, I know I’m using fewer tests than I normally would, so I am inclined to bear down a bit on producing more automated tests.
A bit less hackery?
That trick of taking both elements of a colliding pair and telling them both to die caused a lot of trouble. It produced duplicate explosions and other confusing events. Maybe I was too hungry to see something happen, and hammered in a solution thinking I’d clean it up later. This isn’t quite the same as “be smarter”. This is “don’t be so dumb when you know you’re being dumb”. Maybe I can do that, but I have a long history of doing dumb things.1 Still, I’ll try to notice when I’m feeling like rushing into something.
Express more intention?
Code like the interactAll
above doesn’t express intention very well. Its earlier iterations were even worse. When I’ve worked more explicitly to express intention here, things have gone pretty well. So let’s try to write by intention more often, and let’s look at code and ensure that it moves in a more expressive direction.
Cohesion (and coupling)?
That interactAll
still lacks a bit of cohesion, I think. It owns the decision of when objects interact, and what happens when they do. Perhaps all it should do is cause every pair of objects to interact
and leave it up to them to decide what to do about it. We still will have the issue of A interacting with B and then B interacting with A, but at least it would give the objects a hand in the decision. So let’s try to keep the code a bit more clean.
I am actually surprised …
I wasn’t writing this article to make a point, just to have some learning. And yet, it seems to come down to answers that are very common in my thinking: clean, well-tested code. No deep insights here: I didn’t expect any. But the learnings do seem to fall very nicely into the common buckets of testing, intention, and refactoring.
So I’ll try to do that. More testing, cleaner code via intention and refactoring.
See you tomorrow!
-
One time, I bought a used Lamborghini. But that’s another story. ↩