Flappy Bird game clone for a beginners' programming class

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 p

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

-
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.

-
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.

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

-
Name is_bird_collision make not sense English.

-
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.

-
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.

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

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

-
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!

StackExchange Code Review Q#61477, answer score: 33