Python Asteroids on GitHub

We’ve improved the SurfaceManager a lot. We’ll do a quick review of that, and then see what we’ve missed. Why? It’s good for us and for our organization. Late note: Ron was too clever.

Yesterday, in #197, we used a series of very small changes, mostly automated, to make our three somewhat similar surface creation methods into four methods that were so similar that, except for data values, they could all be handled by one method.

This is nearly good, but as we’ll see, it can be better.

Why?

Why do I do this? Well, I do it because I enjoy it, and because making myself aware of subtle differences and small opportunities makes my code better. Why might you do something similar? Perhaps for the same reasons. You might do it from time to time in your production code; you might do it with fun code like mine, if you lean that way. You might do it alone, and you might benefit even more from doing it with someone else.

Cui bono?

What is the benefit? Well, aside from making us better programmers, which is good for us and good for our organization, refactoring our production code to be better allows us to go faster than we would otherwise go.

Here’s why.

Suppose we wanted to add a new visible object to the Asteroids game. Maybe a ringed planet,or a fancy sun, or a helper alien space creature, anything that needs a drawing surface created. As things were before yesterday, to set up a new surface, we would have to understand code like the following, and how to create something similar for our new object:

class SurfaceMaker:
    @staticmethod
    def asteroid_surface(actual_size):
        raw_rock_points = SurfaceMaker.get_next_shape()
        raw_points_span = 8
        raw_points_offset = Vector2(4, 4)
        scale = actual_size / raw_points_span
        room_for_fat_line = 2
        surface_size = actual_size + room_for_fat_line
        surface = SurfaceMaker.create_scaled_surface((surface_size, surface_size),
                                                     raw_points_offset, scale, raw_rock_points)
        return surface

Now, we have to understand this:

    @staticmethod
    def asteroid_surface(object_size: Vector2):
        points_to_draw = SurfaceMaker.get_next_shape()
        room_for_fat_line = Vector2(2, 2)
        raw_points_span = Vector2(8, 8)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

That’s easier to figure out and it’s far easier to produce a new one for our Space Goblin.

And yet …

But it’s not totally obvious. What is raw_points_span? Well, you and I know that it is a hand-crafted pair containing the width and height of the surface that will just contain the lines drawn in the points_to_draw. Well, almost … because in the case of the ship, it’s actually the span of both segments of the ship, its body and its flare.

That is obscure, undocumented, unexplained, not obvious, and quite error-prone. In addition, it’s not at all clear what it is.

Let’s see if we can make it more clear and easier to create.

What is this thing anyway? It is a vector containing the width and height of a rectangle that can just contain the points in the corresponding points_to_draw. Let’s look at an asteroid:

[
Vector2(4.0, 2.0), Vector2(3.0, 0.0), Vector2(4.0, -2.0),
Vector2(1.0, -4.0), Vector2(-2.0, -4.0), Vector2(-4.0, -2.0),
Vector2(-4.0, 2.0), Vector2(-2.0, 4.0), Vector2(0.0, 2.0),
Vector2(2.0, 4.0), Vector2(4.0, 2.0),
],

The span for this is (8, 8). Why? Because x ranges from -4 to 4 and y ranges from -4 to 4, so the asteroid will fit in a rectangle that is (4 - (-4), 4 - (-4)) or (8, 8).

We could compute that. Except for the ship, which has two shapes:

raw_ship_points = [Vector2(-3.0, -2.0), Vector2(-3.0, 2.0), Vector2(-5.0, 4.0),
                   Vector2(7.0, 0.0), Vector2(-5.0, -4.0), Vector2(-3.0, -2.0)]
raw_flare_points = [Vector2(-3.0, -2.0), Vector2(-7.0, 0.0), Vector2(-3.0, 2.0)]

To get a rectangle that will contain both of those, we have to consider them together, to observe that x varies from -7 to 7.

We can code something up that computes these rectangles and then when the SpaceGoblin comes along, it’ll be more obvious how to set up the right span.

I don’t see a really clever way to do this, so let’s TDD up a simple way to do it.

    def test_span(self):
        points = [Vector2(-3, 9), Vector2(2, 5)]
        span = SurfaceMaker.span(points)
        assert span == Vector2(5, 4)

Now let me code span. I’ll just keep the mins and maxes and difference them, I guess.

class SurfaceMaker:
    @staticmethod
    def span(points):
        max_x = max([point.x for point in points])
        max_y = max([point.y for point in points])
        min_x = min([point.x for point in points])
        min_y = min([point.y for point in points])
        return Vector2(max_x - min_x, max_y - min_y)

Test passes.

Let’s see about using our new span thing, here:

    @staticmethod
    def asteroid_surface(object_size: Vector2):
        points_to_draw = SurfaceMaker.get_next_shape()
        room_for_fat_line = Vector2(2, 2)
        raw_points_span = Vector2(8, 8)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

We can do this:

    @staticmethod
    def asteroid_surface(object_size: Vector2):
        points_to_draw = SurfaceMaker.get_next_shape()
        room_for_fat_line = Vector2(2, 2)
        raw_points_span = SurfaceMaker.span(points_to_draw)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

And having done that, we should move the line up:

    @staticmethod
    def asteroid_surface(object_size: Vector2):
        points_to_draw = SurfaceMaker.get_next_shape()
        raw_points_span = SurfaceMaker.span(points_to_draw)
        room_for_fat_line = Vector2(2, 2)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

Nice. Commit: asteroid computes points span rather than use literal.

We can do saucer similarly:

    @staticmethod
    def saucer_surface(object_size: Vector2):
        points_to_draw = raw_saucer_points
        raw_points_span = SurfaceMaker.span(points_to_draw)
        room_for_fat_line = Vector2(0, 2)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

