4
\$\begingroup\$

I am a beginner in Python. This is my first project in Python (that I tried to complete).

There is a grey square that acts as the Flappy 'Bird' with green obstacles spawning with random gap sizes. The difficulty of the game stays the same for now. If an obstacle is hit, the game closes (I haven't implemented any menu thing yet). Pressing Esc pauses/unpauses the game.

How does the code look like and what improvements can I make to it in terms of organization, scalability, best practices, etc?

pygame 2.5.0 (SDL 2.28.0, Python 3.11.4)

  1. game_objects.py
class Player:
    def __init__(self, position: tuple[float, float], size: tuple[int, int], color: tuple[int, int, int]):
        self.x, self.y = position
        
        self.width, self.height = size
        
        self.acc_y = 0.0
        self.vel_y = 0.0
        
        self.color = color

class Obstacle:
    vel_x = -65.0
    
    def __init__(self, position: tuple[float, float], size: tuple[int, int], color: tuple[int, int, int]):
        self.x, self.y = position
        
        self.width, self.height = size
        
        self.color = color
  1. main.py
import pygame, random
from game_objects import *

# Initializing pygame and setting up the screen
pygame.init()

pygame.display.set_caption("Flappy Bird")

SCREEN_WIDTH, SCREEN_HEIGHT = 600, 600

screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT), flags=pygame.SCALED, vsync=1)

# The player
player = Player((85.0, 150.0), (25, 25), (50, 50, 50))
player.acc_y = 500

# Obstacle list
obstacles: list[Obstacle] = []

def spawn_obstacle():
    gap_height = random.randint(125, 250)
    gap_y = random.randint(0, (SCREEN_HEIGHT - gap_height))
    
    obstacles.append(Obstacle((SCREEN_WIDTH, 0.0), (65, gap_y), (45, 168, 40)))
    obstacles.append(Obstacle((SCREEN_WIDTH, (gap_y + gap_height)), (65, (SCREEN_HEIGHT - (gap_y + gap_height))), (45, 168, 40)))

##### The game loop

# For calculating delta_time
last_tick = pygame.time.get_ticks()

# For spawning obstacles at regular intervals
obstacle_interval = 3000
time_since_last_obstacle = obstacle_interval
last_obstacle_tick = (last_tick - time_since_last_obstacle)

paused = False
lost = False

running = True
while (running):
    # Handling various events
    for event in pygame.event.get():
        if (event.type == pygame.QUIT):
            running = False
        elif (event.type == pygame.KEYDOWN):
            if ((event.key == pygame.K_w) or (event.key == pygame.K_UP)):
                player.vel_y = -250.
            elif (event.key == pygame.K_ESCAPE):
                if (paused):
                    paused = False
                    last_tick = pygame.time.get_ticks()
                    last_obstacle_tick = (last_tick - time_since_last_obstacle)
                else:
                    paused = True
    
    if (not paused):
        # Check if the player lost the previous iteration
        if (lost):
            print("You lost!")
            running = False
            continue
        
        # Calculate delta_time
        current_tick = pygame.time.get_ticks()
        delta_time = (current_tick - last_tick) / 1000.0
        last_tick = current_tick
        
        # Clear the screen for fresh drawing
        screen.fill((156, 204, 255))
        
        # Spawning obstacles every obstacle_interval milliseconds
        time_since_last_obstacle = (current_tick - last_obstacle_tick)
        if (time_since_last_obstacle > obstacle_interval):
            last_obstacle_tick = current_tick
            spawn_obstacle()
        
        # Updating the player's movement and position
        player.vel_y += (player.acc_y * delta_time)
        player.y += (player.vel_y * delta_time)
        
        # Check whether the player is colliding with the horizontal edges
        if ((player.y < 0.0) or (player.y > (SCREEN_HEIGHT - player.height))):
            lost = True
        
        # Obstacle logic
        for obstacle in obstacles:
            obstacle.x += (Obstacle.vel_x * delta_time)
            
            # Check whether the obstacle is no longer visible
            if (obstacle.x < -obstacle.width):
                obstacles.remove(obstacle)
                continue
            
            # Check whether the player is colliding with the obstacle
            if ((player.x > (obstacle.x - player.width)) and (player.x < (obstacle.x + obstacle.width))):
                if ((player.y > (obstacle.y - player.height)) and (player.y < (obstacle.y + obstacle.height))):
                    lost = True
            
            # Drawing the obstacle
            pygame.draw.rect(screen, obstacle.color, pygame.Rect(int(obstacle.x), int(obstacle.y), obstacle.width, obstacle.height))
        
        # Drawing the player
        pygame.draw.rect(screen, player.color, pygame.Rect(int(player.x), int(player.y), player.width, player.height))
        
    # Update the entire window
    pygame.display.flip()
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

The game is too hard for me. As for the code, I appreciate the comments. The main point I want to insist on is to separate things into classes/functions/files where they belong.

Constants

Separate all the constants/settings/parameters to another file. This way if you need to adjust the parameters or maybe add dificulty levels later on, everything is in the same place.

