Flappy Bird game clone for a beginners' programming class

Question

I'll soon begin teaching a beginners' programming class. It's voluntary, so I thought I'd make it interesting by teaching Python programming and then introduce the kids to Pygame, so that they can make their own games. To try out Pygame (I've never used it before) and to see exactly how easy it is to make a game, I made a clone of Flappy Bird.

What do you think? Could this be made simpler/shorter? Is there anything in there that I shouldn't teach my students?

Github repo, with images

#!/usr/bin/env python3
"""Flappy Bird, implemented using Pygame."""
import math
import os
from random import randint
import pygame
from pygame.locals import *
FPS = 60
EVENT_NEWPIPE = USEREVENT + 1 # custom event
PIPE_ADD_INTERVAL = 3000 # milliseconds
FRAME_ANIMATION_WIDTH = 3 # pixels per frame
FRAME_BIRD_DROP_HEIGHT = 3 # pixels per frame
FRAME_BIRD_JUMP_HEIGHT = 5 # pixels per frame
BIRD_JUMP_STEPS = 20 # see get_frame_jump_height docstring
WIN_WIDTH = 284 * 2 # BG image size: 284x512 px; tiled twice
WIN_HEIGHT = 512
PIPE_WIDTH = 80
PIPE_PIECE_HEIGHT = BIRD_WIDTH = BIRD_HEIGHT = 32
class PipePair:
 """Represents an obstacle.
 A PipePair has a top and a bottom pipe, and only between them can
 the bird pass -- if it collides with either part, the game is over.
 Attributes:
 x: The PipePair's X position. Note that there is no y attribute,
 as it will only ever be 0.
 surface: A pygame.Surface which can be blitted to the main surface
 to display the PipePair.
 top_pieces: The number of pieces, including the end piece, in the
 top pipe.
 bottom_pieces: The number of pieces, including the end piece, in
 the bottom pipe.
 """
 def __init__(self, surface, top_pieces, bottom_pieces):
 """Initialises a new PipePair with the given arguments.
 The new PipePair will automatically be assigned an x attribute of
 WIN_WIDTH.
 Arguments:
 surface: A pygame.Surface which can be blitted to the main
 surface to display the PipePair. You are responsible for
 converting it, if desired.
 top_pieces: The number of pieces, including the end piece, which
 make up the top pipe.
 bottom_pieces: The number of pieces, including the end piece,
 which make up the bottom pipe.
 """
 self.x = WIN_WIDTH
 self.surface = surface
 self.top_pieces = top_pieces
 self.bottom_pieces = bottom_pieces
 self.score_counted = False
 @property
 def top_height_px(self):
 """Get the top pipe's height, in pixels."""
 return self.top_pieces * PIPE_PIECE_HEIGHT
 @property
 def bottom_height_px(self):
 """Get the bottom pipe's height, in pixels."""
 return self.bottom_pieces * PIPE_PIECE_HEIGHT
 def is_bird_collision(self, bird_position):
 """Get whether the bird crashed into a pipe in this PipePair.
 Arguments:
 bird_position: The bird's position on screen, as a tuple in
 the form (X, Y).
 """
 bx, by = bird_position
 in_x_range = bx + BIRD_WIDTH > self.x and bx < self.x + PIPE_WIDTH
 in_y_range = (by < self.top_height_px or
 by + BIRD_HEIGHT > WIN_HEIGHT - self.bottom_height_px)
 return in_x_range and in_y_range
def load_images():
 """Load all images required by the game and return a dict of them.
 The returned dict has the following keys:
 background: The game's background image.
 bird-wingup: An image of the bird with its wing pointing upward.
 Use this and bird-wingdown to create a flapping bird.
 bird-wingdown: An image of the bird with its wing pointing downward.
 Use this and bird-wingup to create a flapping bird.
 pipe-end: An image of a pipe's end piece (the slightly wider bit).
 Use this and pipe-body to make pipes.
 pipe-body: An image of a slice of a pipe's body. Use this and
 pipe-body to make pipes.
 """
 def load_image(img_file_name):
 """Return the loaded pygame image with the specified file name.
 This function looks for images in the game's images folder
 (./images/). All images are converted before being returned to
 speed up blitting.
 Arguments:
 img_file_name: The file name (including its extension, e.g.
 '.png') of the required image, without a file path.
 """
 file_name = os.path.join('.', 'images', img_file_name)
 img = pygame.image.load(file_name)
 # converting all images before use speeds up blitting
 img.convert()
 return img
 return {'background': load_image('background.png'),
 'pipe-end': load_image('pipe_end.png'),
 'pipe-body': load_image('pipe_body.png'), 
 # images for animating the flapping bird -- animated GIFs are
 # not supported in pygame
 'bird-wingup': load_image('bird_wing_up.png'),
 'bird-wingdown': load_image('bird_wing_down.png')}
def get_frame_jump_height(jump_step):
 """Calculate how high the bird should jump in a particular frame.
 This function uses the cosine function to achieve a smooth jump:
 In the first and last few frames, the bird jumps very little, in the
 middle of the jump, it jumps a lot.
 After a completed jump, the bird will have jumped
 FRAME_BIRD_JUMP_HEIGHT * BIRD_JUMP_STEPS pixels high, thus jumping,
 on average, FRAME_BIRD_JUMP_HEIGHT pixels every step.
 Arguments:
 jump_step: Which frame of the jump this is, where one complete jump
 consists of BIRD_JUMP_STEPS frames.
 """
 frac_jump_done = jump_step / float(BIRD_JUMP_STEPS)
 return (1 - math.cos(frac_jump_done * math.pi)) * FRAME_BIRD_JUMP_HEIGHT
def random_pipe_pair(pipe_end_img, pipe_body_img):
 """Return a PipePair with pipes of random height.
 The returned PipePair's surface will contain one bottom-up pipe
 and one top-down pipe. The pipes will have a distance of
 BIRD_HEIGHT*3.
 Both passed images are assumed to have a size of (PIPE_WIDTH,
 PIPE_PIECE_HEIGHT).
 Arguments:
 pipe_end_img: The image to use to represent a pipe's endpiece.
 pipe_body_img: The image to use to represent one horizontal slice
 of a pipe's body.
 """
 surface = pygame.Surface((PIPE_WIDTH, WIN_HEIGHT), SRCALPHA)
 surface.convert() # speeds up blitting
 surface.fill((0, 0, 0, 0))
 max_pipe_body_pieces = int(
 (WIN_HEIGHT - # fill window from top to bottom
 3 * BIRD_HEIGHT - # make room for bird to fit through
 3 * PIPE_PIECE_HEIGHT) / # 2 end pieces and 1 body piece for top pipe
 PIPE_PIECE_HEIGHT # to get number of pipe pieces
 )
 bottom_pipe_pieces = randint(1, max_pipe_body_pieces)
 top_pipe_pieces = max_pipe_body_pieces - bottom_pipe_pieces
 # bottom pipe
 for i in range(1, bottom_pipe_pieces + 1):
 surface.blit(pipe_body_img, (0, WIN_HEIGHT - i*PIPE_PIECE_HEIGHT))
 bottom_pipe_end_y = WIN_HEIGHT - bottom_pipe_pieces*PIPE_PIECE_HEIGHT
 surface.blit(pipe_end_img, (0, bottom_pipe_end_y - PIPE_PIECE_HEIGHT))
 # top pipe
 for i in range(top_pipe_pieces):
 surface.blit(pipe_body_img, (0, i * PIPE_PIECE_HEIGHT))
 top_pipe_end_y = top_pipe_pieces * PIPE_PIECE_HEIGHT
 surface.blit(pipe_end_img, (0, top_pipe_end_y))
 # compensate for added end pieces
 top_pipe_pieces += 1
 bottom_pipe_pieces += 1
 return PipePair(surface, top_pipe_pieces, bottom_pipe_pieces)
def main():
 """The application's entry point.
 If someone executes this module (instead of importing it, for
 example), this function is called.
 """
 pygame.init()
 display_surface = pygame.display.set_mode((WIN_WIDTH, WIN_HEIGHT))
 pygame.display.set_caption('Pygame Flappy Bird')
 clock = pygame.time.Clock()
 score_font = pygame.font.SysFont(None, 32, bold=True) # default font
 # the bird stays in the same x position, so BIRD_X is a constant
 BIRD_X = 50
 bird_y = int(WIN_HEIGHT/2 - BIRD_HEIGHT/2) # center bird on screen
 images = load_images()
 # timer for adding new pipes
 pygame.time.set_timer(EVENT_NEWPIPE, PIPE_ADD_INTERVAL)
 pipes = []
 steps_to_jump = 2
 score = 0
 done = paused = False
 while not done:
 for e in pygame.event.get():
 if e.type == QUIT or (e.type == KEYUP and e.key == K_ESCAPE):
 done = True
 break
 elif e.type == KEYUP and e.key in (K_PAUSE, K_p):
 paused = not paused
 elif e.type == MOUSEBUTTONUP or (e.type == KEYUP and
 e.key in (K_UP, K_RETURN, K_SPACE)):
 steps_to_jump = BIRD_JUMP_STEPS
 elif e.type == EVENT_NEWPIPE:
 pp = random_pipe_pair(images['pipe-end'], images['pipe-body'])
 pipes.append(pp)
 clock.tick(FPS)
 if paused:
 continue # don't draw anything
 for x in (0, WIN_WIDTH / 2):
 display_surface.blit(images['background'], (x, 0))
 for p in pipes:
 p.x -= FRAME_ANIMATION_WIDTH
 if p.x <= -PIPE_WIDTH: # PipePair is off screen
 pipes.remove(p)
 else:
 display_surface.blit(p.surface, (p.x, 0))
 # calculate position of jumping bird
 if steps_to_jump > 0:
 bird_y -= get_frame_jump_height(BIRD_JUMP_STEPS - steps_to_jump)
 steps_to_jump -= 1
 else:
 bird_y += FRAME_BIRD_DROP_HEIGHT
 # because pygame doesn't support animated GIFs, we have to
 # animate the flapping bird ourselves
 if pygame.time.get_ticks() % 500 >= 250:
 display_surface.blit(images['bird-wingup'], (BIRD_X, bird_y))
 else:
 display_surface.blit(images['bird-wingdown'], (BIRD_X, bird_y))
 # update and display score
 for p in pipes:
 if p.x + PIPE_WIDTH < BIRD_X and not p.score_counted:
 score += 1
 p.score_counted = True
 score_surface = score_font.render(str(score), True, (255, 255, 255))
 score_x = WIN_WIDTH/2 - score_surface.get_width()/2
 display_surface.blit(score_surface, (score_x, PIPE_PIECE_HEIGHT))
 pygame.display.update()
 # check for collisions
 pipe_collisions = [p.is_bird_collision((BIRD_X, bird_y)) for p in pipes]
 if (0 >= bird_y or bird_y >= WIN_HEIGHT - BIRD_HEIGHT or
 True in pipe_collisions):
 print('You crashed! Score: %i' % score)
 break
 pygame.quit()
if __name__ == '__main__':
 # If this module had been imported, __name__ would be 'flappybird'.
 # It was executed (e.g. by double-clicking the file), so call main.
 main()

• Simon Forsberg: Oh how I wish more teachers were like you! Welcome to Code Review! Teach your students to review code!
• Timo: @SimonAndréForsberg: thanks! And, that's good idea -- I'll find some good code they can learn from, or some bad code for error-spotting.
• 200_success: The frozen GitHub tree at the time this question was asked
• tim: @Timo no problem :) Yes, go ahead and change it there. Also, you might wait a while for more answers, adjust your code and post a follow-up review if you want more remarks on the updated code.
• JaDogg: Nice, Once the students complete their projects, and if their code is working, tell them to create accounts and post them here. Good Luck.

