Python Asteroids on GitHub

What shall we do now? Think about splitting Gunner? Discover something different in there? Create a debug keystroke? Why not all the above?

Let’s start by reviewing Gunner. It’s a nice little object that has improved the cohesion of Saucer, but I have the impression that there’s a bit of a “seam” in it, between a part that deals with firing logic and a part that deals with geometry. Here is Gunner:

class Gunner:
    def __init__(self, saucer_radius=20):
        self._timer = Timer(u.SAUCER_MISSILE_DELAY)
        self._radius = saucer_radius

    def fire(self, delta_time, saucer, ship_or_none: Ship | None, fleets):
        if ship_or_none:
            ship_position = ship_or_none.position
        else:
            ship_position = self.random_position()
        self._timer.tick(delta_time, self.fire_missile, saucer, ship_position, fleets)

    def random_position(self):
        return Vector2(self.random_coordinate(), self.random_coordinate())

    @staticmethod
    def random_coordinate():
        return random.randrange(0, u.SCREEN_SIZE)

    def fire_missile(self, saucer, ship_position, fleets):
        if saucer.missile_tally >= u.SAUCER_MISSILE_LIMIT:
            return
        should_target = random.random()
        self.select_missile(fleets, saucer, ship_position, should_target)

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if ship_position and should_target <= u.SAUCER_TARGETING_FRACTION:
            self.create_targeted_missile(saucer.position, ship_position, fleets)
        else:
            random_angle = random.random()
            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 create_targeted_missile(self, from_position, to_position, fleets):
        best_aiming_point = self.best_aiming_point(from_position, to_position, u.SCREEN_SIZE)
        angle = self.angle_to_hit(best_aiming_point, from_position)
        missile = self.missile_at_angle(from_position, angle, Vector2(0, 0))
        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)

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

    def best_aiming_point(self, shooter_position, target_position, wrap_size):
        nearest_x = self.nearest(shooter_position.x, target_position.x, wrap_size)
        nearest_y = self.nearest(shooter_position.y, target_position.y, wrap_size)
        return Vector2(nearest_x, nearest_y)

    @staticmethod
    def nearest(shooter_coord, target_coord, screen_size):
        #     Handy Diagram
        #  ______|______|______
        #   T      T---S++T
        # Central T is too far away.
        # We are to his right, so
        # we shoot toward the right!
        direct_distance = abs(target_coord - shooter_coord)
        if direct_distance <= screen_size / 2:
            return target_coord
        elif shooter_coord > target_coord:
            return target_coord + screen_size
        else:
            return target_coord - screen_size

The methods are more or less in order from top-most, followed by the methods called from there, then the ones deeper down. If we look at the last two, best_aiming_point and nearest, we can certainly see that those are not like the top ones. Those two methods really just deal with coordinates. best_ accepts two-dimensional coordinates, and nearest deals with one dimension at a time.

angle_to is purely geometric, returning the angle to one point to another.

The break seems to be between select_missile, which mostly decides what kind of missile we want, and the two methods it calls, create_targeted_missile and create_random_missile.

It occurs to me that there’s a place where we create a random target:

    def fire(self, delta_time, saucer, ship_or_none: Ship | None, fleets):
        if ship_or_none:
            ship_position = ship_or_none.position
        else:
            ship_position = self.random_position()
        self._timer.tick(delta_time, self.fire_missile, saucer, ship_position, fleets)

Maybe we should always fire at a point, rather than have two ways of aiming.

What if we changed this:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if ship_position and should_target <= u.SAUCER_TARGETING_FRACTION:
            self.create_targeted_missile(saucer.position, ship_position, fleets)
        else:
            random_angle = random.random()
            self.create_random_missile(random_angle, saucer.position, saucer.velocity, fleets)

To this:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if ship_position and should_target <= u.SAUCER_TARGETING_FRACTION:
            self.create_targeted_missile(saucer.position, ship_position, fleets)
        else:
            random_target = self.random_position()
            self.create_targeted_missile(saucer.position, random_target, fleets)

