Python Asteroids on GitHub

Looking at Game, and it seems we can improve quite a bit. Great outcome, nearly perfect!

I was just glancing at Game during an on-line chat. I have a sticky here that says that the method move in Fleets should be named update, so I was looking to see where it is called in the Game, and observed this:

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.asteroids_tick(self.delta_time)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def asteroids_tick(self, delta_time):
        self.control_game()
        self.fleets.move(delta_time)
        self.perform_interactions()
        self.fleets.tick(delta_time)
        self.draw_everything()

    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())

    def perform_interactions(self):
        self.fleets.perform_interactions()

    def draw_everything(self):
        screen = self.screen
        screen.fill("midnightblue")
        self.fleets.draw(screen)
        self.draw_score()

    def draw_score(self):
        pass

We see something there we could remove. Done. Commit: remove unused draw_score.

We could inline perform_interactions. It will change two tests but no big deal there, it’s probably the right thing since the work is in Fleets anyway.

Commit: inline perform_interactions.

Now we have this:

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.asteroids_tick(self.delta_time)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def asteroids_tick(self, delta_time):
        self.control_game()
        self.fleets.move(delta_time)
        self.fleets.perform_interactions()
        self.fleets.tick(delta_time)
        self.draw_everything()

    def draw_everything(self):
        screen = self.screen
        screen.fill("midnightblue")
        self.fleets.draw(screen)

I see two topics here, at least. One is that the draw_everything stands out now as different, and the other is that sending three or four consecutive messages to the same object suggests that the object needs a new method to do those things.

Let’s rearrange things a bit. What I want to end up with is with four calls to self.fleets, passing in the screen with its fill already done: I think the fill belongs to the Game, not to the fleet objects. We might want to do a red game or something.

That would be easy enough to do manually, but let’s see if we can make it easier. Extract:

    def draw_everything(self):
        screen = self.prepare_screen()
        self.fleets.draw(screen)

    def prepare_screen(self):
        screen = self.screen
        screen.fill("midnightblue")
        return screen

Just for fun: Commit: pull out prepare_screen method.

