Python Asteroids on GitHub

I see the issue and fix it. An aliasing issue. I haven’t had one of those for a very long time. Still no committed code change but some good ideas. Next time?

Warning
Still no resolution, but some refined thinking and a really interesting discovery with the aliasing. Skim, see if you can see what I’m thinking and why.

It’s 1310, and I now know what the problem is with += and perhaps even how to explain it.

Suppose that we write:

    target = target + something

In the code above, a new object is created with the value target + something and the new object is stored into target. But suppose that instead we write:

    target += something

In this case, target is updated in place.

That would not matter except for what happened earlier in our code:

    def lead_the_target(self, best_target_position, safe_distance, shooter_position):
        # this line is the problem!
        target_position = best_target_position
        for _ in range(3):
            target_position += self.aim_adjustment(
                target_position,
                self.ship.velocity,
                best_target_position,
                shooter_position,
                u.MISSILE_SPEED,
                safe_distance)
        return target_position

When we said

        target_position = best_target_position

it turns out that target_position and best_target_position are identical. target_position is an alias for best_target_position. Therefore, when we update target_position, we are updating best_target_position as well. Since we use that value as part of our calculation, and expect it to be unchanging … it changes and we get wrong answers:

    @staticmethod
    def aim_adjustment(
            initial_aiming_point,
            ship_velocity,
            ship_position,
            saucer_position,
            missile_speed,
            missile_offset):
        missile_travel_distance = saucer_position.distance_to(initial_aiming_point) - missile_offset
        missile_travel_time = missile_travel_distance / missile_speed
        ship_motion = missile_travel_time * ship_velocity
        anticipated_ship_position = ship_position + ship_motion
        delta = anticipated_ship_position - initial_aiming_point
        return delta

We can fix the problem simply enough, like this:

    def lead_the_target(self, best_target_position, safe_distance, shooter_position):
    	# note the `copy` below.
        target_position = copy(best_target_position)
        for _ in range(3):
            target_position += self.aim_adjustment(
                target_position,
                self.ship.velocity,
                best_target_position,
                shooter_position,
                u.MISSILE_SPEED,
                safe_distance)
        return target_position

It would be far better if we could do the protection inside the aim_adjustment function, but it’s too late: if ship_position is aliased, we can’t protect it from the inside.

Note
This seemed like a good idea but didn’t solve the problem. At this writing I do not know how to solve the problem: I think someone somewhere may need to do a copy …

Maybe we should do the copy way down deep in the Flyer’s position property? Hmm … I’ll keep that in mind for next time. Anyway, down a ways we’ll belay this idea, at least for now.

What if we were to pass in the ship instead of the strange velocity / position thing?

I’ll do it incrementally:

    def lead_the_target(self, best_target_position, safe_distance, shooter_position):
        target_position = copy(best_target_position)
        for _ in range(3):
            target_position += self.aim_adjustment(self.ship, target_position, self.ship.velocity, best_target_position,
                                                   shooter_position, u.MISSILE_SPEED, safe_distance)
        return target_position

    @staticmethod
    def aim_adjustment(ship, initial_aiming_point, ship_velocity, ship_position, saucer_position, missile_speed,
                       missile_offset):
        missile_travel_distance = saucer_position.distance_to(initial_aiming_point) - missile_offset
        missile_travel_time = missile_travel_distance / missile_speed
        ship_motion = missile_travel_time * ship.velocity
        anticipated_ship_position = ship_position + ship_motion
        delta = anticipated_ship_position - initial_aiming_point
        return delta

I’m just using ship.velocity for now. I had to fix up the tests to pass in a ship, since they were all just passing in values.

Now next step:

    @staticmethod
    def aim_adjustment(ship, initial_aiming_point, ship_velocity, ship_position, saucer_position, missile_speed,
                       missile_offset):
        missile_travel_distance = saucer_position.distance_to(initial_aiming_point) - missile_offset
        missile_travel_time = missile_travel_distance / missile_speed
        ship_motion = missile_travel_time * ship.velocity
        anticipated_ship_position = ship.position + ship_motion
        delta = anticipated_ship_position - initial_aiming_point
        return delta

Green. I could be committing this, but I’ll hold off a bit.

Note
I’m glad I didn’t. We’ll do better next time. I’ll scrap this work.

Remove two parameters:

    @staticmethod
    def aim_adjustment(ship, initial_aiming_point, saucer_position, missile_speed, missile_offset):
        missile_travel_distance = saucer_position.distance_to(initial_aiming_point) - missile_offset
        missile_travel_time = missile_travel_distance / missile_speed
        ship_motion = missile_travel_time * ship.velocity
        anticipated_ship_position = ship.position + ship_motion
        delta = anticipated_ship_position - initial_aiming_point
        return delta

Two renames in there:

    @staticmethod
    def aim_adjustment(ship, initial_aiming_point, saucer_position, missile_speed, missile_offset):
        missile_travel_distance = saucer_position.distance_to(initial_aiming_point) - missile_offset
        missile_travel_time = missile_travel_distance / missile_speed
        ship_motion = missile_travel_time * ship.velocity
        adjusted_ship_position = ship.position + ship_motion
        adjustment = adjusted_ship_position - initial_aiming_point
        return adjustment

I think we can remove the copy now. No, we really cannot. I think the right thing to do is to use the long form and perhaps leave a note in the code.

But I really like the expressiveness of the +=.

Meh. If that’s the case, then pushing ship down wasn’t useful. And we don’t have a save point. That may be a good thing. I’m rolling back. I do want to improve this code, and I’ve learned a lot about the issues.

And I admit to being a bit bummed not to have made substantial progress in all this time, but the fact is we’re just a quick change away from a decent step if we want to, the change from returning the aiming point to returning the adjustment.

But we’ll save it for next time. And let’s hope that next time I don’t start at oh dark thirty.

See you then!