The only discernible difference is that the saucer missiles now all come out at the same speed: if it fires toward its rear, the missile moves away rapidly rather than slowly. Despite that at least one user has noticed that behavior, the game seems perfectly fine.

Let’s push this idea a bit.

Observation
This isn’t what we had in mind a moment ago, which had to do with simplifying Gunner by splitting it up. That’s OK. We’re here to improve the code, not to put in a given feature. So if we improve it, we improve it.
    def select_missile(self, fleets, saucer, ship_position, should_target):
        if ship_position and should_target <= u.SAUCER_TARGETING_FRACTION:
            pass
        else:
            ship_position = self.random_position()
        self.create_targeted_missile(saucer.position, ship_position, fleets)

Remove duplication. Now let’s invert that if:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if not ship_position or should_target > u.SAUCER_TARGETING_FRACTION:
            ship_position = self.random_position()
        else:
            pass
        self.create_targeted_missile(saucer.position, ship_position, fleets)

PyCharm did that for me. Now remove the useless else.

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if not ship_position or should_target > u.SAUCER_TARGETING_FRACTION:
            ship_position = self.random_position()
        self.create_targeted_missile(saucer.position, ship_position, fleets)

We can remove create_random_missile, and the test that calls it. I’ll comment the test for now: I might still want the test for some reason.

Now suppose we want that speed effect for our random missiles. Note how missile_at_angle works:

    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)

Before these changes, when we wanted a targeted missile, we passed a zero vector to velocity_adjustment, and for the random ones we passed saucer.velocity. That’s what makes them speed up or slow down depending on which way we’re firing. Let’s pass that value down into create_targeted_missile depending on what we are doing. We have these methods:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if not ship_position or should_target > u.SAUCER_TARGETING_FRACTION:
            ship_position = self.random_position()
        self.create_targeted_missile(saucer.position, ship_position, fleets)

    def create_targeted_missile(self, from_position, to_position, fleets):
        best_aiming_point = self.best_aiming_point(from_position, to_position, u.SCREEN_SIZE)
        angle = self.angle_to_hit(best_aiming_point, from_position)
        missile = self.missile_at_angle(from_position, angle, Vector2(0, 0))
        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)

Change Signature create_targeted_missile to receive a velocity adjustment.

    def create_targeted_missile(self, from_position, to_position, velocity_adjustment, fleets):
        best_aiming_point = self.best_aiming_point(from_position, to_position, u.SCREEN_SIZE)
        angle = self.angle_to_hit(best_aiming_point, from_position)
        missile = self.missile_at_angle(from_position, angle, Vector2(0, 0))
        fleets.add_flyer(missile)

I have shifted from a sort of spike mode, see what happens if we do this, to committed to do it. I can see this is going to work fine, but let’s complete the job before we commit. A bit risky but I don’t think I need a save point.

Why not branch?
Why don’t I work on a branch, you ask? Because if I work on a branch, I can keep the branch open for as long as I wish, and that could easily extend over more than one session. I do what we call “trunk-based development”. I never branch, always commit to main.

Doesn’t that get you in trouble when you are working on a team? Not if you commit frequently, and if you don’t commit frequently, branches won’t help you. In my opinion. I know that some disagree, and that’s OK.

We change the method to use the new parameter:

    def create_targeted_missile(self, from_position, to_position, velocity_adjustment, fleets):
        best_aiming_point = self.best_aiming_point(from_position, to_position, u.SCREEN_SIZE)
        angle = self.angle_to_hit(best_aiming_point, from_position)
        missile = self.missile_at_angle(from_position, angle, velocity_adjustment)
        fleets.add_flyer(missile)

However, the new parameter is always Vector2(0, 0) here:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if not ship_position or should_target > u.SAUCER_TARGETING_FRACTION:
            ship_position = self.random_position()
        self.create_targeted_missile(saucer.position, ship_position, Vector2(0, 0), fleets)

We’re here to change that. Curiously, the best path is to re-invert that if. Who knew?

