Python 123 - Loud Noises
We are getting a new roof today. Might be hard to concentrate. Fortunately, I try never to need deep concentration anyway. Shall we work on Saucer today? We shall. (TDD really works nicely here.)
Wow those guys are loud at 0730. Tearing off the old roof, I guess. Kitty is nowhere to be seen, probably hiding in the basement. I don’t blame her.
So, the idea this morning will be to extract the firing logic from Saucer and move it to a class of its own. I was thinking of calling it TargetingComputer but after typing HyperspaceGenerator a million times yesterday, I may try to come up with a shorter yet still meaningful name.
Let’s begin with something simple. The Saucer, as part of targeting, calculates the angle between its position and that of the Ship. The code for that is presently this, the best I could figure out at the time:
class Saucer(Flyer):
def angle_to(self, ship):
aiming_point = nearest_point(self.position, ship.position, u.SCREEN_SIZE)
angle_point = aiming_point - self.position
return degrees(atan2(angle_point.y, angle_point.x))
We calculate the best point of aiming, possibly across the boundary of the screen. Then the difference between the two points gives us the (dx,dy), and the arctangent of that is the angle we need.
There is a better way that I’ve discovered, and I have a rudimentary test that I used to check it:
def test_angle_to(self):
ship_position = Vector2(550, 550)
ship = Ship(ship_position)
saucer_position = Vector2(500, 500)
saucer = Saucer()
saucer.move_to(saucer_position)
angle = saucer.angle_to(ship)
# assert angle == 90
vector_angle = Vector2(0, 0).angle_to(ship_position - saucer_position)
assert vector_angle == angle
I just plugged a couple of values in there to check it. I don’t know whether we have a decent test for this elsewhere. I have my doubts. I am still confident enough to change angle_to:
def angle_to(self, ship):
aiming_point = nearest_point(self.position, ship.position, u.SCREEN_SIZE)
return Vector2(0, 0).angle_to(aiming_point - self.position)
We do need tests for that. But we’ll probably get some as we do the new object. Make a Jira sticky note anyway: “tests for targeting”. Commit this: use pygame angle_to.
OK, fingers warmed up. Let’s see what we might do here.
We have those two top-level functions about calculating the nearest target, i.e. the shortest path to the ship, which might be a wrap-around shot across the border. Should we start there? No.
Let’s look at where we might call our new object.
def update(self, delta_time, fleets):
player.play("saucer_big", self._location, False)
self.fire_if_possible(delta_time, fleets)
self.check_zigzag(delta_time)
self._move(delta_time, fleets)
def fire_if_possible(self, delta_time, fleets):
self._missile_timer.tick(delta_time, self.fire_if_missile_available, fleets)
def fire_if_missile_available(self, fleets) -> bool:
if self._missile_tally >= u.SAUCER_MISSILE_LIMIT:
return False
missile = self.create_missile()
fleets.add_flyer(missile)
return True
def create_missile(self):
"""callback method, called from saucer_missiles.fire"""
should_target = random.random()
random_angle = random.random()
return self.suitable_missile(should_target, random_angle)
That comment is wrong, left over from when the fleet did the firing. Remove, commit: remove comment.
Where should we split the code between Saucer and the Gunner (trying out names)? I don’t think we want the Gunner to be a separate Flyer, just a part of the Saucer. So the _missile_tally and _ship members will be ginned up by Ship. We could pass them all in whenever we call the Gunner.
What if we move everything but the update to Gunner? We’ll have it implement fire_if_possible, accepting the tally and ship right there.
How should we proceed? This thing is rife with random numbers, which makes it difficult to test.
Alternatives
I see two ways to go. One would be to TDD a new targeting computer object, and then plug it in. The other would be to write tests for the existing targeting and then incrementally move code over to the new object in a strangler kind of fashion.
Because of the random numbers being hidden inside methods, I think TDDing a new version, but likely borrowing the old code, is the better course. We might even get a better design out of the deal.
Begin with a test …
class TestGunner:
def test_exists(self):
Gunner()
I was considering giving Gunner a link back to Saucer, but let’s try to do without that, instead providing whatever it needs when we call it.
That’s enough to create the class.
class Gunner:
pass
I import Gunner into the test and it passes. Woot! Shall we commit? Sure, and then a small party. Commit: Initial Gunner.
So what do we know? We know that we’ll be calling Gunner.fire, or fire_if_possible, on every update. Shall we speculate a bit on its calling sequence? I think we can safely say that it needs to know the Ship, which could be None, the missile tally, delta-time, and the fleets. And it needs to know the Saucer’s position, doesn’t it?
No, we can do better. It needs the ship’s position, not the ship. And that position could still be None.
Wait! Weird idea. The way it works now, if there is no ship, or if the dice so dictate, the saucer fires a random missile. We currently do that … No. Premature optimization. Belay that idea before it’s even well formed. Get back to work.
Let’s TDD fire, just to drive out the shell of the method:
def test_fire(self):
delta_time = 0.1
saucer_position = Vector2(0, 0 )
ship_position = Vector2(1, 1)
fleets = Fleets()
Gunner().fire(delta_time, saucer_position, ship_position, fleets)
Nice sketch of what we’ll provide, I think. PyGame wants to help me create the method.
class Gunner:
def fire(self, delta_time, saucer_position, ship_position, fleets):
pass
Perfect so far. Is there anything we could assert? No saucer missiles in fleets, it’s too early.
def test_fire(self):
delta_time = 0.1
saucer_position = Vector2(0, 0 )
ship_position = Vector2(1, 1)
fleets = Fleets()
Gunner().fire(delta_time, saucer_position, ship_position, fleets)
assert not FI(fleets).saucer_missiles
Hm. Now if we wrote a test with a larger delta_time, we can expect a missile, even though we won’t know much about what it is. Let’s write that. It should drive out the timer.
- About the Timer
- Can we just start the timer on init? I think so, because we create a new Saucer every time around, so its timer can be created when we create the Saucer’s Gunner.
I rename the test above test_no_fire and create a new one:
def test_fire(self):
delta_time = u.SAUCER_MISSILE_DELAY
saucer_position = Vector2(0, 0 )
ship_position = Vector2(1, 1)
fleets = Fleets()
Gunner().fire(delta_time, saucer_position, ship_position, fleets)
assert FI(fleets).saucer_missiles
Now to make that work. We’ll put a timer in an __init__:
class Gunner:
def __init__(self):
self._timer = Timer(u.SAUCER_MISSILE_DELAY)
Then improve fire from its current form:
def fire(self, delta_time, saucer_position, ship_position, fleets):
pass
How about this:
def fire(self, delta_time, saucer_position, ship_position, fleets):
self._timer.tick(delta_time, self.fire_missile, saucer_position, ship_position, fleets)
def fire_missile(self, saucer_position, ship_position, fleets):
pass
Nearly there. Just need this:
def fire_missile(self, saucer_position, ship_position, fleets):
fleets.add_flyer(Missile.from_saucer(saucer_position, Vector2(0, 0)))
Test passes. We just fired a zero-velocity missile at our own location. Probably unwise.
Commit: Gunner improving. I could probably call these commits “Save point”, that’s all they are, places to which we can roll back if we need to.
Now, in fire_missile, we know we are really going to create a missile and add it: the timer has expired. And there are two random numbers that we may need. If the ship is present, we roll against u.SAUCER_TARGETING_FRACTION to see if we target a missile at the ship, and if not we roll a random angle at which to fire it.
As we test this thing, we will do well to provide places to inject the results of the rolls, so that our tests can be explicit. I think what I’ll do is something like this:
In a given method, we’ll roll the random number, and then immediately call another method, passing that value. That second method is the one we’ll test, passing in our hand-crafted numbers, which will let us check expectations on the Missile produced.
We’ll have to have confidence that the Gunner methods call those inner methods, and we won’t be able to test that easily, but it’ll be straightforward and free of any conditional logic.
Let’s extend the Gunner logic right now.
def fire_missile(self, saucer_position, ship_position, fleets):
should_target = random.random()
random_angle = random.random()
self.create_missile(should_target, random_angle, saucer_position, ship_position, fleets)
def create_missile(self, should_target, random_angle, saucer_position, ship_position, fleets):
fleets.add_flyer(Missile.from_saucer(saucer_position, Vector2(0, 0)))
Now we have something we can call with specific expectations.
Let’s do a “random” missile.
def test_random_missile(self):
no_target = 0.5
angle = 0.0
fleets = Fleets()
fi = FI(fleets)
Gunner().create_missile(no_target, angle, Vector2(500, 500), None, fleets)
assert fi.saucer_missiles
This is green now, but we’ll set some criteria on the missile we get.
And I realize we’re going to need information we don’t have. Gunner is going to need the saucer velocity to add to missile velocity. We’ll deal with that. Another parameter, or a link back. We’ll see.
Let me create the Saucer explicitly in that test.
No. We’ll pass the velocity. That’s better. We’ll do it now, changing the signature of these methods.
def fire(self, delta_time, saucer_position, saucer_velocity, ship_position, fleets):
self._timer.tick(delta_time, self.fire_missile, saucer_position, saucer_velocity, ship_position, fleets)
def fire_missile(self, saucer_position, saucer_velocity, ship_position, fleets):
should_target = random.random()
random_angle = random.random()
self.create_missile(should_target, random_angle, saucer_position, saucer_velocity, ship_position, fleets)
def create_missile(self, should_target, random_angle, saucer_position, saucer_velocity, ship_position, fleets):
fleets.add_flyer(Missile.from_saucer(saucer_position, saucer_velocity))
Those calling sequences are getting a bit long. We’ll let that be for now.
Let’s see what Missile.from_saucer actually does, we’ll be checking it shortly.
It just creates a missile with the values provided. It’s up to the caller to miss himself and set the velocity.
We’ll extend the code further, relying on our existing tests.
def create_missile(self, should_target, random_angle, saucer_position, saucer_velocity, ship_position, fleets):
if should_target <= u.SAUCER_TARGETING_FRACTION:
pass
else:
self.create_random_missile(random_angle, saucer_position, saucer_velocity, fleets)
def create_random_missile(self, random_angle, saucer_position, saucer_velocity, fleets):
fleets.add_flyer(Missile.from_saucer(saucer_position, saucer_velocity))
Reflection
Do you see what’s happening here? Supported by our few tests, which are strong enough for what we’re doing so far, we’re sketching in the shape of the Gunner code. We just modified it so that it is honoring the random number that selects whether to target the ship. (That will need to be made stronger, let’s not forget, in case there is no ship. We’re not there yet.)
We have much of the shape of the object in place, and it’s fairly neat, although the long parameter lists are slightly irritating.
Now let’s refine our most recent test to ask for some info from the resulting missile, so that we can determine whether it’s correct.
My plan is to cheat on this a bit, because we have working code to borrow.
def create_random_missile(self, random_angle, saucer_position, saucer_velocity, fleets):
missile = self.missile_at_angle(saucer_position, random_angle*360.0, saucer_velocity)
fleets.add_flyer(missile)
def missile_at_angle(self, position, desired_angle, velocity_adjustment):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(desired_angle) + velocity_adjustment
offset = Vector2(2 * self._radius, 0).rotate(desired_angle)
return Missile.from_saucer(position + offset, missile_velocity)
Intermittent Test! Neat!
The tests pass … almost always. We have an intermittent test because now that we have rolled the dice, our first missile test can hit the pass in the should_target code. Let’s create a faulty missile from there.
def create_missile(self, should_target, random_angle, saucer_position, saucer_velocity, ship_position, fleets):
if should_target <= u.SAUCER_TARGETING_FRACTION:
self.create_targeted_missile(saucer_position, ship_position, fleets)
else:
self.create_random_missile(random_angle, saucer_position, saucer_velocity, fleets)
def create_random_missile(self, random_angle, saucer_position, saucer_velocity, fleets):
missile = self.missile_at_angle(saucer_position, random_angle*360.0, saucer_velocity)
fleets.add_flyer(missile)
def missile_at_angle(self, position, desired_angle, velocity_adjustment):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(desired_angle) + velocity_adjustment
offset = Vector2(2 * self._radius, 0).rotate(desired_angle)
return Missile.from_saucer(position + offset, missile_velocity)
def create_targeted_missile(self, saucer_position, ship_position, fleets):
missile = Missile.from_saucer(Vector2(-5, -5), Vector2(0, 0))
fleets.add_flyer(missile)
The tests are solidly green now. Let’s check details on the current un-targeted test:
def test_random_missile(self):
no_target = 0.5
angle = 0.0
fleets = Fleets()
fi = FI(fleets)
position = Vector2(500, 500)
Gunner().create_missile(no_target, angle, position, Vector2(0, 0), None, fleets)
assert fi.saucer_missiles
missile = fi.saucer_missiles[0]
assert missile.position == position
I expect this to fail with a difference of 40, twice the saucer radius, in some direction.
Expected :<Vector2(500, 500)>
Actual :<Vector2(540, 500)>
Super. Angle zero is +x, good to know. Fix test.
def test_random_missile(self):
no_target = 0.5
angle = 0.0
fleets = Fleets()
fi = FI(fleets)
position = Vector2(500, 500)
Gunner().create_missile(no_target, angle, position, Vector2(0, 0), None, fleets)
assert fi.saucer_missiles
missile = fi.saucer_missiles[0]
assert missile.position == position + Vector2(40, 0)
Test is green. Commit: save point.
Now test the velocity. It should be whatever the missile velocity is, all in the x direction. No, let’s put a velocity in on the create and see it added in.
def test_random_missile(self):
no_target = 0.5
angle = 0.0
fleets = Fleets()
fi = FI(fleets)
position = Vector2(500, 500)
Gunner().create_missile(no_target, angle, position, Vector2(12, 34), None, fleets)
assert fi.saucer_missiles
missile = fi.saucer_missiles[0]
assert missile.position == position + Vector2(40, 0)
assert missile.velocity_testing_only == Vector2(0, 0)
Expected :<Vector2(0, 0)>
Actual :<Vector2(178.667, 34)>
Weird number, let’s check in u:
MISSILE_SPEED = SPEED_OF_LIGHT/3
Speed of light is 500. 166.667 + 12. Great. Improve the test:
def test_random_missile(self):
no_target = 0.5
angle = 0.0
fleets = Fleets()
fi = FI(fleets)
position = Vector2(500, 500)
Gunner().create_missile(no_target, angle, position, Vector2(12, 34), None, fleets)
assert fi.saucer_missiles
missile = fi.saucer_missiles[0]
assert missile.position == position + Vector2(40, 0)
assert missile.velocity_testing_only == Vector2(u.MISSILE_SPEED + 12, 34)
Commit: Gunner correctly creates random missiles.
Reflection and Summary
This is going very nicely, isn’t it? Let’s wrap up this article here, it’s long enough, you deserve a break, and so do I. We’ll take this up later in the day.
We have essentially no tests for saucer missiles prior to today’s. Someone must have just coded up those missiles and debugged it by hand. Here’s the initial article on saucer missiles if you want to check up on me. I note in reading that that we are not passing in the missile tally to fire, so we are missing a test. I make a sticky note with a red star to remind me to do that.
The development of Gunner is proceeding in a very nice step-by-step fashion. There was that one stumble that created the intermittent test, and I wish it had created a solidly-failing test. Maybe we should change the earlier test to call the new method that accepts random numbers, set so that it goes down the other path. Good idea, I’ll make a note of that too.
The Gunner class so far looks very nice and it is more testable than the existing code, because we have a place to inject the random numbers for testing purposes.
There may be a lesson in here somewhere. Something about the value of the TDD style? Anyway it’s going most smoothly, and I think we’ll be able to plug the new thing in pretty easily. Perhaps there will be a bit of adjustment needed but right now I think it will be ready to go.
See you next time!