constants.py


## Graphics options
SCREEN_WIDTH, SCREEN_HEIGHT = 600, 600

## Gameplay options
OBSTACLE_SPEED = 65
OBSTACLE_WIDTH = 65
OBSTACLE_GAP_RANGE = (125,250)
OBSTACLE_SPAWN_SPACE_INTERVAL = 200

PLAYER_SPAWN_POS = (85.0, 150.0)
PLAYER_SIZE = (25, 25)
PLAYER_INITIAL_DECEL = 500

As for the colors, you've coded them in instance declarations. I suggest using an Enum to define every color you'll need and separate variables to pick object colors. This way you can play around with colors and objects independantly in the future. In constants.py or another file.

from enum import Enum
## Colors
class Color(Enum):
    GREEN =  (45, 168, 40)
    GREY = (50, 50, 50)
    LIGHT_BLUE = (156, 204, 255)

PLAYER_COLOR = Color.GREY
OBSTACLE_COLOR = Color.GREEN
BACKGROUND_COLOR = Color.LIGHT_BLUE

Game objects

You've defined classes for players, which is good. Consider using dataclasses. They let you skip all the boilerplate init code and add useful methods for printing and equality checks. Your objects also share a few attributes such as position, size and color. These are common to any game object and if you were to add any new type of game object they woud have it too. Maybe its a good idea to define an abstract UI object class:

@dataclass
class UI_object:
    # Also possible to go with tuples instead of x, y eg:
    # size: Tuple[int,int] and position Tuple[float,float]
    x: float
    y: float
    width: int
    height: int
    color: cst.Color

@dataclass()
class Obstacle(UI_object):
    ...

@dataclass
class Player(UI_object):
    ...

Your code logic is all over the place in your main file. I suggest you separate logic related to each class into its own methods. Spawning players or obstacles should be class methods. Checking if the player is out of bounds shoud be a Player instance method. Checking if the player hit an obstacle could be a Player or Obstacle instance method. Also you update object positions for players and obstacles separately. I would group the logic together into the abstract class.

game_objects.py

from dataclasses import dataclass
from typing import Optional
import constants as cst
import random


@dataclass
class UI_object:
    x: float
    y: float
    width: int
    height: int
    color: cst.Color
    vel_x: Optional[float] = 0
    vel_y: Optional[float] = 0
    acc_x: Optional[float] = 0
    acc_y: Optional[float] = 0

    def update_pos(self, time_delta: float):
        self.vel_x += self.acc_x * time_delta
        self.x += self.vel_x * time_delta
        self.vel_y += self.acc_y * time_delta
        self.y += self.vel_y * time_delta


@dataclass()
class Obstacle(UI_object):
    vel_x: float = -cst.OBSTACLE_SPEED

    @classmethod
    def spawn_obstacle(cls):
        gap_height = random.randint(
            cst.OBSTACLE_GAP_INTERVAL[0], cst.OBSTACLE_GAP_INTERVAL[1]
        )
        gap_y = random.randint(0, (cst.SCREEN_HEIGHT - gap_height))

        lower_obstacle = cls(
            x=cst.SCREEN_HEIGHT,
            y=0,
            width=cst.OBSTACLE_WIDTH,
            height=gap_y,
            color=cst.OBSTACLE_COLOR,
        )
        upper_obstacle = cls(
            x=cst.SCREEN_HEIGHT,
            y=gap_y + gap_height,
            width=cst.OBSTACLE_WIDTH,
            height=cst.SCREEN_HEIGHT - gap_y - gap_height,
            color=cst.OBSTACLE_COLOR,
        )
        return lower_obstacle, upper_obstacle


@dataclass
class Player(UI_object):
    acc_y: float = cst.PLAYER_INITIAL_DECEL

    def is_out_of_bounds(self):
        return (self.y < 0.0) or (self.y > (cst.SCREEN_HEIGHT - self.height))

    def is_touching_obstacle(self, obstacle: Obstacle):
        return (
            (obstacle.x - self.width) < self.x < (obstacle.x + obstacle.width)
        ) and ((obstacle.y - self.height) < self.y < (obstacle.y + obstacle.height))

    @classmethod
    def spawn(cls):
        return cls(
            x=cst.PLAYER_SPAWN_POS[0],
            y=cst.PLAYER_SPAWN_POS[1],
            width=cst.PLAYER_SIZE[0],
            height=cst.PLAYER_SIZE[1],
            color = cst.PLAYER_COLOR
        )

Main file

It's recommended to use a __main__ == "__name__" check in your python scripts mainly to prevent accidental execution of the script. I created a Game Class to organise the code but you could just as easily achieve the same with functions only.

class Flappy_bird:
    def __init__(self) -> None:
        pygame.init()
        pygame.display.set_caption("Flappy Bird")
        self.screen =  pygame.display.set_mode((cst.SCREEN_WIDTH, cst.SCREEN_HEIGHT), flags=pygame.SCALED, vsync=1)
        self.player : Player = Player.spawn()
        self.obstacles : List[Obstacle] = []
        self.paused = False
        self.last_tick:int = 0
        self.last_obstacle_tick: int = 0