PyCharm happily provides this:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if ship_position and should_target <= u.SAUCER_TARGETING_FRACTION:
            pass
        else:
            ship_position = self.random_position()
        self.create_targeted_missile(saucer.position, ship_position, Vector2(0, 0), fleets)

We provide this:

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if ship_position and should_target <= u.SAUCER_TARGETING_FRACTION:
            velocity_adjustment = Vector2(0, 0)
        else:
            velocity_adjustment = saucer.velocity
            ship_position = self.random_position()
        self.create_targeted_missile(saucer.position, ship_position, velocity_adjustment, fleets)

Green. Now we add in the saucer velocity except on targeted missiles, just as before.

Commit: simplify Gunner to always target some point.

No, wait. Let’s review this. We already have a random position if we didn’t get the ship:

    def fire(self, delta_time, saucer, ship_or_none: Ship | None, fleets):
        if ship_or_none:
            ship_position = ship_or_none.position
        else:
            ship_position = self.random_position()
        self._timer.tick(delta_time, self.fire_missile, saucer, ship_position, fleets)

select_missile doesn’t have to check ship_position, and we don’t have to randomize it. It’s always there, possibly already randomized. We can do this:

    def fire_missile(self, saucer, ship_position, fleets):
        if saucer.missile_tally >= u.SAUCER_MISSILE_LIMIT:
            return
        should_target = random.random()
        self.select_missile(fleets, saucer, ship_position, should_target)

    def select_missile(self, fleets, saucer, ship_position, should_target):
        if should_target <= u.SAUCER_TARGETING_FRACTION:
            velocity_adjustment = Vector2(0, 0)
        else:
            velocity_adjustment = saucer.velocity
        self.create_targeted_missile(saucer.position, ship_position, velocity_adjustment, fleets)

This suffices. But now why bother with should_target? Inline that, it’ll be clear enough.

    def fire_missile(self, saucer, ship_position, fleets):
        if saucer.missile_tally >= u.SAUCER_MISSILE_LIMIT:
            return
        self.select_missile(fleets, saucer, ship_position)

    def select_missile(self, fleets, saucer, ship_position):
        if random.random() <= u.SAUCER_TARGETING_FRACTION:
            velocity_adjustment = Vector2(0, 0)
        else:
            velocity_adjustment = saucer.velocity
        self.create_targeted_missile(saucer.position, ship_position, velocity_adjustment, fleets)

Better. But of course we can do this:

    def fire_missile(self, saucer, ship_position, fleets):
        if saucer.missile_tally < u.SAUCER_MISSILE_LIMIT:
            self.select_missile(fleets, saucer, ship_position)

Green. Commit: simplifying Gunner by always targeting some (possibly random) point.

I had commented out the creation of WaveMaker in quarter, so that I could more easily watch the saucer and its missiles. I forgot and committed that, and then of course fixed it immediately. I think I want a way to run without asteroids. Let’s do that. We have a sticky somewhere for Debug Keys. Or did I tear that up?

We have this code:

    def control_game(self):
        keys = pygame.key.get_pressed()
        if keys[pygame.K_q]:
            self.fleets.add_flyer(Coin.quarter())

Let’s have a special key for running without asteroids:

    def control_game(self):
        keys = pygame.key.get_pressed()
        if keys[pygame.K_q]:
            self.fleets.add_flyer(Coin.quarter())
        elif keys[pygame.K_a]:
            self.fleets.add_flyer(Coin.no_asteroids())

And in Coin:

class Coin(Flyer):
    @classmethod
    def quarter(cls):
        return cls(True, True)

    @classmethod
    def slug(cls):
        return cls(False, True)

    @classmethod
    def no_asteroids(cls):
        return cls(True, False)

    def __init__(self, is_quarter=True, want_asteroids=True):
        self.is_quarter = is_quarter
        self.want_asteroids = want_asteroids

    def tick(self, delta_time, fleets):
        fleets.clear()
        fleets.add_flyer(SaucerMaker())
        fleets.add_flyer(ScoreKeeper())
        fleets.add_flyer(Thumper())
        if self.want_asteroids:
            fleets.add_flyer(WaveMaker())
        fleets.add_flyer(ShipMaker() if self.is_quarter else GameOver())

