Python Asteroids on GitHub

Bill Wake gives me some thoughts about a static method. We’ll look at that, and see what else may need a bit of improvement.

Over on mastodon, the brilliant Bill Wake noted:

Looking at it today, I’d say that quadratic formula should be pulled out - the mathematical part of what you do with a, b, and c. (It could be a method on numbers.) That leaves the calculation of what a/b/c is explicit in ShotOptimizer, and that’s the interesting part.

He’s certainly correct, because he so often is, and we’ll look at that code in a moment. I want to mention a Python / PyCharm thing that comes into play more often than this single case.

PyCharm notices methods on an object that do not reference self, and flags them as “Could be static”. It has been my practice to accept PyCharm’s kind offer to make them static, because it makes a distracting squiggly line go away. I attribute that to my relative inexperience with Python, combined with my amazing ability to develop bad habits.

The thing is, when we find ourselves writing a method that doesn’t reference self, it is commonly a strong indication that the method has nothing to do with the object of which it is a method. Why is it a strong indication? Because in fact the method literally has nothing to do with the object.

This is always a hint that something should be done.

Now, I come from a land where every function must be a method of some object. That’s right, a land where there are no free-standing top level functions. None, zero, not any. So I have a lot of resistance to creating them, and that leaves me, quite often, with nowhere to put a method that doesn’t reference self. So I mark them static and move on.

However, that’s really not legitimate. In principle, a static method is one that you can call referencing the class, not an instance. Except for a very few factory methods, my statics are just marked that way to make the squiggles go away, not because they are really intended to be called on the class.

All this comes down to saying that I do not have in my Python bag of tricks, a well-burnished general approach to things like these three static methods in ShotOptimizer:

