Python 205 - Moving Invaders II
Python Asteroids+Invaders on GitHub
Let’s continue with moving the invaders. We have some issues to deal with, and for at least one, I have no solution in mind. Lots of thinking tells me there’s something funky in here.
What were those items from last time? Oh, right:
- Write test for incremental motion
- Write test for incremental reversal
- Write test for stepping down.
- Fix reversal to work correctly for incremental motion. OK for now.
- Adjust InvaderFleet origin downward on reversal.
When we moved from moving all the invaders at once to moving them one at a time, reversing direction went wonky, though it looks OK on the screen. The thing is that whenever any invader hits the Bumper, she sends the at_edge message back to the InvaderFleet. When that message has been sent, the Fleet reverses the step, in the end_interaction event, changing it from positive to negative or vice versa. That’s too soon: the cycle isn’t done yet.
Before, when they all moved at once, all the ones in the edge column would cross the Bumper in the same move. Each would send at_edge, but no harm done, it would only happen once. We’d reverse direction and the next move would move them all out of the Bumper. But now, they move in one at a time. Let’s see if I can explain what happens: that seems critical to me before trying to fix it.
- We run interactions and no invader is in the bumper;
- We move one invader and she is not in the bumper;
- We run interactions again, move invaders again, no bumper;
- Finally we move some invader into the bumper;
- We run interactions and receive
at_edgefrom her; - We reverse the step, but we only use it when all invaders have moved;
- This repeats. A few invaders cross the bumper, we reverse the step each time;
- If by the time we move all invaders, an odd number have hit the bumper, we’ll reverse, but if an even number have hit it, we won’t reverse because two reverses cancel out.
Right. That’s it.
So what if we didn’t change the reverse flag in end_interactions, but only after all the invaders have moved? Presently we have this:
class InvaderFleet(Flyer):
def at_edge(self):
self.reverse = True
def end_interactions(self, fleets):
if self.reverse:
self.reverse = False
self.step = -self.step
def update(self, delta_time, _fleets):
if self.next_invader >= len(self.invaders):
self.origin += self.step*delta_time
self.next_invader = 0
self.invaders[self.next_invader].move_relative(self.origin)
self.next_invader += 1
Instead, what if we do this?
def end_interactions(self, fleets):
pass
def update(self, delta_time, _fleets):
if self.next_invader >= len(self.invaders):
self.origin += self.step*delta_time
self.next_invader = 0
if self.reverse:
self.reverse = False
self.step = -self.step + Vector2(0, 8*48)
self.invaders[self.next_invader].move_relative(self.origin)
self.next_invader += 1
Well, by actual test, that doesn’t work. They get to the edge and then reverse, stepping down, but immediately reverse again, stepping down again. Why? Because, since they don’t all move out at once, there’s always at least one invader left in the bumper and she calls for another reverse.
Meh. I’d really better write the tests. Roll back.
How can we test this? We really need something better than running a million cycles to move the invaders all around.
What do we want to know? “Has the fleet hit the edge?” And, I think Bruce said something about this, how about “and are they moving toward it?” Only then should they revert.
I think I can code this better than test it.1 That’s troubling: when I can’t think of a test, there’s something wrong with the code, my head, or both. Well, let’s at least think about how it might be coded and see if that suggests a test.
The invaders don’t know which way they’re going. But suppose the Bumpers held a value +1 or -1 depending on which side of the screen they’re on. And suppose that when an invader hits a bumper, she sends at_edge(Bumper.side) or something like that, and the InvaderFleet only flags reverse if the sign of the side matches the sign of the direction we’re currently moving.
I still don’t see the test. Let me spike something. Maybe that will get me to a test.
First I want to have a simple direction flag, instead of negating step. I think that will make it easier to compare direction with the bumper.
class InvaderFleet(Flyer):
def __init__(self):
self.invaders = [Invader(x%11, x//11) for x in range(55)]
self.origin = Vector2(u.SCREEN_SIZE / 2 - 5*64, 512)
self.step = Vector2(30, 0)*8
self.reverse = False
self.next_invader = len(self.invaders)
self.direction = 1
# self.update(0, None)
for invader in self.invaders:
invader.move_relative(self.origin)
def end_interactions(self, fleets):
if self.reverse:
self.reverse = False
self.direction = -self.direction
def update(self, delta_time, _fleets):
if self.next_invader >= len(self.invaders):
self.origin += self.direction*self.step*delta_time
self.next_invader = 0
self.invaders[self.next_invader].move_relative(self.origin)
self.next_invader += 1
I think that should leave me where I was. Not quite, a test fails. I fix it to refer to direction:
def test_fleet_motion(self):
fleet = InvaderFleet()
fleet.step = Vector2(30, 0)
pos = fleet.invaders[0].position
fleet.update(1.0, None)
new_pos = fleet.invaders[0].position
assert new_pos - pos == fleet.step
fleet.at_edge()
fleet.end_interactions(None)
assert fleet.direction == -1
Green. So far so good. Now let’s big the bumpers a value. Let’s see. We want to report at_edge if the fleet direction is +1 at the right. Let’s give the bumpers incoming_direction:
class Bumper(Flyer):
def __init__(self, x, incoming_direction):
self.rect = Rect(x, 0, x+1, u.SCREEN_SIZE)
self.incoming_direction = incoming_direction
coin.py
def invaders(fleets):
fleets.clear()
fleets.append(Bumper(16, -1))
fleets.append(Bumper(u.SCREEN_SIZE - 16, +1))
fleets.append(InvaderFleet())
Something is failing. Ah, a bumper test, needs the new parameter.
def test_bumper_invader_collision(self):
fleet = InvaderFleet()
bumper_x = 16
bumper = Bumper(bumper_x, -1)
invader_column = 5
invader = Invader(invader_column, 2)
start = Vector2(64, 512)
step = 64
invader.move_relative(fleet.origin)
invader.interact_with_bumper(bumper, fleet)
assert not fleet.reverse
start_x = bumper_x - invader_column*step
start = Vector2(start_x, 512)
invader.move_relative(start)
invader.interact_with_bumper(bumper, fleet)
assert fleet.reverse
This might actually serve as a decent test for the change, at least part of it.
OK, now to use the new bumper notion:
class Invader:
def interact_with_bumper(self, bumper, invader_fleet):
if bumper.rect.colliderect(self.rect):
invader_fleet.at_edge(bumper.incoming_direction)
Here we pass in the incoming direction and here …
class InvaderFleet(Flyer):
def at_edge(self, direction):
self.reverse = direction == self.direction
We only reverse if the input direction is our direction, that is, we are incoming, not outgoing. Should I rename that parameter? I think so.
def at_edge(self, bumper_incoming_direction):
self.reverse = bumper_incoming_direction == self.direction
The good news is that this works. The bad news is that it has broken two tests. I was drawn away into a long teaching session right here, and now I am dead tired.
I’ll take a break and pick this up maybe tomorrow, with some red tests. Or I might revert and take my learning with me. I’ll decide tomorrow.
The tests turned out to be trivial, needed bumper values set correctly. We are green, but I’m still not going to commit. See you tomorrow or Monday.
Sunday 0615 Hours
OK, I need to figure out where I am. It has been almost 14 hours since I knew that.
The invaders detect bumpers and send at_edge, passing the bumper incoming_direction to the invader fleet:
class Invader:
def interact_with_bumper(self, bumper, invader_fleet):
if bumper.rect.colliderect(self.rect):
invader_fleet.at_edge(bumper.incoming_direction)
The invader fleet records whether a reverse is needed by checking for the incoming direction equaling the fleet direction:
class InvaderFleet(Flyer):
def at_edge(self, bumper_incoming_direction):
self.reverse = bumper_incoming_direction == self.direction
Note that on the way in, when invaders find themselves on the bumper, each time they call in, we’ll set reverse to true. Presently, we set reverse in end_interactions:
class InvaderFleet(Flyer):
def end_interactions(self, fleets):
if self.reverse:
self.reverse = False
self.direction = -self.direction
We apply self.direction only when we get to the end of the invader-moving cycle:
def update(self, delta_time, _fleets):
if self.next_invader >= len(self.invaders):
self.origin += self.direction*self.step*delta_time
self.next_invader = 0
self.invaders[self.next_invader].move_relative(self.origin)
self.next_invader += 1
So, no matter how many invaders report hitting the bumper, we’ll only update direction once.
I think this means that I can apply the downward step at this point as well. Let me try it.
No. I am mistaken. If I put a down-step on the setting of self.origin, we’ll step down on every cycle. We only want to step down if we are reversing. What if we don’t clear the reverse flag in end_interactions and instead check it in update?
def end_interactions(self, fleets):
pass
def update(self, delta_time, _fleets):
if self.next_invader >= len(self.invaders):
if self.reverse:
self.reverse = False
self.direction = -self.direction
self.origin += self.direction*self.step*delta_time + Vector2(0, 32)
else:
self.origin += self.direction*self.step*delta_time
self.next_invader = 0
self.invaders[self.next_invader].move_relative(self.origin)
self.next_invader += 1
Messy but seems to work nicely. Let’s clean this up a bit with an extract or even more than one:
def update(self, delta_time, _fleets):
self.check_end_cycle(delta_time)
self.invaders[self.next_invader].move_relative(self.origin)
self.next_invader += 1
def check_end_cycle(self, delta_time):
if self.next_invader >= len(self.invaders):
self.reverse_or_continue(delta_time)
def reverse_or_continue(self, delta_time):
if self.reverse:
self.reverse = False
self.direction = -self.direction
self.origin += self.direction * self.step * delta_time + Vector2(0, 32)
else:
self.origin += self.direction * self.step * delta_time
self.next_invader = 0
I like those two new names, check_end_cycle and reverse_or_continue. The reverse or continue method could perhaps be simpler. Let me fiddle with that a bit. But first, there’s an unhappy test.
def test_fleet_motion(self):
fleet = InvaderFleet()
fleet.step = Vector2(30, 0)
pos = fleet.invaders[0].position
fleet.update(1.0, None)
new_pos = fleet.invaders[0].position
assert new_pos - pos == fleet.step
fleet.at_edge(+1)
fleet.end_interactions(None)
assert fleet.direction == -1
Ah, yes. The change of direction isn’t in end_interactions any more. Good catch, test. The repaired test doesn’t please me:
def test_fleet_motion(self):
fleet = InvaderFleet()
fleet.step = Vector2(30, 0)
pos = fleet.invaders[0].position
fleet.update(1.0, None)
new_pos = fleet.invaders[0].position
assert fleet.direction == +1
assert new_pos - pos == fleet.step
fleet.at_edge(+1)
assert fleet.reverse
fleet.next_invader = len(fleet.invaders)
fleet.update(1.0, None)
assert fleet.direction == -1
It runs, but look at all the messing about that I had to do. Is it sufficient to call reverse_or_continue? This test can pass with just that. But then where do we check the conditional logic of when we call it, up in check_end_cycle? Let’s first write a test for reverse_or_continue:
def test_reverse_or_continue_no_edge(self):
fleet = InvaderFleet()
assert fleet.direction == +1
fleet.reverse_or_continue(0.1)
assert fleet.direction == +1
def test_reverse_or_continue_at_edge(self):
fleet = InvaderFleet()
assert fleet.direction == +1
fleet.at_edge(+1)
fleet.reverse_or_continue(0.1)
assert fleet.direction == -1
Two, one for each possibility. That logic is reasonably well tested. Should we check whether the origin changes as well? Yes.2
OK, this is interesting. Note the calls to copy in this test. They are necessary.
def test_reverse_or_continue_no_edge(self):
fleet = InvaderFleet()
step = Vector2(10, 0)
fleet.step = step.copy()
origin = fleet.origin.copy()
assert origin == Vector2(192, 512)
assert fleet.direction == +1
fleet.reverse_or_continue(1.0)
assert fleet.origin == origin + step
assert fleet.direction == +1
Why did I need the copy calls? We’ve been here before. I am not sure why they’ve made this choice, but the Pygame Vector2 += operation modifies the vector in place. This is not OK.
Somewhere I must have a +=. Yes, right here:
def reverse_or_continue(self, delta_time):
if self.reverse:
self.reverse = False
self.direction = -self.direction
self.origin += self.direction * self.step * delta_time + Vector2(0, 32)
else:
self.origin += self.direction * self.step * delta_time
self.next_invader = 0
I hate having to do this, but we’d better.
I change the other test:
def test_reverse_or_continue_at_edge(self):
fleet = InvaderFleet()
step = Vector2(10, 0)
fleet.step = step
origin = fleet.origin
assert origin == Vector2(192, 512)
assert fleet.direction == +1
fleet.at_edge(+1)
fleet.reverse_or_continue(0.1)
assert fleet.origin == origin + step + fleet.down_step
assert fleet.direction == -1
There is no down_step, yet. Extract Field on the Vector2(0, 32) and we have it. But the test is still not passing. Oh. It should subtract step, since we are reversing:
def test_reverse_or_continue_at_edge(self):
fleet = InvaderFleet()
step = Vector2(10, 0)
fleet.step = step
origin = fleet.origin
assert origin == Vector2(192, 512)
assert fleet.direction == +1
fleet.at_edge(+1)
fleet.reverse_or_continue(1.0)
assert fleet.origin == origin - step + fleet.down_step
assert fleet.direction == -1
Green.
I am still not happy. These tests seem too difficult to write, nor are they easy to read. I am happy to have them, since they found that += issue. We could have worked around it but it seems best not to allow the aliasing at all, by just avoiding use of += on vectors. But I am not pleased with the difficulty of writing them.
Let’s wrestle with that question a bit and then see about closing out this double-length article.
The direction check in those last couple of tests was pretty easy. It was my decision to check the actual motion that got messy. Let’s split the tests3 and look again.
def test_direction__reverse_or_continue_no_edge(self):
fleet = InvaderFleet()
assert fleet.direction == +1
fleet.reverse_or_continue(1.0)
assert fleet.direction == +1
def test_direction_reverse_or_continue_at_edge(self):
fleet = InvaderFleet()
assert fleet.direction == +1
fleet.at_edge(+1)
fleet.reverse_or_continue(1.0)
assert fleet.direction == -1
def test_origin_reverse_or_continue_no_edge(self):
fleet = InvaderFleet()
step = Vector2(10, 0)
fleet.step = step
origin = fleet.origin
assert origin == Vector2(192, 512)
fleet.reverse_or_continue(1.0)
assert fleet.origin == origin + step, "if this fails someone has modified a vector in place"
def test_origin_reverse_or_continue_at_edge(self):
fleet = InvaderFleet()
step = Vector2(10, 0)
fleet.step = step
origin = fleet.origin
assert origin == Vector2(192, 512)
fleet.at_edge(+1)
fleet.reverse_or_continue(1.0)
assert fleet.origin == origin - step + fleet.down_step
What if we didn’t set the step, just used what’s there? And some better names and better order:
def test_direction_reverses_at_edge(self):
fleet = InvaderFleet()
assert fleet.direction == +1
fleet.at_edge(+1)
fleet.reverse_or_continue(1.0)
assert fleet.direction == -1
def test_direction_unchanged_when_not_at_edge(self):
fleet = InvaderFleet()
assert fleet.direction == +1
fleet.reverse_or_continue(1.0)
assert fleet.direction == +1
def test_step_down_at_edge(self):
fleet = InvaderFleet()
origin = fleet.origin
fleet.at_edge(+1)
fleet.reverse_or_continue(1.0)
assert fleet.origin == origin - fleet.step + fleet.down_step, \
"if this fails someone may have modified a vector in place"
def test_no_step_down_when_not_at_edge(self):
fleet = InvaderFleet()
origin = fleet.origin
fleet.reverse_or_continue(1.0)
assert fleet.origin == origin + fleet.step, \
"if this fails someone may have modified a vector in place"
OK, these don’t seem so bad to me now. I’ll allow them. Need to think further on this.
Anyway, the invaders do step down now at the edge. The speed seems way too slow, but my rough calculations on the step sizes make me think it’s the same scale as the original game and my Codea Lua version. Must think on that as well. But we are green enough to commit:
Commit: Invaders step down at edge. Tuning needed.
Summary
It could be down to the smoke in the air or the fog in my brain, and it shouldn’t be a surprise that changing from moving everyone to moving one at a time might be tricky, but something about this whole double session makes me think the objects aren’t helping me as much as they might. Perhaps the InvaderFleet needs a cycle handler or direction manager or some such object to help us.
Next time, if I remember, we’ll look at InvaderFleet and see how its members change. I’m sure we’ll find some subsets that change at different rates. That’s a big clue that there might be more than one object in there, because the object isn’t as cohesive as it might be.
Another clue is that the tests were hard to write, although I did finally boil them down to a few shorter simpler ones by just avoiding my bad habit of checking too many things in one test. Still, there’s something wrong because I couldn’t see how to write them at all until I had the code.
Let’s underline that
It often seems reasonable to someone who doesn’t use TDD, or is new to it, to think “how can I write a test for my code if I don’t have any code?”
The thing is, at least if we are Detroit4 School TDDers, we write our tests to check results, typically values. We don’t test to see whether something was called, we test to see that we got the desired result. When it is difficult to know what the right result is in advance, it’s a dead giveaway that we don’t have a result in mind.
Well, sometimes, I freely grant, I seem to have to code a bit to see what the result should be, but in general, when I can’t see how to write a test, it’s because I’m not clear on he objective … or because the objective is hard to see in the code.
Either way, difficulty with a test suggests lack of clarity in the code … or in our mind. Either way, we probably need to make the code more clear, so that our mind will be more clear.
So I believe that there is something funky going on in InvaderFleet, most likely that it is trying to ask for some helper objects and I’m not hearing it yet.
We’ll consider that next time. This time, get real, it’s Sunday and time to do almost anything but this.
See you next time!
-
Note added Sunday: This is an early sign that there’s trouble. I think, much later, that the objects aren’t helping me. It’s also possible that I was just fuzzy in the brain. ↩
-
Well, no. We see further down that four simpler tests are much better than two complicated ones. Who knew? (Right. Everyone.) ↩
-
Yes. Smaller simpler tests. At last I begin to learn. Hey, six decades isn’t that long, really, is it? ↩
-
It’s “Detroit School v London School”, not “Chicago School v London School”. Detroit School TDD was first used in the Detroit suburb of Centerline, and while it was frequently taught in later years in Chicago and many other cities, the proper name is “Detroit School”. I don’t know who started saying “Chicago School” but they were at best mistaken. ↩