Python Asteroids on GitHub

Today I’ll keep moving things into the Game object. Probably boring. One Learning: the current_instance trick was useful. Mentioned at the top for your convenience.

One Learning
Shortly after starting the move, I provided a method to save self in a global variable named current_instance. Sort of a poor-man’s Singleton. Then, when a given function got moved inside game, other top-level functions that used to call that directly could be changed to refer to current_instance and still work. That allowed me to move methods and quickly link things back up, so that I could commit after each move.

That may be the only truly good idea I had. But it was a helpful one.

Tedium Continues

I’ll begin by scanning the methods that are already in Game, to see what variables we might be able to bring in as members (fields, I think Pythonistas call them), and what functions should be next.

    def check_ship_spawn(self, ship, ships, delta_time):
        global game_over, ship_timer, ships_remaining
        if ships: return
        if ships_remaining <= 0:
            game_over = True
            return
        ship_timer -= delta_time
        if ship_timer <= 0 and safe_to_emerge(missiles, asteroids):
            ship.reset()
            ships.append(ship)
            ships_remaining -= 1

We see those three global references. They’re all candidates to move into the class.

I should mention that there are a lot of globals out there, 14 right now, and the Game object already has about four fields. That many fields in one object, even a top-level one, is a lot, and it suggests that we may need some consolidation, some refactoring into smaller objects. But unless something jumps out at me, we’ll defer that until we get everything inside. A big picture will surely emerge. For now, we just want to get everything under the covers.

Let’s see who references those globals.

Good news: game_over is referenced in this file 6 times, and in one test. I think that all six of this file’s references are in the class already, except for the global definition itself. This should be easy.

class Game:
    def __init__(self, testing=False):
        if testing: return
        self.game_over = False

Then I’ll search for the others and add self to them. This is facilitated by PyCharm: you can click game_over, Command+B, get a list of references, and dock the list so that you can click through it changing things. Nice.

Yes, I could try a global replace. I resist those for some reason. Probably frightened by one in my youth. I’ve changed the seven, removed the global, and tests are green. Shall I run the game as well? Sure. Works fine, and I’m not surprised. Commit: remove global game_over.

I’ll do another. ship_timer is all in Game. I think I will try a global replace on that one. It went OK, but I had to step manually because of the method set_ship_timer, which matched my pattern. You can probably do a regex in there but let’s not get fancy.

Interesting Bug

The change is made but two tests are failing, saying that Game does not have a ship_timer. That’s because they are creating a game with the testing flag set and:

class Game:
    def __init__(self, testing=False):
        if testing: return
        self.ship_timer = 0
        self.game_over = False
        self.score_font = None
        self.running = False
        self.screen = pygame.display.set_mode((u.SCREEN_SIZE, u.SCREEN_SIZE))
        self.clock = pygame.time.Clock()
        self.delta_time = 0

I think what needs to happen is that we should exit before the screen setting and probably the clock setting. I’ll move those down, and the testing check as well:

    def __init__(self, testing=False):
        self.ship_timer = 0
        self.game_over = False
        self.score_font = None
        self.running = False
        self.delta_time = 0
        if not testing:
            self.screen = pygame.display.set_mode((u.SCREEN_SIZE, u.SCREEN_SIZE))
            self.clock = pygame.time.Clock()

Tests are green. Commit: ship_timer moved into Game.

OK, ships_remaining. Ten usages, three in tests. Nothing to see here, and I’ll try the replace again.

Test tweaking was easy enough. I’m actually rather pleased with how easy it has been so far, mostly a matter of creating a test game instance and referring to it.

Commit: ships_remaining moved into Game.

Continuing

I’ll report in more detail if anything interesting happens. I hope it won’t.

Python wants all your fields defined in the __init__, which I approve of. A lot of them are being defined as None, but I think that’ll be fine, at least they’re in there.

I find that help_lines is defined as a global but not in the top-level list, just in the middle of some code. Nasty. I should know better. I probably panicked.

Observation

I’ve been moving things out of globals and into Game, and if there are references to the globals in functions not yet moved into Game, I prefix the references with current_instance, which is set, no surprise, to the currently running instance. I’m down now to only the asteroids, missiles, and ship collections, and the ship instance in the globals. Once these are done in the fashion just described, it should be easy to move the remaining functions inside Game and replace current_instance with self.