Now … let’s change the signature on asteroids_tick to provide the screen.

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.asteroids_tick(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def asteroids_tick(self, delta_time, screen):
        self.control_game()
        self.fleets.move(delta_time)
        self.fleets.perform_interactions()
        self.fleets.tick(delta_time)
        self.draw_everything()

Commit: pass screen to asteroids_tick.

Now put a call to prepare_screen ahead of the call to asteroids_tick:

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.prepare_screen()
            self.asteroids_tick(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

I didn’t need the return, so ignored it. Commit: prepare_screen in main_loop.

I inline the draw_everything but don’t like the result as much as I’d hoped:

    def asteroids_tick(self, delta_time, screen):
        self.control_game()
        self.fleets.move(delta_time)
        self.fleets.perform_interactions()
        self.fleets.tick(delta_time)
        screen1 = self.prepare_screen()
        self.fleets.draw(screen1)

Meh. Nothing for it. Remove the prepare line and refer to the parameter. I’ll have to do it by hand.

Darn. Took me a mouse click and two or three keystrokes. I am tired now.

    def asteroids_tick(self, delta_time, screen):
        self.control_game()
        self.fleets.move(delta_time)
        self.fleets.perform_interactions()
        self.fleets.tick(delta_time)
        self.fleets.draw(screen)

Commit: asteroids_tick now has four calls to fleets in a row.

Extract:

    def asteroids_tick(self, delta_time, screen):
        self.control_game()
        self.cycle(delta_time, screen)

    def cycle(self, delta_time, screen):
        self.fleets.move(delta_time)
        self.fleets.perform_interactions()
        self.fleets.tick(delta_time)
        self.fleets.draw(screen)

Commit: extract cycle method

I don’t see a way for PyCharm to move this method between classes. Just do it:

class Fleets:
    def cycle(self, delta_time, screen):
        self.move(delta_time)
        self.perform_interactions()
        self.tick(delta_time)
        self.draw(screen)

Replace in Game:

class Game:
    def asteroids_tick(self, delta_time, screen):
        self.control_game()
        self.fleets.cycle(delta_time, screen)

Delete Game.cycle. Commit: use fleets cycle directly.

Rename asteroids_tick to cycle.

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.prepare_screen()
            self.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def cycle(self, delta_time, screen):
        self.control_game()
        self.fleets.cycle(delta_time, screen)

Commit: rename method to cycle.

Inline:

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.prepare_screen()
            self.control_game()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

Commit: inline cycle method.

Notice this:

    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())

Rename, reorder two lines:

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.accept_coins()
            self.prepare_screen()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def accept_coins(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())

Commit: rename control_game to accept_coins.

Smile. Let’s review Game now:

class Game:
    def __init__(self, testing=False):
        self.delta_time = 0
        self.init_pygame_and_display(testing)
        self.fleets = Fleets()
        self.fleets.add_flyer(Coin.slug())

    # noinspection PyAttributeOutsideInit
    def init_pygame_and_display(self, testing):
        if testing:
            return
        pygame.init()
        pygame.mixer.init()
        pygame.mixer.set_num_channels(16)
        player.init_sounds()
        pygame.display.set_caption("Asteroids")
        self.clock = pygame.time.Clock()
        self.screen = pygame.display.set_mode((u.SCREEN_SIZE, u.SCREEN_SIZE))

    def main_loop(self):
        running = True
        while running:
            for event in pygame.event.get():
                if event.type == pygame.QUIT:
                    running = False

            self.accept_coins()
            self.prepare_screen()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def accept_coins(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())

    def prepare_screen(self):
        screen = self.screen
        screen.fill("midnightblue")
        return screen

This seems to me to be nearly good. I don’t entirely love the shape of the main_loop function with that event-eating for loop at the beginning. Maybe we can think of a better way to handle that. Oh. Like this, with an extract:

    def main_loop(self):
        running = True
        while running:
            running = self.quit_if_desired(running)

            self.accept_coins()
            self.prepare_screen()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    @staticmethod
    def quit_if_desired(running):
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                running = False
        return running

We can do better than that. Undo and extract without the parameter. PyCharm does the thing but breaks the code:

    def main_loop(self):
        running = True
        while running:
            running = self.quit_if_desired()

            self.accept_coins()
            self.prepare_screen()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def quit_if_desired(self):
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                running = False
        return running

The return running is squiggled saying “might be referenced before assignment”.

I’ll fix it.

    def main_loop(self):
        running = True
        while running:
            running = self.quit_if_desired()

            self.accept_coins()
            self.prepare_screen()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    def quit_if_desired(self):
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                return False
        return True

Name isn’t good. Rename:

    def main_loop(self):
        running = True
        while running:
            running = self.should_we_keep_running()

            self.accept_coins()
            self.prepare_screen()
            self.fleets.cycle(self.delta_time, self.screen)

            pygame.display.flip()
            self.delta_time = self.clock.tick(60) / 1000
        pygame.quit()

    @staticmethod
    def should_we_keep_running():
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                return False
        return True

I’ll buy that. Commit: extract event checking.

We’ll let that ride. Let’s sum up.

Summary

I think we have just about removed everything Asteroids from Game and, for that matter, from Fleets. (I suspect Fleets could use a new name now, but not today.) The Game just knows two kinds of coin, slugs and quarters, presumably to set up attract mode and a real game.

Aw, there’s still an asteroids clue there:

    def accept_coins(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())

We’ll have to revisit that. Maybe we can set it up so that the Game just inserts a slug, and move the coin insertion there somewhere inside. I’ve made a sticky.

Anyway, we’ve moved all knowledge of how the cycle works from Game and put it in Fleets. That includes the whole cycle, including drawing, because we pass in the screen to the cycle method.

Game is simplified and Fleets is only more complex by one method that calls four others.

The changes were almost all automated. I think I had to resort to actual typing only two or three times. We managed 11 commits in 50 minutes, a new record for me. And I only broke the build once. (I forgot to mention that, but when I did the game_loop renaming, PyCharm renamed something it shouldn’t have, and I didn’t notice that the tests had gone red. Fixed immediately.)

Nearly good, and that’s the way uh-huh uh-huh I like it!

See you next time!