Answer 1

1. You have docstrings for your functions and classes, which makes your code better than 95% of code submitted to Code Review.

2. The behaviour of the pipes is split into several pieces: (i) the PipePair class; (ii) the motion, drawing, and destruction logic in main; (iii) the scoring logic in main; (iv) the factory function random_pipe_pair. It would make the code easier to understand and maintain if you collected all the pipe logic into methods on the PipePair class.

3. Similarly, the behaviour of the bird is distributed among several places: (i) the local variables bird_y and steps_to_jump in main; (ii) the "calculate position of jumping bird" logic; (iii) the flapping animation logic; (iv) the get_frame_jump_height function. It would make the code easier to understand if you collected all the bird logic into methods on a Bird class.

4. The word "jumping" doesn't seem like a good description of the behaviour of the bird.

5. Name is_bird_collision make not sense English.

6. In the collision logic you're effectively testing for intersection of rectangular hit boxes. Pygame provides a Rect class with various collide methods that would make your code clearer, and would make it easier to do things like drawing the hit boxes to help with debugging.

7. You store the pipes in a list, but this is inefficient when it comes to remove a pipe: list.remove takes time proportional to the length of the list. You should use a set, or, since you know that pipes get created on the right and destroyed on the left, a collections.deque.

8. When you test for collisions, you store the collision results in a list and then test to see if True is an element of the list. Instead, you should use the built-in function any:

   if any(p.collides_with(bird) for p in pipes):
   

   (This has the additional advantage of short-circuiting: that is, stopping as soon as a collision has been detected, instead of going on to test the remaining pipes.)