It’s getting easier, and hasn’t been difficult at any point. No credit to me, I have no particular expertise in this kind of thing, moving from global to object. I mostly never put myself into this position. Anyway getting rid of these last few globals should be pretty much the same and then the next phase should be quick and easy.

All In

All the globals are moved inside Game, except, of course, for current_instance. We could make that a class variable but our actual plan is to get rid of it entirely.

I think that I can now move any of the outside functions inside game and only need to change the references to it to self or current_instance as may be. There may be a more clever way to do this but I think doing the current instance thing will make the final step safe and easy.

I’ve moved an easy one, safe_to_emerge. Now a more tricky one, I’ll see about moving the check_collisions and its subordinates.

Nota Bene
I’m noticing a lot of could-be-static methods, which tells me that they may not really belong in Game. We’ll think about that … later.

The first two methods move easily. Three. Four. Five. Six. Seven. Eight.

All that’s left are the draw functions. This is going smoothly, just rather boring and a bit of nitty-gritty detail.

These guys are all only called once, so it should be simple enough. I think I’ll move them all and clean them up all in one go. Nothing could go wrong, I’m sure.

That actually went well. Commit: moved all the drawing. Everything moved.

Replace all the current_instance references to self. Find the references to the setter and remove those, remove the setter. We are free, free at last!

Commit: All top level functions in game.py converted to methods.

Let’s review, reflect, and rest.

R & R, & R

All in all, the conversion to the Game object went smoothly, but it took a long time, seemingly because I wanted to do it incrementally. I suspect, however, that had I tried to do it all at once it would have taken longer, because Python isn’t great at noticing certain references to things that don’t exist. I think I’d have had a lot more debugging to do. Anyway, we’ll never know, I did it this way and it went well.

The bug, and it was in me not in the code, was that I let the top-level functions just propagate and propagate. I was sort of thinking that the original game was all top-level functions, being written in assembler and all, and that it was OK. And, yes, it was OK. I think this is better. YMMV.

We have a lot of members in Game now:

    def __init__(self, testing=False):
        self.asteroids = []
        self.asteroids_in_this_wave = None
        self.delta_time = 0
        self.game_over = False
        self.game_over_pos = None
        self.game_over_surface = None
        self.help_lines = None
        self.missiles = []
        self.running = False
        self.score_font = None
        self.ship = Ship(pygame.Vector2(u.SCREEN_SIZE / 2, u.SCREEN_SIZE / 2))
        self.ship_timer = 0
        self.ships = []
        self.ships_remaining = 0
        self.wave_timer = u.ASTEROID_TIMER_STOPPED
        if not testing:
            self.clock = pygame.time.Clock()
            self.screen = pygame.display.set_mode((u.SCREEN_SIZE, u.SCREEN_SIZE))

I think that’s all there are. I’ll check the warnings to see if there are others not reflected in the init.

There are some pretty obvious classifications here, such as drawing surfaces, positions; game-level state; space objects; timers. Those may be hinting at objects that we should have. Maybe there should be a display object that inits the surfaces and draws all the things somehow. And so on.

    def draw_available_ships(self):
        ship = Ship(Vector2(20, 100))
        ship.angle = 90
        for i in range(0, self.ships_remaining):
            self.draw_available_ship(i, ship)

    def draw_available_ship(self, ship_number, ship):
        ship.position += Vector2(35, 0)
        ship.draw(self.screen)

PyCharm tells me that ship_number isn’t used. That code can be simplified:

    def draw_available_ships(self):
        ship = Ship(Vector2(20, 100))
        ship.angle = 90
        for i in range(0, self.ships_remaining):
            self.draw_available_ship(ship)

    def draw_available_ship(self, ship):
        ship.position += Vector2(35, 0)
        ship.draw(self.screen)

Slightly better. Maybe something else should be done.

I realize that I’ve been at this for around three hours, which is a lot all in a row like that. I’ll call it a morning and assess where we are next time. For sure, we’ve accomplished the intended Big Thing, moving to a game object.

One thing added to “Jira”: u.score -> Game. The score is kept in u and should be moved to Game.

See you next time!