Python Asteroids on GitHub

Again today, I plan to make the necessary adjustment to missile velocity to improve targeting. Again today, I have an idea. (First of a few articles all reflecting one morning session.)

Given that the missile is targeted, its starting position is such that it gets a small head start compared to where the targeting solution thought it would start, so it tends to get to the target a bit early, early enough to miss quite frequently. I believe that if we were to scale the missile’s velocity down at the last minute, we could adjust properly.

The scaling, I believe, should go something like this:

``````distance_to_target = missile_velocity*aiming_time
adjusted_distance = distance_to_target = 2*saucer_radius
adjustment = adjusted_distance / distance_to_target
velocity = velocity*adjustment
``````

The trick is where to put it, and having the necessary values at that time.

Let’s review the code, yet again.

``````    def fire_available_missile(self, chance, fleets, saucer, ship_or_none):
ship_position = self.select_aiming_point(chance, saucer, ship_or_none)
self.create_targeted_missile(saucer.position, ship_position, fleets)

def select_aiming_point(self, chance, saucer, ship_or_none):
if not ship_or_none:
return self.random_position()
elif saucer.always_target:
return self.optimal_aiming_point(saucer, ship_or_none)
elif chance < u.SAUCER_TARGETING_FRACTION:
return self.optimal_aiming_point(saucer, ship_or_none)
else:
return self.random_position()

def optimal_aiming_point(self, saucer, ship):
target_position = self.closest_aiming_point(saucer.position, ship.position, u.SCREEN_SIZE)
delta_position = target_position - saucer.position
aim_time = self.time_to_target(delta_position, ship.velocity)
return target_position + ship.velocity * aim_time

def missile_at_angle(self, position, desired_angle):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(desired_angle)
offset = Vector2(2 * self._radius, 0).rotate(desired_angle)
return Missile.from_saucer(position + offset, missile_velocity)
``````

This code only sets the missile velocity down there in `missile_at_angle`. We can only compute the desired ratio, in this code, just after the calculation of `aim_time`, in `optimal_aiming_point`. And, the ratio should be set to one if we’ve not called `optimal_aiming_point`.

Let’s try a spike. I am green with no pending changes.

We’ll patch in a new member:

``````    def __init__(self, saucer_radius=20):
self._timer = Timer(u.SAUCER_MISSILE_DELAY)
self._radius = saucer_radius
self._adjustment_ratio = 1
``````

We’ll make sure to initialize it before firing a missile:

``````    def fire_available_missile(self, chance, fleets, saucer, ship_or_none):
self._adjustment_ratio = 1
ship_position = self.select_aiming_point(chance, saucer, ship_or_none)
self.create_targeted_missile(saucer.position, ship_position, fleets)
``````

We’ll set it right after we get the `aim_time`.

``````    def optimal_aiming_point(self, saucer, ship):
target_position = self.closest_aiming_point(saucer.position, ship.position, u.SCREEN_SIZE)
delta_position = target_position - saucer.position
aim_time = self.time_to_target(delta_position, ship.velocity)
# egregious hack
self.set_adjustment_ratio(aim_time)
return target_position + ship.velocity * aim_time

def set_adjustment_ratio(self, aim_time):
distance_to_target = aim_time*u.MISSILE_SPEED
adjusted_distance = distance_to_target - 2*self._radius
self._adjustment_ratio = adjusted_distance / distance_to_target
``````

And we’ll use it when we set the velocity:

``````    def missile_at_angle(self, position, desired_angle):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(desired_angle) * self._adjustment_ratio
offset = Vector2(2 * self._radius, 0).rotate(desired_angle)
return Missile.from_saucer(position + offset, missile_velocity)
``````

Some tests have failed. I am not entirely surprised but I didn’t predict this so let’s see what they are. This is in contrast to my usual impatient approach of running the game right away.

One is a check against missile velocity, so it will need fixing. The other is a test to see whether the missile hits the target, and it had an allowance of 20 units and got zero. So that’s a good sign.

Now I’ll force a small saucer and try it out.

I’m glad I did that. With constant movement of the ship, the small saucer hit me every time, generally with its first shot. That is exactly what we want.

And I got a division by zero crash, which was not what I want but I’m glad to have it, because I really wasn’t expecting that. It occurred on the division calculating the adjustment, with distance_to_target coming back zero. Of course that can happen, because aim_time can return zero, when the answer is no good.

``````    def set_adjustment_ratio(self, aim_time):
if not aim_time:
return
distance_to_target = aim_time*u.MISSILE_SPEED
adjusted_distance = distance_to_target - 2*self._radius
self._adjustment_ratio = adjusted_distance / distance_to_target
``````

