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()
asked Aug 6, 2023 at 17:03
\$\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()
```
answered Aug 8, 2023 at 15:49
\$\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\$ 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\$ Commented Aug 10, 2023 at 9:57

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.