9. You measure time in frames (for example, pipes move leftwards at a particular number of pixels per frame). This has the consequence that you cannot change the framerate without having to change many other parameters. It is more general to measure time in seconds: this makes it possible to vary the framerate.

   (In this kind of simple game you can get away with measuring in frames, but in more sophisticated games you'll need to be able to vary the framerate and so it's worth practicing the necessary techniques.)

10. In commit 583c3e49 you broke the game by (i) removing the function random_pipe_pair without changing the caller; and (ii) changing the local variable surface to an attribute self.surface in some places but not others.

    We all make commits in error from time to time, but there are four commits after this one, which suggests that you haven't been testing your code before committing it. This is a bad habit to get into!

• Timo: I agree, the bird doesn't really jump, but what verb would you suggest? Fly up? Climb (as used in aviation)?
• 200_success: I suggest climb/sink, rise/fall, or ascend/descend.
• Gareth Rees: "flap" would be my choice.
• Timo: All the answers here are good, but I'm accepting this one because it has the most suggestions.
• Timo: Alright, I think I've got everything covered. The latest code is in the Github repo.

Answer 2

This is pretty nice code! I can still nitpitck a bit :)

---

You could use the cool .. < .. < .. operator for this:

in_x_range = bx + BIRD_WIDTH > self.x and bx < self.x + PIPE_WIDTH

