Python Asteroids on GitHub

On our quest to improve SurfaceMaker right out of existence, let’s try a LinePainter object to replace it. Except that I change my mind almost immediately. Notes within on hints that I wasn’t working ideally.

Yesterday, a spike allowed us to draw an Asteroid directly to the screen, instead of creating a surface and blitting the surface to the screen. The simple code looked like this:

    def draw(self, screen):
        raw = raw_rocks[0]
        scaled = [point * self.radius / 4 for point in raw]
        moved = [point + self.position for point in scaled]
        pygame.draw.lines(screen, "white", False, moved, 3 )

As I look at that, it kind of stops me in my tracks. First of all, we could bring that down to fewer lines if we chose to:

    def draw(self, screen):
        adjusted = [(point * self.radius / 4) + self.position for point in (raw_rocks[0])]
        pygame.draw.lines(screen, "white", False, adjusted, 3)

Now, in actual play, we wouldn’t refer to raw_rocks[0]. We’d have a member containing whichever list of points we were assigned for this asteroid.

But however you cut it, it sure looks like drawing an asteroid comes down to a couple of lines of code.

In these little games, Asteroids, Space Invaders, Spacewar, and the like, it has generally been my practice to have an object know how to draw itself. My reasons have probably been twofold. First, drawing the thing has been near the top of my priority list, so that I can see the game evolving. Second, drawing is generally very simple and there’s not much pressure to abstract it away into some additional object.

Separation of Concerns

There is a case to be made against that practice. It’s easier to see if we imagine a much more complicated game. In such a game, a game object may have a great deal of behavior, and its graphical representation might be quite complex. Imagine a game with enemies that can run and hide and use multiple weapons. We might have some developers working on the logic of those enemies, and other developers creating the complex graphics that make them look good on screen.

In the light of a larger program, it becomes more clear that object behavior and object appearance are not quite the same thing, and that while they are related, all the code probably doesn’t belong in a single object.

But here, when it’s always going to come down to a couple of lines of code? It’s far less obvious.

Today’s Preconceptions

When I came in here this morning, I was going to build a LinePainter object that contains the raw points for an asteroid, saucer, or ship, and scales it, positions it, and draws it on the screen. Looking at those few lines of code makes me wonder whether I should do it at all.

Suppose that we do want to convert our Asteroids program to directly draw the lines to the screen rather than blit the surface. There are a few ways to go, including:

  1. Write the necessary code into each object’s draw function.
  2. Do #1 and then refactor to remove duplication. Perhaps discover a nice object.
  3. Invent a new object and plug it in.

I had in mind #3 when I got here. And we could certainly do that. How hard could it be? But that is not my preferred way to work. My preferred way to work is to build the code that I need, keep it running all the time, and refactor to better code. In essence, #2.

So let’s do that. We may still get a LinePainter out of the deal, or we may get something better.

Let’s plan a bit.

Tentative Plan

First, finish up what we have started in Asteroid, which mostly means using different rocks for different asteroids.

Then do Ship, because Ship has an issue that Asteroid and Saucer do not have: the ship rotates. So that will be interesting. It’s so interesting, in fact, that I am almost inclined to do it even before finishing Asteroid.

Meta-Comment
Here’s the first hint that I may be trying to go too fast. You’ll see below that I make some mistakes, perhaps due to haste.

Then, with Ship in place, we can look around to see what might need refactoring. Do it if it seems reasonable.

Meta-Comment
Huh. As I review this before publishing, I see that when I got down to it, I didn’t even give a moment’s thought to refactoring toward separating the concerns. Haste? Or going too fast? You decide.

Then do Saucer and look for refactoring again.

Get Started

We want each Asteroid to have one of the four shapes. The easiest thing I can think of is this:

class Asteroid(Flyer):
    def __init__(self, size=2, position=None):
        self._rock_index = random.randint(0, 3)
        ,,,

    def draw(self, screen):
        adjusted = [(point * self.radius / 4) + self.position for point in (raw_rocks[self._rock_index])]
        pygame.draw.lines(screen, "white", False, adjusted, 3)