That’s a large hammer for a small method but it’ll do the job. I’ll play a bit more just to see if anything else bad happens, then we’ll see if we can devise a better way to do this.

Perfect. The saucer even nailed me at some incredible speed, leading the ship perfectly. Of course it often misses if you’re going fast enough, because the missiles can’t catch up.

Faster missiles would make it even more deadly, but I think the main point here is that this patch actually seems to work. Let’s fix the tests. Changes too obvious to interest you, I promise.

I think this clearly works. The question is whether to commit. I hold that the answer is “yes”, even though the implementation we have is poor. Despite the fact that it involves caching a value and applying it much later, the calculation is, I argue, unquestionably correct, and it is done in a way that ensures that we only adjust actual aimed shots.

Commit: Targeted missiles now adjust for the firing offset and are far more accurate. Code needs improvement.

What is wrong with this code?

I’m glad you asked. First of all, this small change required code changes in four places. A really good change requires changes in one place. Second, the change involves a new member variable, `_adjustment_ratio`, which changes at a rate different from the other members, which never change over the lifetime of the gunner instance. In a really good object, things change at the same rate. (I freely grant that this is not the only object in Asteroids that has this problem. But we’re assessing this one.)

The problem is exacerbated and also made worse by the fact that we mostly do our targeting with points in space, but we do the final calculation of the missile’s velocity using an angle:

``````    def create_targeted_missile(self, from_position, to_position, fleets):
angle = self.angle_to_hit(to_position, from_position)
missile = self.missile_at_angle(from_position, angle)
fleets.append(missile)

def missile_at_angle(self, position, desired_angle):
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(desired_angle) * self._adjustment_ratio
offset = Vector2(2 * self._radius, 0).rotate(desired_angle)
return Missile.from_saucer(position + offset, missile_velocity)

@staticmethod
def angle_to_hit(best_aiming_point, saucer_position):
return Vector2(0, 0).angle_to(best_aiming_point - saucer_position)
``````

We need not do that. We could do it another way. Suppose we did this:

1. Take the difference of `to_position` and `from_position`. That is a vector from `from_position` to `to_position`.
2. Normalize the vector, to a unit vector.
3. Multiple that by `u.MISSILE_SPEED`. Now it is a vector of magnitude MISSILE_SPEED in the correct direction.

Let’s inline some of this and change over to that calculation just to see what the code looks like:

``````    def create_targeted_missile(self, from_position, to_position, fleets):
angle = Vector2(0, 0).angle_to(to_position - from_position)
missile_velocity = Vector2(u.MISSILE_SPEED, 0).rotate(angle) * self._adjustment_ratio
offset = Vector2(2 * self._radius, 0).rotate(angle)
missile = Missile.from_saucer(from_position + offset, missile_velocity)
fleets.append(missile)
``````

Now let’s cast that in terms of velocities.

``````    def create_targeted_missile(self, from_position, to_position, fleets):
vector_to_target = to_position - from_position
direction_to_target = vector_to_target.normalize()
missile_velocity = u.MISSILE_SPEED * direction_to_target
adjusted_velocity = missile_velocity * self._adjustment_ratio
offset = 2 * self._radius * direction_to_target
missile = Missile.from_saucer(from_position + offset, adjusted_velocity)
fleets.append(missile)
``````

Tests are green. (Some of them may be calling those lower-level methods that we are not bypassing. I’m not sure. But the tests did find at least two typo-defects in the code above.)

### Worse to Get Better

The code we had when we started this morning looked fairly decent. Small methods, mostly very direct usage, call down, get answer come back up kind of thing. Our change with the `_adjustment_ratio` showed us that things were not happening in the order we need them.

By inlining two methods, you could argue, I’ve made the code worse. This new `create_targeted_missile` now includes enough steps that, on a given day, I’d say that it should be refactored, extracting a couple of methods to “make it better”.

But when our code isn’t really supporting what we need — and this code isn’t — we often have to make things worse so that we can make them better. I think that’s the case here, and as an expert in making things worse, I feel up to the job.

And remember: we have not committed the current changes and we’re well and truly ready to roll back to our working but somewhat nasty prior commit.

In Retrospect
Committing here would have been OK and not a bad idea in any way that I can now see.

## Break

The session does not end here. The code is not committed. It is, however, working. This article has become huge. Let’s break it up here.

In the next article, we’ll start improving this code.

Take a break, then come on back.

Next in series