P-225 - Still Shooting
Python Asteroids+Invaders on GitHub
I had an interesting realization about the shot patterns. And we have a hanging commit, some broken tests, and more to do before we are done.Things go well. Thoughts on small steps.
I didn’t mention the following: I just “fixed” it. I noticed that in the original source, the column tables seemed to be incomplete. One of the invader shots had a cycle of 16 columns, and the other shows only 15. I assumed that to be a mistake and extended the table for the second one.
I realized last night that if shots alternate, and the shot patterns are of equal length, the whole cycle will repeat. After the squiggle shoots from column 7, it’s likely that you would know where the plunger will next shoot from. That’s the sort of thing that expert players pick up on. If we make the lists 16 and 15 in length, the cycle becomes 225 steps long and no one is likely to have that one memorized. So let’s fix that.
But first, I’d like to get my tests running and make the commit that is hanging since yesterday afternoon.
The first test is failing with this message:
> invader = self.invader_fleet.invader_group.bottom_of_column(col)
E AttributeError: 'NoneType' object has no attribute 'invader_group'
The test is this:
def test_fires_three_different_shots(self):
fleets = Fleets()
fi = FI(fleets)
controller = ShotController()
for _ in range(3):
controller.fire_next_shot(fleets)
shots = fi.invader_shots
assert len(shots) == 3
s1, s2, s3 = shots
assert s1.map != s2.map and s2.map != s3.map and s3.map != s1.map
assert s1.position != ShotController.available
assert s2.position != ShotController.available
assert s2.position != ShotController.available
controller.fire_next_shot(fleets)
assert len(fi.invader_shots) == 3
s1.die(fleets)
s2.die(fleets)
s3.die(fleets)
assert len(fi.invader_shots) == 0
for _ in range(3):
controller.fire_next_shot(fleets)
assert len(fi.invader_shots) == 3
All this wants to do is to fire a few shots and make sure they are of different types, and that after they die we can fire them again.
(I needed an “and” to describe this test. Solid clue that it is too big. Not our current issue but bears thinking about. I tend to write tests that are too long.)
The issue is that the controller wants to use an InvaderFleet instance to access the InvaderGroup instance (Law of Demeter violation), to select a column from which to fire. In this test we do not have an InvaderFleet. Let’s see whether we can just inject one and move on.
def test_fires_three_different_shots(self):
fleets = Fleets()
fi = FI(fleets)
controller = ShotController()
invader_fleet = InvaderFleet()
controller.invader_fleet = invader_fleet
...
Yes. Test runs. I think the other failing test may have the same problem. No, it is easier. It is a test for the plunger columns and is not taking into account that the numbers all changed because I realized they were one-based instead of zero-based, which I need.
I change the test to look up the actual tables and make sure that we’re cycling through them:
def test_plunger_columns(self):
controller = ShotController()
desired = controller.columns
assert controller.next_column_for(0) == desired[0][0]
assert controller.next_column_for(0) == desired[0][1]
assert controller.next_column_for(1) == desired[1][0]
assert controller.next_column_for(1) == desired[1][1]
for _ in range(14):
controller.next_column_for(1)
assert controller.next_column_for(1) == desired[1][0]
We are green. I can commit: squiggle and rolling shots follow column tables for firing pattern.
Now let’s fix the pattern bug by shortening the second table and making the code work. This will break the test above, which is checking wrap-around.
class ShotController(InvadersFlyer):
def __init__(self):
self.time_since_firing = 0
self.shots = [
InvaderShot(self.available, BitmapMaker.instance().squiggles),
InvaderShot(self.available, BitmapMaker.instance().rollers),
InvaderShot(self.available, BitmapMaker.instance().plungers)]
self.columns = [
[0x00, 0x06, 0x00, 0x00, 0x00, 0x03, 0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07],
[0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07, 0x01, 0x0A, 0x03, 0x06, 0x09]]
self.current_columns = [0, 0]
self.shot_index = 0
self.invader_fleet = None
Test immediately breaks, probably off the end of the table. I want to know where that happens and the test will tell me. I think I have an explicit mod 16 that needs to be changed.
The test says this:
def next_column_for(self, shot_index):
column_number = self.current_columns[shot_index]
self.current_columns[shot_index] = (self.current_columns[shot_index] + 1) % 16
> return self.columns[shot_index][column_number]
E IndexError: list index out of range
And there’s yer trouble right there.
def next_column_for(self, shot_index):
column_number = self.current_columns[shot_index]
self.current_columns[shot_index] = \
(self.current_columns[shot_index] + 1) \
% len(self.current_columns)
return self.columns[shot_index][column_number]
I am surprised, however. I expected that the test would continue to fail because it was going to wrap around sooner.
def test_plunger_columns(self):
controller = ShotController()
desired = controller.columns
assert controller.next_column_for(0) == desired[0][0]
assert controller.next_column_for(0) == desired[0][1]
assert controller.next_column_for(1) == desired[1][0]
assert controller.next_column_for(1) == desired[1][1]
for _ in range(14):
controller.next_column_for(1)
assert controller.next_column_for(1) == desired[1][0]
If there are only 15 elements in that list, I’d expect that last assertion to require desired[1][1]
.
I’m going to add some assertions to figure this out. This passes:
def test_plunger_columns(self):
controller = ShotController()
desired = controller.columns
assert len(desired[0]) == 16
assert len(desired[1]) == 15
assert controller.next_column_for(0) == desired[0][0]
assert controller.next_column_for(0) == desired[0][1]
assert controller.next_column_for(1) == desired[1][0]
assert controller.next_column_for(1) == desired[1][1]
for _ in range(14):
controller.next_column_for(1)
assert controller.next_column_for(1) == desired[1][0]
Let’s loop over the #1 guy and check all his values.
He still fails, which forces me to accept that the code I just pasted up above is wrong. It’s checking the wrong table. Should be:
def next_column_for(self, shot_index):
column_number = self.current_columns[shot_index]
self.current_columns[shot_index] = \
(self.current_columns[shot_index] + 1) \
% len(self.columns[shot_index])
return self.columns[shot_index][column_number]
The current_columns
is just a list of two values, the current column to access for whichever shot we’re doing. the columns
is the actual list of values.
Test is now green. Commit: fix shot column lists to be of different lengths per original. provides more random appearance.
Reflection
What has this experience just taught us? Well, sure, tests are good, good tests are better. But there’s more.
We have these members inside shot controller:
def __init__(self):
self.time_since_firing = 0
self.shots = [
InvaderShot(self.available, BitmapMaker.instance().squiggles),
InvaderShot(self.available, BitmapMaker.instance().rollers),
InvaderShot(self.available, BitmapMaker.instance().plungers)]
self.columns = [
[0x00, 0x06, 0x00, 0x00, 0x00, 0x03, 0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07],
[0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07, 0x01, 0x0A, 0x03, 0x06, 0x09]]
self.current_columns = [0, 0]
self.shot_index = 0
self.invader_fleet = None
The two members columns
and current_columns
are a matched pair of two-tuples. Well, tables, but I can fix that in an instant. “Two-tables” doesn’t have the same ring as “two-tuples”. It’s that vowel order thing that makes “pip pop” sound good and “pop pip” sound wrong somehow.
Thing is, current_columns[0]
indexes into columns[0]
and similarly for 1
. They are coupled. They belong together and need to be used together.
There is a tiny object trying to be born here. I think its name is something like CycleGenerator or Cycler or something. Its job is to give you the next number every time you ask it.
Let’s TDD that little thing and then use it. … pause …
I feel myself resisting putting the tests in a new test file and the class in a new file. I do think that Pythonistas do use files with more than one class in them, modules, I think they call them. It’s just a pain to have all these little files. But I’m going to do the right thing by the current coding standard chez Ron1, which is separate files.
class TestCycler:
def test_exists(self):
Cycler([])
class Cycler:
def __init__(self, values):
self._values = values
self._index = 0
As I often do, I went beyond my test. The strict rule of TDD is to write only the code required to pass the test. My personal rule is that if there is a bit more code in your fingers, it’s OK to blurt it out. The strict rule is probably better.
Moving right along …
def test_cycles(self):
values = [0, 100, 200]
cycler = Cycler(values)
for i in range(6):
expected = (i % len(values)) * 100
checked = cycler.next()
assert checked == expected
If that works, I think the class works. Since the class is so trivial, I could even be right.
class Cycler:
def __init__(self, values):
self._values = values
self._index = 0
def next(self):
next = self._values[self._index]
self._index = (self._index + 1) % len(self._values)
return next
Test runs. Green. Commit: New Cycler object returns values from a list over and over.
Now let’s use it in ShotController.
class ShotController(InvadersFlyer):
def __init__(self):
self.time_since_firing = 0
self.shots = [
InvaderShot(self.available, BitmapMaker.instance().squiggles),
InvaderShot(self.available, BitmapMaker.instance().rollers),
InvaderShot(self.available, BitmapMaker.instance().plungers)]
self.columns = [
Cycler([0x00, 0x06, 0x00, 0x00, 0x00, 0x03, 0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07]),
Cycler([0x0A, 0x00, 0x05, 0x02, 0x00, 0x00, 0x0A, 0x08, 0x01, 0x07, 0x01, 0x0A, 0x03, 0x06, 0x09])]
self.shot_index = 0
self.invader_fleet = None
def next_column_for(self, shot_index):
return self.columns[shot_index].next()
One test is failing, surely the one that is looking up the values and checking them.
def test_plunger_columns(self):
controller = ShotController()
desired = controller.columns
assert len(desired[0]) == 16
assert len(desired[1]) == 15
assert controller.next_column_for(0) == desired[0][0]
assert controller.next_column_for(0) == desired[0][1]
for i in range(22):
col = controller.next_column_for(1)
index = i % len(desired[1])
assert col == desired[1][index]
The question in my mind is whether we still need this test. I think we do. We know that Cycler works, but we don’t know that ShotController uses it, and uses it correctly. So let’s recast this test.
I’ll just refer to one of the shots, it’s only supposed to be checking plunger anyway.
def test_plunger_columns(self):
controller = ShotController()
desired = controller.columns[1]._values
for i in range(22):
col = controller.next_column_for(1)
index = i % len(desired)
assert col == desired[index]
Invasive but accurate. I’ll allow it. Commit: ShotController uses Cycler to cycle columns.
Reflection
I feel that that tiny Cycler object has made ShotController much more clear, by removing that bizarre method:
def next_column_for(self, shot_index):
column_number = self.current_columns[shot_index]
self.current_columns[shot_index] = (self.current_columns[shot_index] + 1) % len(self.columns[shot_index])
return self.columns[shot_index][column_number]
It’s replaced by this:
def next_column_for(self, shot_index):
return self.columns[shot_index].next()
And if you care to know how Cycler works, it’s trivially short.
I often find that little objects like this can be quite helpful. One thing that I do not like is that Cycler is now visible at the top level of the program, and while I trust everyone to use it properly, it’s just one more thing to scroll past, so it adds a bit to the clutter.
Fortunately, in PyCharm, I rarely look at the big list of files. I always use the Shift-Shift search and type a few letters of the class I’m looking for. So the clutter doesn’t bother me much. I should research approved ways of organizing larger Python apps, in my copious free time.
Let’s review that checklist I made a while back. I’ll strike out the things that are done.
-
Alternate among the three types of shots; -
No more than three shots alive; -
Each type can occur only once on the screen at the same time; -
Make shots emanate from specific columns; -
Make them emanate from the bottom of the column; - Improve the animation of the player when hit.
- Pick ideal column for targeted rolling shot.
Nice progress. Let’s pick off the player animation, it might be easy and it’ll be good for a change.
Here’s what we do now:
class InvaderPlayer(InvadersFlyer):
def interact_with_invadershot(self, shot, fleets):
collider = Collider(self, shot)
if collider.colliding():
self.explode_time = 1
def draw(self, screen):
self.explode_time -= 1/60
if self.explode_time > 0:
player = self.players[1]
else:
player = self.player
screen.blit(player, self.rect)
So currently we display the players[1]
image for a second after being hit. We should really alternate. Let’s first try something simple:
def draw(self, screen):
self.explode_time -= 1/60
if self.explode_time > 0:
player = self.players[random.randint(1,2)]
else:
player = self.player
screen.blit(player, self.rect)
That actually looks quite good:
When you win, quit, that’s my motto. Alternating won’t look as random as that does. I think we’ll let it ride for now. We’re going to have an issue with removing the player when we get there, but we’re not there now. Where we are is here:
-
Alternate among the three types of shots; -
No more than three shots alive; -
Each type can occur only once on the screen at the same time; -
Make shots emanate from specific columns; -
Make them emanate from the bottom of the column; -
Improve the animation of the player when hit. - Pick ideal column for targeted rolling shot.
And this is plenty of progress for a Sunday. I have to go read some more in Venomous Lumpsucker. Really. Clarke award winner. Quite odd, somewhat funny, a bit dark, eco-future thing.
Summary
I left red tests yesterday afternoon, because I had been distracted and was tired. I just wasn’t sharp enough to make good decisions, but I was sharp enough to know it. The tests were easy enough to fix, which let me get the code committed.
Then we fixed my mistaken understanding of why the two column lists were of different length.
That led to the observation that the code for managing those lists was confusing: remember, I made the change incorrectly the first time. And that reminded me that I had been thinking that a tiny object could help out.
We TDD’d Cycler, almost trivially, and applied it quite easily. And ShotController is a bit simpler, and that’s a good thing.
Then a quick change to the player to make its explosion wiggle, and we’re done.
The thing is to notice the small steps, not so much planned as just taken in a roughly right direction. They move us to better and better code, to more and more capable features, yet they are opportunistic, taking advantage of the things we find in the code. I could not plan at the level of detail that these changes represent: I can’t hold that much detail in my head. Instead, I keep myself aware of the bigger picture, and pick my way along a path from place to place in the code, always trying to move from a good place to a better place, and in the general direction of what we want.
I want to quote what I said the other day:
We take the path we take. We know that the path wanders. Sometimes it’s a really good path, sometimes it’s so bad we double back2. We get somewhere better. We are at peace.
This is the way.
Taking small steps is hard for me. I have over six decades of programming experience, most of them taking very big steps. I believe that on the average, these days, my steps average at least twice as large as they need to be, and often much larger.
And the smaller I make them, the better things go. I’ll keep trying to make them smaller. If I ever do one that is too small, I’ll let you know.3 4 5
Today, good results and I am at peace. See you next time!
-
If I weren’t closing my company, I should probably rename it to “Chez Ron”. Or not. ↩
-
Not necessarily by reversion. – Hill, private communication. ↩
-
One thing that causes me to take larger steps is that sometimes I’m not sure that I won’t want to roll back a decision, and since I push every commit to GitHub, reverting is a pain (i.e. I don’t really know how to do it). That’s not a good reason to take a larger step. Fail again. Fail Better. ↩
-
Another thing leading to larger steps is that often I have more code in mind than the test requires. If I am aware of what the members will be, I tend to create the
__init__
before the test that really needs it. It’s not like I’ll forget, will I? What were we talking about? ↩ -
My tests tend to be too long. This might force larger steps, if a test checks a lot of things. And even if I build it up incrementally, I tend not to commit after each stage works. Log tests are also an indication that something is hard to test and thus perhaps not of an idea design. I’ll try to do better, but no promises. ↩