Test that. Automated tests are green. Game looks fine. Commit: asteroid now adjusts and draws rocks directly with draw.lines.

Plan the Ship

The ship has two components to its shape, raw_ship_points and raw_flare_points. The former is the ship without its engine flare, and the second is the flare. When accelerating, the ship draws the flare 2/3 of the time when accelerating:

    def draw(self, screen):
        transformed = pygame.transform.rotate(self.select_ship_source(), self._angle)
        if self._drop_in > 1:
            transformed = pygame.transform.scale_by(transformed, self._drop_in)
        half = pygame.Vector2(transformed.get_size()) / 2
        screen.blit(transformed, self.position - half)

    def select_ship_source(self):
        if self._accelerating and random.random() >= 0.66:
            return self._ship_accelerating_surface
        else:
            return self._ship_surface

We note the drop_in, where we scale the ship dynamically to make it appear to drop onto the screen. We’ll have to duplicate that. The drop_in value is a simple scale factor.

The surfaces have been built so that the accelerating surface has both the ship and the flare in one surface. We can deal with the issue with lines by appending the two lists, if nothing else comes to mind. I think Python even has a clever way to do that without actually copying the lists.

What about the rotation? We know that the raw points are centered around the rotation center of the ship, ranging from -7 to 7 in x and -4 to 4 in y. Rotation should be easy in that case.

We’ll put this in incrementally. I’m going to comment out the existing code as a reminder of what needs to be done. At first I’ll just draw the ship.

    def draw(self, screen):
        points = raw_ship_points
        pygame.draw.lines(screen, "white", False, points, 3)