like this:

in_x_range = bx - PIPE_WIDTH < self.x < bx + BIRD_WIDTH

---

Maybe random_pipe_pair will be slightly more readable if you added a few line breaks.

---

Moot point, but keeping it anyway, see my conclusion at the bottom.

(削除) You don't need the parentheses here: (削除ここまで)

in_y_range = (by < self.top_height_px or
 by + BIRD_HEIGHT > WIN_HEIGHT - self.bottom_height_px)

and here:

if e.type == QUIT or (e.type == KEYUP and e.key == K_ESCAPE):

and here the outer parens:

elif e.type == MOUSEBUTTONUP or (e.type == KEYUP and
 e.key in (K_UP, K_RETURN, K_SPACE)):

and here:

if (0 >= bird_y or bird_y >= WIN_HEIGHT - BIRD_HEIGHT or
 True in pipe_collisions):

BUT... As you commented, you use the parens mostly to allow breaking long lines (probably following PEP8) without the ugly \. I totally agree with that, so yeah, keep 'em! (In fact I didn't even know this was possible, so thanks for the lesson, teach'!)

• Timo: Good points! About the parentheses: either I'm using them for line continuation so I don't have to use that ugly backslash :), or I'm using them for clarity -- I know the precedence of and and or, but 'explicit is better than implicit', as the Zen of Python says :)
• DarkDust: I don't actually know Python but found the question interesting. About removing the parenthesis from if a or (b and c): I would leave those in, simply because it makes it explicit and you don't need to know/remember the operator precedence. I've worked with about 20 languages now and operator precedence rules is something I got sick of: just add parenthesis and everybody knows how it's supposed to work.
• janos: I agree that explicit is better than implicit, but this is a bit bordering paranoid coding for me. BUT, I agree with your other point about line breaking, and I updated my post
• Timo: Changed the code to include 'the cool x < y < z operator' -- thanks! :)