Close enough. Commit: add secret ‘a’ key to play without asteroids. not much fun really.

Back to Gunner. We should note that what we’ve done here is not quite a refactoring. We have changed the behavior of the code just a bit. When we started, all random missiles had the Saucer’s velocity added to them. Now, if the ship is absent, we roll up a random target and if the targeting roll also hits, the missile will be fired without adding velocity into its base velocity. There’s no noticeable difference in play. But strictly speaking this was not a refactoring.

Let’s review things one more time.

class Gunner:
    def __init__(self, saucer_radius=20):
        self._timer = Timer(u.SAUCER_MISSILE_DELAY)
        self._radius = saucer_radius

    def fire(self, delta_time, saucer, ship_or_none: Ship | None, fleets):
        if ship_or_none:
            ship_position = ship_or_none.position
        else:
            ship_position = self.random_position()
        self._timer.tick(delta_time, self.fire_missile, saucer, ship_position, fleets)

    def fire_missile(self, saucer, ship_position, fleets):
        if saucer.missile_tally < u.SAUCER_MISSILE_LIMIT:
            self.select_missile(fleets, saucer, ship_position)

    def select_missile(self, fleets, saucer, ship_position):
        if random.random() <= u.SAUCER_TARGETING_FRACTION:
            velocity_adjustment = Vector2(0, 0)
        else:
            velocity_adjustment = saucer.velocity
        self.create_targeted_missile(saucer.position, ship_position, velocity_adjustment, fleets)

    def create_targeted_missile(self, from_position, to_position, velocity_adjustment, fleets):
        best_aiming_point = self.best_aiming_point(from_position, to_position, u.SCREEN_SIZE)
        angle = self.angle_to_hit(best_aiming_point, from_position)
        missile = self.missile_at_angle(from_position, angle, velocity_adjustment)
        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)

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

    def best_aiming_point(self, shooter_position, target_position, wrap_size):
        nearest_x = self.nearest(shooter_position.x, target_position.x, wrap_size)
        nearest_y = self.nearest(shooter_position.y, target_position.y, wrap_size)
        return Vector2(nearest_x, nearest_y)

    @staticmethod
    def nearest(shooter_coord, target_coord, screen_size):
        #     Handy Diagram
        #  ______|______|______
        #   T      T---S++T
        # Central T is too far away.
        # We are to his right, so
        # we shoot toward the right!
        direct_distance = abs(target_coord - shooter_coord)
        if direct_distance <= screen_size / 2:
            return target_coord
        elif shooter_coord > target_coord:
            return target_coord + screen_size
        else:
            return target_coord - screen_size

    def random_position(self):
        return Vector2(self.random_coordinate(), self.random_coordinate())

    @staticmethod
    def random_coordinate():
        return random.randrange(0, u.SCREEN_SIZE)

These last two methods have nothing to do with Gunners, targeting, or missiles. They are specialized random number functions. If we had an object like ScreenPoint, they could be class methods on that. Maybe we should have such a thing. We are presently using Vector2 to mean both position and velocity, despite that those are quite different.

The best_aiming_point and nearest methods might well be methods on a ScreenPoint, come to think of it. And angle_to might fit in there as well.

The seam between missile thinking and geometry thinking does seem to be somewhat real. Perhaps we should have a more robust object or even two, to manage our positions and velocities. Then again, perhaps we should do something more useful, like a small saucer or some new game feature.

We’re here to practice, not to build a product

I freely grant that these changes may not be “worth it” in terms of the time I spend on them, from the viewpoint of actual improvement to the game. We’re not here to write a game, however. We’re here to see what kinds of improvements we can make to code, building up understanding and skills to use in our day-to-day work.

Or, we’re just messing around. That’s fine too.

See you next time!