class ShotOptimizer:

    @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

    @staticmethod
    def compensate_for_offset(aim_time, initial_offset):
        distance_to_target = aim_time * u.MISSILE_SPEED
        adjusted_distance = distance_to_target - initial_offset
        return adjusted_distance / distance_to_target

    @staticmethod
    def time_to_target(delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        disc = b*b - 4*a*c
        if disc < 0:
            return 0
        divisor = (math.sqrt(disc) - b)
        if divisor == 0:
            return 0
        return 2*c / divisor

Now, taking Bill’s idea first, we can look at the time_to_target method and observe that it clearly has two parts, one computing a, b, and c, and one solving the equation.

Freely Granted
I still do not know why the code is what it is. You would think that a quadratic solution would return “minus b, plus or minus the square root of b squared minus four a c, all over two a”, because that is the incantation we all learned back in kindergarten.

But this stolen equation is returning an entirely different animal. And I just don’t see why it’s doing what it does. If I remain confused about this, I may have to resort to pencil and paper to figure out what’s going on here. I just don’t see 2*c showing up anywhere in the quadratic equation.

Despite my discomfort, I’m going with it, because it works as discovered on the Internet. I’ve tried plugging in the actual formula result and it works if the ship is not moving but not if it is. Weird. I need to read more articles, I guess.

Anyway …

Bill would have me calculate a, b, and c, and then call a generic quadratic solver. The main question for me is where to put it: we can’t go around adding things to existing python files.

If I do the extract and make the new thing a top-level function, we get this:

    @staticmethod
    def time_to_target(delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        return solve_quadratic(a, b, c)


def solve_quadratic(a, b, c):
    disc = b * b - 4 * a * c
    if disc < 0:
        return 0
    # why not: return (-b + math.sqrt(disc))/2*a
    divisor = (math.sqrt(disc) - b)
    if divisor == 0:
        return 0
    return 2 * c / divisor

I’m not fond of that. I could promote the other guy to a top-level, which, in use, gives me this:

    def optimal_shot(self, delta_position, delta_velocity, initial_offset):
        aim_time = time_to_target(delta_position, delta_velocity)
        adjustment_ratio = self.velocity_adjustment(aim_time, initial_offset)
        return aim_time, adjustment_ratio

        
def time_to_target(delta_position, relative_velocity):
    # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
    # return time for hit or 0
    # quadratic
    # Quadratic equation coefficients a*t^2 + b*t + c = 0
    a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
    b = 2 * relative_velocity.dot(delta_position)
    c = delta_position.dot(delta_position)
    return solve_quadratic(a, b, c)


def solve_quadratic(a, b, c):
    disc = b * b - 4 * a * c
    if disc < 0:
        return 0
    # why not: return (-b + math.sqrt(disc))/2*a
    divisor = (math.sqrt(disc) - b)
    if divisor == 0:
        return 0
    return 2 * c / divisor

Honestly, I don’t like that. I wouldn’t like it any better if I were to move those functions over into a utility functions file. Roll this back.

What else might we do? What about a tiny class? Maybe like this:

class ShotOptimizer:
    def optimal_shot(self, delta_position, delta_velocity, initial_offset):
        aim_time = TimeToTarget(delta_position, delta_velocity).time
        adjustment_ratio = self.velocity_adjustment(aim_time, initial_offset)
        return aim_time, adjustment_ratio

class TimeToTarget:
    def __init__(self, delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        disc = b*b - 4*a*c
        if disc < 0:
            self.result = 0
            return
        # why not: self.result = (-b + math.sqrt(disc))/2*a
        divisor = (math.sqrt(disc) - b)
        if divisor == 0:
            self.result = 0
            return
        self.result = 2*c / divisor

    @property
    def time(self):
        return self.result

We could refactor the init a bit … could have a class method instead …

Let’s do break the __init__ up, see if we prefer that:

class TimeToTarget:
    def __init__(self, delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        self.quadratic_formula(a, b, c)

    def quadratic_formula(self, a, b, c):
        disc = b * b - 4 * a * c
        if disc < 0:
            self.result = 0
        else:
            divisor = (math.sqrt(disc) - b)
            if divisor == 0:
                self.result = 0
            else:
                self.result = 2 * c / divisor

I had to inject the else stuff to get PyCharm to do the extract. I could have done it manually. But instead:

class TimeToTarget:
    def __init__(self, delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        self.result = self.quadratic_formula(a, b, c)

    def quadratic_formula(self, a, b, c):
        disc = b*b - 4*a*c
        if disc < 0:
            return 0
        else:
            divisor = (math.sqrt(disc) - b)
            if divisor == 0:
                return 0
            else:
                return 2*c / divisor

Of course now the quadratic_formula method is static again, which is the issue we were trying to resolve.

Maybe like this:

class TimeToTarget:
    def __init__(self, delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        self.result = self.quadratic_formula(a, b, c)

    def quadratic_formula(self, a, b, c):
        disc = b*b - 4*a*c
        if disc < 0:
            return 0
        else:
            return self.calculate(b, c, disc)

    def calculate(self, b, c, disc):
        divisor = (math.sqrt(disc) - b)
        if divisor == 0:
            return 0
        else:
            return 2 * c / divisor

Or maybe:

class TimeToTarget:
    def __init__(self, delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        self.result = self.quadratic_formula(a, b, c)

    def quadratic_formula(self, a, b, c):
        disc = b*b - 4*a*c
        return 0 if disc < 0 else self.calculate(b, c, disc)

    def calculate(self, b, c, disc):
        divisor = (math.sqrt(disc) - b)
        return 0 if divisor == 0 else 2 * c / divisor

I’m going to commit this. I’ve decided that I like it better than without the TimeToTarget class.

Commit: added new TimeToTarget class to contain the quadratic stuff.

A bit more messing about and I accept making two functions top-level. Somehow, inside their own file, I don’t mind so much:

class TimeToTarget:
    def __init__(self, delta_position, relative_velocity):
        # from https://www.gamedeveloper.com/programming/shooting-a-moving-target#close-modal
        # return time for hit or 0
        # quadratic
        # Quadratic equation coefficients a*t^2 + b*t + c = 0
        a = relative_velocity.dot(relative_velocity) - u.MISSILE_SPEED*u.MISSILE_SPEED
        b = 2 * relative_velocity.dot(delta_position)
        c = delta_position.dot(delta_position)
        self.result = quadratic_formula(a, b, c)

    @property
    def time(self):
        return self.result


def calculate(b, c, disc):
    divisor = (math.sqrt(disc) - b)
    return 2 * c / divisor if divisor != 0 else 0


def quadratic_formula(a, b, c):
    disc = b*b - 4*a*c
    return calculate(b, c, disc) if disc >= 0 else 0

We’ll commit: TimeToTarget refactored.

I should mention that my tests for this section of the program are quite solid and have been failing every time I did anything not quite right, so I have high confidence. So high that I’m going to try the game anyway.

As expected, it works just fine, the small saucer kills me right and left.

Let’s reflect and sum up.

Reflection

Bill’s observation about the quadratic was a good one, and while we don’t know what cues he used to get the idea, we can certainly see that since the whole thing had nothing to do with any of the members of ShotOptimizer, it certainly didn’t belong there.

I didn’t like just promoting it to a function. If it could have been added to the math library, or if we had a whole bunch of library functions, maybe, but perhaps the thing I like least about Python is its tendency toward using top-level functions such as

math.sqrt(b*b - 4*a*c)

Instead of

(b*b - 4*a*c).sqrt()

I would find the latter to be more “object oriented” by my admittedly biased lights.

But now that I have my top-level functions tucked away in a small file of their own, with the class that uses them, I don’t mind so much.

I suggest that the larger issue is that I’d do well to listen to the code a bit more carefully, because @staticmethod is kind of whispering “I might not be in the right place”.

Another issue, given that this is Asteroids, an inherently small and simple program, is that we now have these objects involved in shooting missiles at ships: Saucer, Gunner, ShotOptimizer, FiringSolution, TimeToTarget, and Missile. Of these, Missile is the only one that is used outside the Saucer. We have to ask ourselves whether this level of fine-grain object separation really makes sense.

The question of performance might come up, but I think we can lay it back down. There are certainly more method calls in a scheme like this, but they are not costly and the saucer only fires once every half second at most. So speed, probably not a valid concern.

There is the memory issue that game programmers concern themselves with, that led to the Entity-Component-System style of design. It’s used, not so much because the calls are expensive but because they are typically remote in memory from each other, and the result is that the machine pipeline has to stall and access memory from an unexpected location. The articles I read tell me that that is very expensive, and if every cycle counts, it matters.

That, too, is not our concern here. We don’t have a million asteroids, we have maybe 20 on a very bad day. And my wristwatch has far more memory and a far faster processor than the Asteroids console had back in 1979.

So we need to assess this kind of design on some other dimension than performance. It seems to me that it will come down to how easy or hard it is to make changes to this program, and that comes down to how easy or hard it is to understand how the program works.

We’ve talked about this before. There are programmers who like to try to see everything at once, who prefer large objects and large methods that they can scan to find everything. And there are programmers who like to see small single-purpose objects that have a few small methods, well named.

This second group have learned to craft the names well enough so that they can trust the names to tell them whether to explore downward or continue on. Let’s look at an example from the Saucer.

If we’re scanning Saucer, we run across this:

    def fire_if_possible(self, delta_time, fleets):
        self._gunner.fire(delta_time, self, self._ship, fleets)

If we’re interested in how the saucer moves or draws itself, we know not to look inside gunner. If we’re interested in how it fires, we do look inside gunner. And maybe we find this:

    def fire(self, delta_time, saucer, ship_or_none: Ship | None, fleets):
        self._timer.tick(delta_time, self.fire_if_missile_available, saucer, ship_or_none, fleets)

    def fire_if_missile_available(self, saucer, ship_or_none, fleets):
        if result := saucer.missile_tally < u.SAUCER_MISSILE_LIMIT:
            chance = random.random()
            self.fire_available_missile(chance, fleets, saucer, ship_or_none)
        return result

If we’re interested in how it knows how many to fire, we’re directed back to saucer’s missile_tally. If we want to know how it actually fires, we might read on further:

    def fire_available_missile(self, chance, fleets, saucer, ship_or_none):
        if ship_or_none and self.should_target(chance, saucer):
            self.create_optimal_missile(fleets, saucer, ship_or_none)
        else:
            self.create_random_missile(fleets, saucer)

If we’re interested in missiles in general, we’ll read on here and find out what we need. If we care about the missile targeting:

    @staticmethod
    def create_optimal_missile(fleets, saucer, ship):
        target_solution = ShotOptimizer(saucer, ship)
        fleets.append(Missile.from_saucer(target_solution.start, target_solution.velocity))

Reading this, we’re directed to ShotOptimizer. And so on.

I prefer the small object design style, and to be honest, I have found it to be preferred by most of the programmers I know who are better than I am. I hesitate to draw too firm a conclusion from that, but I do admit that I feel it’s a more mature and “better” way to go.

Your experience may be different, and you should do as you see fit. If you’d do me the honor of observing what I do and say, that’s more than I have a right to ask. Even though, deep in my heart, I do have a view on what’s better and worse, you absolutely should do you.

However. I am not going to claim that the current design is ideal. I expect we’ll look at whether those half-dozen objects are just the thing we need. And I’m a bit uncomfortable with the long init methods that take in a bunch of parameters and do the work right in init. There are always things to explore, always things to improve, and always things to try another way just to learn whether we prefer the other way.

Even this morning, I tried quite a few ways, including top level methods, a separate object, ad a few different ways of dividing up the work and expressing the results. I like how it has turned out, and want to turn our attention to the larger picture again soon.

See you next time!