Answer 3

• I am not comfortable with pause handling. First, the busy-waiting loop. Second, the paused game just doesn't render anything, but it still serves events, e.g. pipes are added.

• random_pipe_pair really wants to be a PipePair constructor. Similarly, images['pipe_body'] and images['pipe_end'] should be static members of PipePair.

• Timo: Good points, thanks. Let's see how that works out.. I'll commit to Github when I'm finished.
• Timo: Finally... after some trouble with git, I committed the changes. I'll update the question as well.
• vnp: It is against CR policy to edit the code, because it invalidates the review. Please roll back the edit. You are more than welcome to post the updated code as a follow-up question.

Answer 4

This is neat :)

I think that unless you're tracking the rects which have changed, display.update() is no better than display.flip(), though I suppose for a student it would be easier to read.

I'm curious as to why collisions are being checked after the view updates? I know that in an event loop situation the actual order of processes can be a little lax, especially if you're working at a high FPS, and I see that it presents the code in a way where the end of the loop contains the "exit or don't" code, but I guess I'm of the opinion that all the state checking work should happen before the view is updated. That's probably small potatoes, I've no idea the age or experience of the students you're working with.

I have to agree that the paused handling is a bit weird, just because it kind of starts a discussion about using states to control the flow of the (very short) script. Would rather see something more like "if not paused: do_stuff()" rather than "if paused: continue".

It's also sort of interesting which aspects of the PyGame API you've chosen to work with; for my part, it feels very strange to do an Intro To Games class without working with sprites. The Pygame sprite.Sprite object is a pretty helpful thing to have around! But as I look over the code it seems like it introduces a range of different concepts, so maybe there's no call for adding yet another concept to the mix.

• Timo: Thanks! Because I'm breaking from the main loop directly if a collision is detected, I put it at the end when I first coded that part of the game so that the user isn't told that they crashed while the bird is still displayed a few pixels away from the pipe. I'll move all state checking to the beginning of the loop and just set done = True -- like it's done for the QUIT event, for example.
• Timo: Good point about display.flip() vs display.update() -- I must have overlooked flip when I read through the documentation.
• Timo: I'm not entirely sure what you mean by if not paused: do_stuff() -- are you suggesting that I extract the entire loop body into another function and just call it if the game isn't paused?
• Timo: Regarding sprites: I could either create a new Bird class subclassing sprite.Sprite and make PipePair subclass sprite.Group, which would, IMHO, make the game more complex than it needs to be; or I could just replace every usage of images[...] with some_sprite.image or something similar, and replace every display_surface.blit(some_pipe_pair.surface, ...) with some_pipe_pair.draw(display_surface), which wouldn't really simplify the code either, IMHO. But I would definitely introduce sprites somehow, perhaps in a different example.
• Stick: Well you don't necessarily have to pull all the view stuff out of the view, but I see what you're saying. As for subclassing sprite.Group, I may have misunderstood what you meant, but Group is a container for sprites with convenience methods. For example, MyGroup.draw(DispSurf) would blit all sprites in that group without the use of a for loop; and a call like pygame.sprite.spritecollide(Bird, PipesGroup) would return a list of all the sprites in PipesGroup the bird hit, so a simple "if spritecollide(Bird, PipesGroup):" would serve as hit detection (since an empty list is falsy)