However, the other two methods have to work differently, because they only consider part of the full ship drawing:

    @staticmethod
    def accelerating_surface(object_size: Vector2):
        points_to_draw = raw_flare_points
        room_for_fat_line = Vector2(0, 0)
        raw_points_span = Vector2(14, 8)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

    @staticmethod
    def ship_surface(object_size: Vector2):
        points_to_draw = raw_ship_points
        room_for_fat_line = Vector2(0, 0)
        raw_points_span = Vector2(14, 8)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

Let’s do this:

    @staticmethod
    def accelerating_surface(object_size: Vector2):
        points_to_draw = raw_flare_points
        raw_points_span = SurfaceMaker.span(raw_ship_points + raw_flare_points)
        room_for_fat_line = Vector2(0, 0)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

    @staticmethod
    def ship_surface(object_size: Vector2):
        points_to_draw = raw_ship_points
        raw_points_span = SurfaceMaker.span(raw_ship_points + raw_flare_points)
        room_for_fat_line = Vector2(0, 0)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

Oops!

In testing this in the game, I realize that there’s a problem. When accelerating, the ship flickers. The reason is that we are blitting the acceleration picture on top of the ship picture, and as coded, the acceleration picture does not include the ship, so it erases it.

I “fixed” that yesterday, causing the flicker. Let’s fix it again, only correctly. The accelerating version must display both ship and flare:

    @staticmethod
    def accelerating_surface(object_size: Vector2):
        points_to_draw = raw_ship_points + raw_flare_points
        raw_points_span = SurfaceMaker.span(points_to_draw)
        room_for_fat_line = Vector2(0, 0)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

There we go. Now I think we need something to make it more clear why this one is different:

    @staticmethod
    def ship_surface(object_size: Vector2):
        points_to_draw = raw_ship_points
        raw_points_span = SurfaceMaker.span(raw_ship_points + raw_flare_points)
        room_for_fat_line = Vector2(0, 0)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

The Future Programmer is going to look at that and wonder why, unlike all the others, this call to span considers a list that’s different from the points to draw. How can we help them? We could add a comment. We could extract a meaningful temp variable.

    @staticmethod
    def ship_surface(object_size: Vector2):
        points_to_draw = raw_ship_points
        points_to_span = raw_ship_points + raw_flare_points
        raw_points_span = SurfaceMaker.span(points_to_span)
        room_for_fat_line = Vector2(0, 0)
        return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)

This should at least make it clear that we really intend this. You might want to add a comment: I choose not to do that. I could be wrong: I frequently am.

Commit: ship and accelerating surfaces use span.

Summary

What have we accomplished here? I think we have accomplished two things:

  1. We have automated calculation of the size of the surface, so that it is no longer subject to human error.
  2. We have made creation of a surface more automatic, so that Future Programmer will find it easier to do.

What have we not accomplished?

SpaceGoblin

Suppose that the SpaceGoblin is supposed to animate, like the aliens in Space Invaders™. Like the ship and its flare, there might be two or more different drawings for the goblin, and it might be necessary to “sum them up”, as we did with the ship and its flare, to get the right surface size.

We have not made it clear quite how to do that. Yes, ship and flare are an example, but they’re not as explicit as they might be. Is that worth doing for this Asteroids program? No. But in a large game application where many game objects might be created with more than one picture, improving how we do it could pay off.

Coupling

We have a code-level coupling between the points that define a shape, and the span for that shape. In our code to use the points, we have to call span. The points themselves are just a list of points, not an object of any notable asteroid-related intelligence.

Quite likely, it would be “better”, in our usual sense of gilding the lily to learn things, if the shapes were objects of our own invention, which would include their points lists and would compute their own spans.

Less likely but still likely, it might be “better” if we had a Simple version of that object, and an Animated version, to accommodate the ship plus flare and the 13 different configurations of SpaceGoblin.

If we did those things, the coupling between points and span would be captured inside this new object and not explicit in the four places it now occurs. It would occur only inside the new drawing thing.

Fascinating?

I don’t mind saying that I am amazed and fascinated by how much material we can find in this tiny program. Again and again we find issues that are just like those we might find in a larger program, and the improvements we make are much the same.

And with the refactoring tools of an IDE like PyCharm, the changes are generally easy to make even if they happen to affect more than one file.

That said … when a change affects more than one file (other than the object and its test file), it is a sign of coupling, and suggests that there is something going on that could be improved. In a larger program, we might encounter more such issues, and we might not be willing to improve all the related modules in one go. When that’s the case, we pay additional prices, including the need for feature flags, the necessity to deprecate methods, or perhaps even more nasty workarounds.

And sometimes we fear to make the change at all, and all the Future Programmers pay the price of a slightly more difficult job. And, sometimes, that’s the right decision.

In my view, far more often than not, doing the code improvement pays off. But your situation is not mine, you are not me, no matter how much you wish you were, and you have to do as you see fit. What we have here is examples of what happens when we do things, for you to observe, laugh at, and learn.

But I want to underline that in my view, this kind of practice improves us as programmers, which is good for our organization and for ourselves. It improves the production code, which makes us more productive, again good for organization and self. Continuous code improvement is good for us all.

Today … we have again made the program a bit better. And that pleases us.

See you next time!

Hey, Wait!
Why am I setting up the room_for_fat_line so carefully? Why not just always give myself two extra pixels in x and y? It’s not like we’re going to go broke because of extra pixels. The whole clever idea is a mistake, we should just do it down in surface creation.

We’ll do that next time. Or not.