Python 048 - Targeting
The Saucer missile should usually be random, but sometimes aimed. Let’s see about aiming. (It goes very smoothly, but I’m a bit light on tests.)
The elements of the aiming story are something like this:
- Every now and then (p = 1/4?), the Saucer’s missile is aimed;
- It selects an aiming point, based on the ship’s position;
- And possibly based on its velocity;
- It fires at that point, or
- If a cross-border shot is closer, fires across the border.
This sums up to picking a point and firing at it,
The border isn’t a mirror, it wraps around: suppose the real ship is at (x,y), not moving, and the screen size is (s,s). If the ship doesn’t move, there are nine points nearby that we could aim at. Consider these nine vectors: (0, 0), (0, -s), (0, s), (-s, 0), (-s, -s), (-s, -s), (s, 0), (s, -s), (s, s).
If we add any of those vectors to the ship’s coordinates, and aim at the resulting point, and if the missile lasts long enough, we’ll hit the ship.
This is hard to see without a diagram. I drew one on a card: here’s a picture of it sitting on top of Jira:
To select the point at which to aim, we should choose the nearest one. We could consider all nine points and compute the full distance, square root and all that, or we could compute distance-squared and avoid the square root, or we can consider the axes separately, choosing the nearest x and nearest y. On the picture above, the best shot for the red saucer in the lower right is to shoot down and to its right, as the arrow shows.
One Sub-problem
One sub-problem, then, is code that, given a target point and a firing point, chooses which of the nine aiming points is nearest. I think we should test-drive that code. Further, I think we should start by driving out a single-value function selecting which of target.x, target.x - s, target.x + s is closer to shooter.x. That function will also work for y, of course.
Let’s TDD:
def test_can_choose_nearest_target(self):
target = 100
screen_size = 500
shooter = 200
assert nearest(shooter, target, screen_size) == 100
I’m going to put the nearest function right with the test for now. This is in test_collisions.py, by the way.
I implement this:
def nearest(shooter, target, size):
dist = abs(target - shooter)
t_min = target - size
t_min_dist = abs(t_min - shooter)
t_max = target + size
t_max_dist = abs(t_max - shooter)
if t_min_dist < dist:
return t_min
elif t_max_dist < dist:
return t_max
else:
return target
That seems to me to be the actual function I need, but I want at least a couple more tests.
def test_can_choose_nearest_target(self):
target = 100
screen_size = 500
shooter = 200
assert nearest(shooter, target, screen_size) == 100
shooter = 400
assert nearest(shooter, target, screen_size) == 100 + 500
target = 400
shooter = 100
assert nearest(shooter, target, screen_size) == 400 - 500
I think we’re good. Now that we have the function nearest, let’s think about the bigger picture. No, first let’s do nearest_point with TDD also, while we’re fresh.
def test_can_choose_nearest_point(self):
target = Vector2(100, 400)
screen_size = 500
shooter = Vector2(150, 150)
assert nearest_point(shooter, target, screen_size) == Vector2(100, 400)
shooter = Vector2(150, 50)
assert nearest_point(shooter, target, screen_size) == Vector2(100, -100)
def nearest_point(shooter, target, size):
nearest_x = nearest(shooter.x, target.x, size)
nearest_y = nearest(shooter.y, target.y, size)
return Vector2(nearest_x, nearest_y)
I could test another combination, but I am satisfied. Now, let’s think about the bigger picture.
Reflection
We’ll probably want to move these functions somewhere outside the tests. We could make them static methods on Saucer, or top-level functions in Saucer. They’re more general than Saucer, so maybe they deserve to be functions in u or something. Anyway, somewhere.
Saucer will need access to the ship. Right now, I think the saucer does everything in its move function:
def move(self, delta_time, saucers, saucer_missiles):
self.fire_if_possible(delta_time, saucer_missiles)
self.check_zigzag(delta_time)
self.position += delta_time*self.velocity
x = self.position.x
if x < 0 or x > u.SCREEN_SIZE:
if self in saucers:
saucers.remove(self)
We must already have a special move call for saucer. Command+B says:
def move_everything(self,dt):
for the_saucer in self.saucers.copy():
the_saucer.move(dt, self.saucers, self.saucer_missiles)
for missile in self.saucer_missiles:
missile.move(dt)
for the_ship in self.ships:
the_ship.move(dt)
for asteroid in self.asteroids:
asteroid.move(dt)
for missile in self.missiles:
missile.move(dt)
Let’s just pass in the ships collection as well, and let the Saucer deal with that. The alternative would be to pass a possibly None ship, and that seems bad.
I’ll just do that much. There are 9 calls, it says here, to this function. Let’s try the Change Signature refactoring.
Signature preview is:
move(self, delta_time, saucers, saucer_missiles, ships = [])
Hit the refactor button and stand back. ARRGH!!! The refactoring tool has done bad things:
for the_saucer in self.saucers.copy():
the_saucer.move(dt, self.saucers, self.saucer_missiles)
for missile in self.saucer_missiles:
missile.move(dt,,
for the_ship in self.ships:
the_ship.move(dt,,
for asteroid in self.asteroids:
asteroid.move(dt,,
for missile in self.missiles:
missile.move(dt,,
That’s not good at all. Fortunately, it will undo. And let’s commit those tests before we foul up again. Commit: test and code for nearest point.
OK, I’m going to do this manually.
class Saucer:
def move(self, delta_time, saucers, saucer_missiles, ships=[]):
self.fire_if_possible(delta_time, saucer_missiles)
self.check_zigzag(delta_time)
self.position += delta_time*self.velocity
x = self.position.x
if x < 0 or x > u.SCREEN_SIZE:
if self in saucers:
saucers.remove(self)
class Game:
def move_everything(self,dt):
for the_saucer in self.saucers.copy():
the_saucer.move(dt, self.saucers, self.saucer_missiles, self.ships)
...
OK, that seems better, and with the default my tests will continue to work, at least for now, if there are other calls to Saucer.move. I don’t think there are.
I think I’m going to work without a net for a while. Consider this a spike, except that if it works, I’ll keep it. With a bit of luck I might be able to think of a test by the time I’m further along. Right now, I cannot.
We’ll pass ships to fire_if_possible:
def move(self, delta_time, saucers, saucer_missiles, ships=[]):
self.fire_if_possible(delta_time, saucer_missiles, ships)
self.check_zigzag(delta_time)
self.position += delta_time*self.velocity
x = self.position.x
if x < 0 or x > u.SCREEN_SIZE:
if self in saucers:
saucers.remove(self)
And:
def fire_if_possible(self, delta_time, saucer_missiles, ships=[]):
if self.firing_is_possible(delta_time, saucer_missiles):
self.fire_a_missile(saucer_missiles)
PyCharm doesn’t like that default becuase it’s mutable. I don’t think I care.
The parameter has to push on down:
def fire_a_missile(self, saucer_missiles, ships=[]):
saucer_missiles.append(self.create_missile(ships))
self.missile_timer = u.SAUCER_MISSILE_DELAY
def create_missile(self, ships=[]):
degrees = random.random()*360.0
return self.missile_at_angle(degrees)
Whew. Tests are all still green. Commit: ships collection passed to Saucer.move and down into firing logic.
That’s kind of bold of me, committing this code, but it is harmless and I don’t think I’ll have to remove it. Now to actually do something with the ship:
def create_missile(self, ships=[]):
if ships:
ship = ships[0]
degrees = self.angle_to(ship.position)
else:
degrees = random.random()*360.0
return self.missile_at_angle(degrees)
This code will call angle_to unconditionally if the ship is present, so assuming we get angle_to right, we should aim at the ship on every shot.
def angle_to(self, position):
aiming_point = nearest_point(self.position, position, u.SCREEN_SIZE)
angle_point = aiming_point - self.position
return degrees(atan2(angle_point.y, angle_point.x))
I have somehow managed to get an import loop. I resolve it by moving the nearest functions over to Saucer and relinking the tests. However, there is another issue, which is that the way I fire now adds the sauce velocity to the missile’s I think I’ll patch that.
def missile_at_angle(self, degrees):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(degrees)
offset = Vector2(2 * self.radius, 0).rotate(degrees)
return Missile.from_saucer(self.position + offset, self.velocity + missile_velocity)
I’ll remove the self.velocity bit. With that change made, and the ship not moving, the sauce missile hits the ship every time. So we know we’ve got it cracked … and we’ll take a break.
Break
Back from some errands, picking up where we left off. It’s been a half hour or more, I’d better review what I’ve got.
def fire_a_missile(self, saucer_missiles, ships=[]):
saucer_missiles.append(self.create_missile(ships))
self.missile_timer = u.SAUCER_MISSILE_DELAY
def create_missile(self, ships=[]):
if ships:
ship = ships[0]
degrees = self.angle_to(ship.position)
else:
degrees = random.random() * 360.0
return self.missile_at_angle(degrees)
def angle_to(self, position):
aiming_point = nearest_point(self.position, position, u.SCREEN_SIZE)
angle_point = aiming_point - self.position
return degrees(atan2(angle_point.y, angle_point.x))
def missile_at_angle(self, degrees):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(degrees)
offset = Vector2(2 * self.radius, 0).rotate(degrees)
return Missile.from_saucer(self.position + offset, missile_velocity)
That last line used to say:
return Missile.from_saucer(self.position + offset, self.velocity + missile_velocity)
Normally we add the saucer’s velocity to the missile’s velocity, because it’s more realistic and looks better, since the missiles will move faster when fired forward, slower when fired backward.
We need to factor the saucer velocity out, however, when we’re targeting.
There’s one more concern, which is that we don’t really want to target all the time when the ship is present, just some of the time. Supposing we want to target 1/4 of the time, we might have something like this:
def create_missile(self, ships=[]):
if ships and random.random() <= u.SAUCER_TARGETING_FRACTION:
ship = ships[0]
degrees = self.angle_to(ship.position)
else:
degrees = random.random() * 360.0
return self.missile_at_angle(degrees)
Well, we could pass in a velocity to missile_at_angle, like this:
def create_missile(self, ships=[]):
if ships and random.random() <= u.SAUCER_TARGETING_FRACTION:
velocity_adjustment = Vector2(0, 0)
ship = ships[0]
degrees = self.angle_to(ship.position)
else:
velocity_adjustment = self.velocity
degrees = random.random() * 360.0
return self.missile_at_angle(degrees, velocity_adjustment)
And then here …
def missile_at_angle(self, degrees, velocity_adjustment):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(degrees) + velocity_adjustment
offset = Vector2(2 * self.radius, 0).rotate(degrees)
return Missile.from_saucer(self.position + offset, missile_velocity)
In the game, that works as advertised. I have three tests failing. They all just needed the new parameter added:
def test_random_missile_velocity_0(self):
saucer = Saucer()
saucer.velocity = Vector2(100, 200)
zero_angle_velocity = Vector2(u.MISSILE_SPEED, 0)
missile = saucer.missile_at_angle(0, saucer.velocity)
assert missile.velocity == saucer.velocity + zero_angle_velocity
def test_random_missile_velocity_90(self):
saucer = Saucer()
saucer.velocity = Vector2(100, 200)
zero_angle_velocity = Vector2(u.MISSILE_SPEED, 0)
missile = saucer.missile_at_angle(90, saucer.velocity)
assert missile.velocity == saucer.velocity + zero_angle_velocity.rotate(90)
def test_random_missile_position_90(self):
saucer = Saucer()
saucer.position = Vector2(123, 456)
missile = saucer.missile_at_angle(90, saucer.velocity)
expected_offset = Vector2(2*saucer.radius, 0).rotate(90)
assert missile.position == saucer.position + expected_offset
We are green and the game works. Commit: When ship is present, saucer fires targeted missile u.SAUCER_TARGETING_FRACTION of the time (0.25 at present).
Let’s review that code.
Reflection
In past implementations of Asteroids, I have struggled to find a good way to make the ship accessible to the saucer at the time of firing. I’ve resorted to various means, making the ship public, or providing access to the Game as public and trolling through it. I don’t think I’ve ever just passed it down into the code, but it’s not a bad thing to have done.
The most irritating bit is that it treks a long way down before it’s used:
def move(self, delta_time, saucers, saucer_missiles, ships=[]):
self.fire_if_possible(delta_time, saucer_missiles, ships)
self.check_zigzag(delta_time)
self.position += delta_time * self.velocity
x = self.position.x
if x < 0 or x > u.SCREEN_SIZE:
if self in saucers:
saucers.remove(self)
def fire_if_possible(self, delta_time, saucer_missiles, ships=[]):
if self.firing_is_possible(delta_time, saucer_missiles):
self.fire_a_missile(saucer_missiles, ships)
def fire_a_missile(self, saucer_missiles, ships=[]):
saucer_missiles.append(self.create_missile(ships))
self.missile_timer = u.SAUCER_MISSILE_DELAY
def create_missile(self, ships=[]):
if ships and random.random() <= u.SAUCER_TARGETING_FRACTION:
velocity_adjustment = Vector2(0, 0)
ship = ships[0]
degrees = self.angle_to(ship.position)
else:
velocity_adjustment = self.velocity
degrees = random.random() * 360.0
return self.missile_at_angle(degrees, velocity_adjustment)
That’s not too awful. I’d like to get rid of the defaulted lists. Let’s make them required.
All I had to do was remove all the =[] and then fix the tests that called move or fire_if_possible to have another [] as a parameter. Those tests are not easy to read. But the job is done, commit: Make ships parameter required in saucer.move and subordinate calls.
Now I’ll fix up those calls, like this:
def test_can_only_fire_two(self):
saucer = Saucer()
saucer_missiles = []
assert saucer.missile_timer == u.SAUCER_MISSILE_DELAY
saucer.fire_if_possible(delta_time=0.1, saucer_missiles=saucer_missiles, ships=[])
assert not saucer_missiles
saucer.fire_if_possible(u.SAUCER_MISSILE_DELAY, saucer_missiles=saucer_missiles, ships=[])
assert len(saucer_missiles) == 1
assert saucer.missile_timer == u.SAUCER_MISSILE_DELAY
saucer.fire_if_possible(u.SAUCER_MISSILE_DELAY, saucer_missiles=saucer_missiles, ships=[])
assert len(saucer_missiles) == 2
saucer.fire_if_possible(u.SAUCER_MISSILE_DELAY, saucer_missiles=saucer_missiles, ships=[])
assert len(saucer_missiles) == 2
That’s certainly more explicit. I fix up all the ones I can find.
def test_move(self):
saucer = Saucer()
saucer.ready()
starting = saucer.position
saucer.move(delta_time=0.1, saucers=[], saucer_missiles=[], ships=[])
assert saucer.position.x == u.SAUCER_VELOCITY.x*0.1
saucer.move(delta_time=0.1, saucers=[], saucer_missiles=[], ships=[])
assert saucer.position.x == 2*u.SAUCER_VELOCITY.x*0.1
def test_vanish_at_edge(self):
saucer = Saucer()
saucers = [saucer]
saucer.ready()
saucer.move(1, saucers, [], [])
assert saucers
while saucer.position.x < u.SCREEN_SIZE:
assert saucers
saucer.move(delta_time=0.1, saucers=saucers, saucer_missiles=[], ships=[])
assert not saucers
I think we’ve improved that. Commit: make arguments explicit in move and fire_if_possible saucer tests.
Summary
The code is decent if not great. I see some possibilities for method extraction, and the Saucer class has a lot of methods, 19 if I’m not mistaken. We could perhaps extract a targeting subsystem or something.
The individual targeting functions are tested but the logic that checks for ships and rolls the dice is not tested. I don’t see a good way to test that, but will ask about it tonight if I remember. Anyway it is simple and looks good to me. Not exactly the fully professional determination there but it’s the best I have at the moment.
I have to give myself a passing but not great grade on testing. Situations like this often leave me without an idea for how to test that conditional logic. I’ll ask my betters tonight if possible.
The feature could be improved. One thing that could be done would be to project the ship’s path a bit forward in time and aim at that location. That would give us a better chance of hitting the moving ship. Or, if we were fanatics, we could find the full quadratic solution for targeting and plug that in. I think I did that in some past version of the game. However, if my recollection of the original game is accurate, its targeted missiles were just aimed in the general direction of the ship, with a narrowing angle of deviation as score increased. Something like that. And, I believe that the small saucer fires only targeted shots, but we don’t even have a small saucer yet.
But we certainly do have a noticeable improvement in Saucer missile accuracy. A good morning’s work.
See you next time!
- OOPS
- While testing the targeting, I turned off asteroid creation and shipped the product that way. Fortunately I caught myself quickly. Fixed. Oops.