This will be the wrong scale, I’m sure. Probably very tiny. Ha. It probably is but I also forgot to position it. It’s probably up in the corner or something.

    def draw(self, screen):
        points = raw_ship_points
        positioned = [point + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

Now we see the tiny ship:

tiny ship

Looking at the Ship’s surface-making code, I see what the scale needs to be:

    def draw(self, screen):
        points = raw_ship_points
        scale = 4 * u.SCALE_FACTOR
        positioned = [(point * scale) + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

right size ship

That looks good. Now to rotate:

    def draw(self, screen):
        points = raw_ship_points
        scale = 4 * u.SCALE_FACTOR
        positioned = [(point.rotate(-self._angle) * scale) + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

correctly rotated

And we’re done. The 4 is a magic number, but it is already a magic number up in init:

    def __init__(self, position, drop_in=2):
        self.radius = 25 * u.SCALE_FACTOR
        self._accelerating = False
        self._acceleration = u.SHIP_ACCELERATION
        self._allow_freebie = True
        self._angle = 0
        self._asteroid_tally = 0
        self._can_fire = True
        self._drop_in = drop_in
        self._hyperspace_generator = HyperspaceGenerator(self)
        self._hyperspace_timer = Timer(u.SHIP_HYPERSPACE_RECHARGE_TIME)
        self._location = MovableLocation(position, Vector2(0, 0))
        self._missile_tally = 0
        self._shipmaker = None
        ship_scale = 4
        ship_size = Vector2(14, 8)*ship_scale*u.SCALE_FACTOR
        self._ship_surface = SurfaceMaker.ship_surface(ship_size)
        self._ship_accelerating_surface = SurfaceMaker.accelerating_surface(ship_size)

Reading that code reminds me that I didn’t remove the surface creation from Asteroid. Let’s remove from ship, commit, then go back and fix up asteroid.

I’ll remove those last four lines from the init, and commit: ship now draws itself directly to screen.

Then remove the surface creation code from Asteroid and commit: remove unused surface creation.

Saucer

What’s left? Saucer. Let’s see how it creates its surface and uses it. That will help me remember to fix up both sides.

Meta-Comment
Am I beginning to realize here that I am going too fast? Otherwise why would I be worried about remembering? Could I have made better use of this concern, perhaps with a card, some sticky notes or something? A bit more attention to my internal state might have helped me.
    def __init__(self, radius, score, sound, is_small, always_target, scale):
        Saucer.direction = -Saucer.direction
        x = 0 if Saucer.direction > 0 else u.SCREEN_SIZE
        position = Vector2(x, random.randrange(0, u.SCREEN_SIZE))
        velocity = Saucer.direction * u.SAUCER_VELOCITY
        self._directions = (velocity.rotate(45), velocity, velocity, velocity.rotate(-45))
        self._gunner = Gunner(always_target)
        self._location = MovableLocation(position, velocity)
        self._radius = radius * u.SCALE_FACTOR
        self._score = score
        self._ship = None
        self._sound = sound
        self._zig_timer = Timer(u.SAUCER_ZIG_TIME)
        self.is_small_saucer = is_small
        self.missile_tally = 0
        self.missile_head_start = 2*self._radius
        self.create_surface_class_members(scale * u.SCALE_FACTOR)

    @staticmethod
    def create_surface_class_members(drawing_scale):
        raw_dimensions = Vector2(10, 6)
        Saucer.offset = raw_dimensions * drawing_scale / 2
        drawing_size = raw_dimensions * drawing_scale
        Saucer.saucer_surface = SurfaceMaker.saucer_surface(drawing_size)

    def draw(self, screen):
        top_left_corner = self.position - Saucer.offset
        screen.blit(Saucer.saucer_surface, top_left_corner)

The creation parameter scale is 4 for the small saucer, 8 for the large. We don’t save that value but we do have is_small_saucer for testing. I think we’ll save the scale value in init:

        self._scale = scale * u.SCALE_FACTOR
        self.create_surface_class_members(self._scale)

That last line will be gone soon. Let’s draw:

    def draw(self, screen):
        adjusted = [point * self._scale + self.position for point in (raw_saucer_points)]
        pygame.draw.lines(screen, "white", False, adjusted, 3)

That’s all it takes. We might want to optimize this code, which we could do like this:

    def draw(self, screen):
        scale = self._scale
        position = self.position
        adjusted = [point * scale + position for point in raw_saucer_points]
        pygame.draw.lines(screen, "white", False, adjusted, 3)

Doing that saves the references to self. We have no real reason to think that’s necessary, but it may improve readability as well.

This works perfectly. Remove the surface creation stuff. Commit: saucer draws itself directly to screen using lines.

Now I think there should be no references to SurfaceMaker.

On no!!! I’ve made a mistake! The Ship isn’t done yet: it doesn’t display its flare.

Meta-Comment
So now I panic, and do a hot fix and then a correct fix. But do I take a break, slow down, smell the roses? I do not. I press on without doing anything to reduce my too-rapid pace.

Change draw to this:

    def draw(self, screen):
        points = self.select_ship_source()
        scale = 4 * u.SCALE_FACTOR
        positioned = [(point.rotate(-self._angle) * scale) + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

Change select_ship_source from this:

    def select_ship_source(self):
        if self._accelerating and random.random() >= 0.66:
            return self._ship_accelerating_surface
        else:
            return self._ship_surface

To this:

    def select_ship_source(self):
        if self._accelerating and random.random() >= 0.66:
            return raw_ship_points + raw_flare_points
        else:
            return raw_ship_points

Test. Works. Commit: ship flare restored.

OK, good quick fix. It’s only 0730 so maybe no one noticed the bug.

Now let’s avoid the list copy implied by the + above. Let’s just do it in init.

    def __init__(self, position, drop_in=2):
        self.radius = 25 * u.SCALE_FACTOR
        self._accelerating = False
        self._acceleration = u.SHIP_ACCELERATION
        self._allow_freebie = True
        self._angle = 0
        self._asteroid_tally = 0
        self._can_fire = True
        self._drop_in = drop_in
        self._hyperspace_generator = HyperspaceGenerator(self)
        self._hyperspace_timer = Timer(u.SHIP_HYPERSPACE_RECHARGE_TIME)
        self._location = MovableLocation(position, Vector2(0, 0))
        self._missile_tally = 0
        self._shipmaker = None
        self._ship_points = raw_ship_points
        self._accelerating_ship_points = raw_ship_points + raw_flare_points

    def select_ship_source(self):
        if self._accelerating and random.random() >= 0.66:
            return self._accelerating_ship_points
        else:
            return self._ship_points

Commit: compute ship and accelerating ship points in init.

And Oh No!! again: we don’t have drop in yet. Two mistakes. We must think about this, and I even saved the old source as a reminder. I just got too excited about how all this was going.

Meta-Comment
Look at me! I know that I’m going too fast, but do I do anything about it? Not that I can see. Someone should have asked me a question or told me to take a break.
    def draw(self, screen):
        points = self.select_ship_source()
        scale = 4 * u.SCALE_FACTOR
        if self._drop_in > 1:
            scale *= self._drop_in
        positioned = [(point.rotate(-self._angle) * scale) + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

Works. Commit: turn drop-in back on.

Reflection

I forgot at least three things so far:

  1. Didn’t remove the surface creation after Asteroid line drawing worked.
  2. Didn’t show acceleration flare on ship.
  3. Didn’t implement drop-in on ship.

Why, and what might I do to avoid such things in future?

It’s not small commits

It is tempting, oh so tempting, to say that small commits caused this problem. And, certainly if I had at least held off on pushing to GitHub, I might have remembered before doing that. But that’s not a good solution, by my lights, because small commits avoid merge problems, and they keep the code base up to date on all our computers. (All of my one computers, but if I were a team, all of them.)

Notes might help

Since I surely knew that we needed acceleration and drop in, and actually saved the old code as a reminder, maybe writing the issues on a card and lining them out when done would help. If I were to develop that habit, it would help me. I have a couple of cards here and will see if I can develop that habit.

In essence, I got all excited by how easily it was going and got ahead of myself. We could call that carelessness and berate me, but that’s not productive. The question is how we can change our system to cause us to do everything that needs to be done.

What about TODO? PyCharm has a TODO thing, but it’s basically fatal to hit one. You can put a TODO in a comment and get a list of them. I wonder if you can check them on commit. I’ll look into that: it might be a useful thing to, um, do.

Let’s move on and finish up, by removing SurfaceMaker, since it is no longer used.

SurfaceMaker

There are essentially no references to SurfaceMaker class, though there are references to the various raw points lists, which are in the SurfaceMaker.py file. There is a SurfaceMaker test.

I think we should remove all that. I remove the tests. Change three imports, remove the class. Let’s rename the module to raw_object_points.py. Done.

Commit: remove SurfaceMaker, rename module.

Arrgh, renamed wrong module. Do again. All better.

I think I’d best take a break, I seem to be making odd mistakes. There I just mis-clicked the file but then missed some opportunities to notice the problem. Let’s sum up.

Meta-Comment
This may not have a haste-induced error: I clicked the wrong line. But the commit showed files changing that shouldn’t have changed. By the time I noticed, I had already pushed, so I went back and reviewed the changes and realized that I had changed the wrong file name. Haste certainly didn’t help but this may have had more to do with dry eyes.

Summary

We have changed all the drawing for asteroid, saucer, and ship, to draw the lines directly, after transforming them to the right scale and position:

class Asteroid(Flyer):
    def draw(self, screen):
        adjusted = [(point * self.radius / 4) + self.position for point in (raw_rocks[self._rock_index])]
        pygame.draw.lines(screen, "white", False, adjusted, 3)

class Saucer(Flyer):
    def draw(self, screen):
        scale = self._scale
        position = self.position
        adjusted = [point * scale + position for point in raw_saucer_points]
        pygame.draw.lines(screen, "white", False, adjusted, 3)

class Ship(Flyer):
    def draw(self, screen):
        points = self.select_ship_source()
        scale = 4 * u.SCALE_FACTOR
        if self._drop_in > 1:
            scale *= self._drop_in
        positioned = [(point.rotate(-self._angle) * scale) + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

    def select_ship_source(self):
        if self._accelerating and random.random() >= 0.66:
            return self._accelerating_ship_points
        else:
            return self._ship_points

I came in here expecting to build a new object, perhaps to be called LinePainter, with instances that did the drawing for these guys. I imagined that Asteroid and Saucer would have instances of Plain Vanilla Line Painter and Ship might have an instance of Flickering Line Painter, or some such object that managed which of the two drawings to display.

Looking at the simplicity of the Asteroid code induced me to just do everything in line in the various draw functions, and they aren’t all that bad. Ship could be better:

class Ship(Flyer):
    def draw(self, screen):
        points = self.select_ship_source()
        scale = 4 * u.SCALE_FACTOR * self._drop_in
        positioned = [(point.rotate(-self._angle) * scale) + self.position for point in points]
        pygame.draw.lines(screen, "white", False, positioned, 3)

We wanted to skip the drop-in when we were going to scale a surface. In the code now, the if statement saves us a multiply. We can afford that. Commit: simplify drop-in.

Meta-Comment
Have you noticed how often I find something to improve while summing up? I’d say it is at least half the time. Why is that? I think it’s because at summary time, I pull my head up from the nitty-gritty and look at the bigger picture. There’s great value to doing that, and in my view, when we see something while reflecting like this, it’s often wise to fix it then and there. That’s why I generally do fix it then and there, sort of a reinforcement of fixing issues when we see them.

Similarities and Differences

Each of these methods has three parts, selecting what to draw, adjusting it by scale and position, and drawing the lines. One, Saucer, always selects the same list. One, Asteroid, selects one of four lists, decided at create time. One, Ship, selects its point list at display time, showing the flare 2/3 of the time and no flare 1/3, when accelerating.

The Asteroid’s scale is defined at creation time, as is the Saucer’s. We could, in principle, scale them then, making a copy of the points at that scale. The cost would be more memory, since then each object would have its own list of points. For Saucer, no big thing, there is only one saucer at a time. For Asteroid, there can be a lot of those and each one would have its own list of a dozen points. For Ship, the scale changes, in principle, on every draw, since drop-in might be in effect. We could rig it differently but you’re accelerating a lot, so there would be no big savings.

The cost of using the raw points is a loop over the points, involving a multiply and an add, except for Ship, which also includes a vector rotate for every point. On a 6502 chip, we might be concerned about that. On a 21st century computer, we are not. We are OK sticking with using the raw points and scaling right before we draw. We have to add in the position in any case, so all we would save is the multiply for scale. A dozen multiplies? Not worried. A dozen times 20 objects on the screen? Still not worried.

Looking at these draw methods right now, I do not quite see an object trying to be born. But there are those three things going on, selection, adjustment, and drawing. There might be a bit of something trying to hatch.

We’ll let this code perk for a day or so and then see whether there’s something better to be done. For now, we have about the same amount of code in our user classes, and we have removed an entire class, SurfaceMaker, from the system.

I even think that the Ship looks better drawn this way, compared to its rotated look when we rotated the surface. I could be wrong: I frequently am.

Raw Points

One issue is the raw points lists, which are now off in a separate module. That used to make sense when the SurfaceMaker was over there, because it had the only references to those lists, Now each object imports its points from over there. That might still be best, but it is a bit odd. Worthy of thought.

Bottom Line

This change did not go as I had anticipated, and that’s good: I think this is better than creating an object would have been. I did make some irritating small mistakes, which were quickly corrected, but not before they could have been noticed. I’ll try to build up a better habit on that, a bit more awareness of the necessary steps before calling a thing done.

But overall, it’s an interesting simplification. There is a common aspect to all three draw methods, so perhaps there is an abstraction to be made. Right now, I do not see it. Next time, perhaps I will.

See you then!