You render the background, the obstacles and the player separataly at differents times and places in your code. Say you decided to render you game differently, you would have to change code in all these different places. Put code that does the same or similar things together.

I grouped up everything so as to end up with a simple looking game loop.

   while True:
        for event in pygame.event.get():
            self.handle_event(event)
        time_delta = self.update_tick()
        if not self.paused:
            self.add_or_remove_obstacles()
            self.update_object_positions(time_delta)
            self.check_if_player_lost()
            self.render()

Nitpicking

  • Instead of (B > A) and (B < C) you can use A < B < C
  • Don't go too deep into if-elses, usually there are ways of simplyfiying
  • Too many flags! You don't need the lost and running flags. You can always return or quit().
  • In general avoid import *
  • I also changed they way you handle new obstacles from a time interval to a space interval. It is both simpler to handle pauses and it allows you to change the obstacle speed without having to also adjust the interval.

All in all really good code for a first project. You could add score keeping, didiculty levels, and maybe saving and reloading a game to learn different concepts.

Final script: main.py

from typing import Any, List
import pygame, random
from game_objects import Player, Obstacle
import constants as cst


class Flappy_bird:
    def __init__(self) -> None:
        pygame.init()
        pygame.display.set_caption("Flappy Bird")
        self.screen = pygame.display.set_mode(
            (cst.SCREEN_WIDTH, cst.SCREEN_HEIGHT), flags=pygame.SCALED, vsync=1
        )
        self.player: Player = Player.spawn()
        self.obstacles: List[Obstacle] = []
        self.paused = False
        self.last_tick: int = 0

    def handle_event(self, event: pygame.event.EventType):
        if event.type == pygame.QUIT:
            exit()
        if event.type == pygame.KEYDOWN:
            if (event.key == pygame.K_w) or (event.key == pygame.K_UP):
                self.player.vel_y = -250.0
            elif event.key == pygame.K_ESCAPE:
                self.paused = not self.paused

    def check_if_player_lost(self):
        if self.player.is_out_of_bounds() or any(
            self.player.is_touching_obstacle(obstacle) for obstacle in self.obstacles
        ):
            print("You lost!")
            exit()

    def render(self):
        # Clear the screen for fresh drawing
        self.screen.fill(cst.BACKGROUND_COLOR.value)
        # Drawing the player
        pygame.draw.rect(
            self.screen,
            self.player.color.value,
            pygame.Rect(
                int(self.player.x),
                int(self.player.y),
                self.player.width,
                self.player.height,
            ),
        )
        # Drawing the obstacles
        for obstacle in self.obstacles:
            pygame.draw.rect(
                self.screen,
                obstacle.color.value,
                pygame.Rect(
                    int(obstacle.x), int(obstacle.y), obstacle.width, obstacle.height
                ),
            )
        # Update the entire window
        pygame.display.flip()

    def update_object_positions(self, time_delta_ms: int):
        time_delta_s = time_delta_ms / 1000
        self.player.update_pos(time_delta_s)
        for obstacle in self.obstacles:
            obstacle.update_pos(time_delta_s)

    def add_or_remove_obstacles(self):
        self.obstacles = [
            obstacle for obstacle in self.obstacles if (obstacle.x >= -obstacle.width)
        ]
        if (
            not self.obstacles
            or cst.SCREEN_WIDTH - self.obstacles[-1].x
            > cst.OBSTACLE_SPAWN_SPACE_INTERVAL
        ):
            self.obstacles.extend(Obstacle.spawn_obstacle())

    def update_tick(self) -> int:
        current_tick = pygame.time.get_ticks()
        time_delta = current_tick - self.last_tick
        self.last_tick = current_tick
        return time_delta

    def run(self):
        # initial ticks
        self.last_tick = pygame.time.get_ticks()
        while True:
            for event in pygame.event.get():
                self.handle_event(event)
            time_delta = self.update_tick()
            if not self.paused:
                self.add_or_remove_obstacles()
                self.update_object_positions(time_delta)
                self.check_if_player_lost()
                self.render()


if __name__ == "__main__":
    game = Flappy_bird()
    game.run()

```
\$\endgroup\$
3
  • \$\begingroup\$ Thank you very much for your time! This is a bit overwhelming for me as a beginner. I will make sure to incorporate these tips into my new and existing projects. Also, I have a question: I am planning to work on a simple interpreter in Python for learning purposes. Is this a good idea or do you recommend something else? \$\endgroup\$ Commented Aug 9, 2023 at 19:09
  • \$\begingroup\$ I've never implemented an interpreter myself so I couldn't say but sounds cool and challenging! \$\endgroup\$
    – kubatucka
    Commented Aug 10, 2023 at 9:54
  • \$\begingroup\$ Oh and I apparently forgot to set the player velocity on key press ( the - 250) in constants.py . It shouldn't be hardcoded in my handle_event method. \$\endgroup\$
    – kubatucka
    Commented Aug 10, 2023 at 9:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.