Code for a basic dodging game in Python

Question

Can someone review the code for this dodging game I made a while back? I want to see if there are any improvements or optimizations I can make as I am more or less a Python beginner.

# A basic dodging game in Pygame

import pygame, sys
import numpy as np
from pathlib import Path

pygame.init()

# Defines window size
win_size = (900, 600)
window = pygame.display.set_mode(win_size)

pygame.display.set_caption("Dodge")

clock = pygame.time.Clock()

scores = np.loadtxt(str(Path().absolute()) + "/scores.txt").astype(np.int64)

try:
    high_score = scores.max()
except ValueError:
    high_score = 0

running = True
timer = 0

projectile_speed = 7 # Defines projectile speed

score = 0

score_font = pygame.font.SysFont("dejavusansmono", 24)
game_over_font = pygame.font.SysFont("dejavusansmono", 48)

title_text = score_font.render("Score: ", True, (45, 45, 45))
score_text = score_font.render(str(score), True, (110, 110, 110))

# AABB Collision implementation
def aabb(r1, r2):
    return r1.x < r2.x + r2.width and r1.x + r1.width > r2.x and r1.y < r2.y + r2.height and r1.y + r1.height > r2.y

# Displays game over content if input(collision) is true
def game_over(val):
    if val:
        game_over_text = game_over_font.render("GAME OVER!", True, (0, 0, 0))
        window.blit(game_over_text, (win_size[0]/2 - game_over_text.get_width()/2, win_size[1]/2 - game_over_text.get_height()/2))
        
        pygame.display.update()
        pygame.time.wait(1000)

        if score > high_score:
            title_text = score_font.render("New Highscore!", True, (45, 45, 45))
            window.blit(title_text, (win_size[0]/2 - game_over_text.get_width()/2, win_size[1]/2 + game_over_text.get_height()/2))

        else:
            title_text = score_font.render("Final Score: ", True, (45, 45, 45))
            score_text = score_font.render(str(score), True, (110, 110, 110))

            window.blit(title_text, (win_size[0]/2 - game_over_text.get_width()/2, win_size[1]/2 + game_over_text.get_height()/2))
            window.blit(score_text, (win_size[0]/2 - game_over_text.get_width()/2 + title_text.get_width(), win_size[1]/2 + game_over_text.get_height()/2))

        pygame.display.update()
        pygame.time.wait(1000)

        return False
    return True

# Object class that parents Player and Projectile classes
class Object:
    global objects
    objects = []

    def __init__(self, x, y, w, h):
        self.x = x
        self.y = y
        self.width = w
        self.height = h

        objects.append(self)

    def draw(self):
        pygame.draw.rect(window, "red", (self.x, self.y, self.width, self.height)) # Default draw method

# Defines Projectile class (child of Object)
class Projectile(Object):

    # Global projectile list stores all projectile data
    global projectiles
    projectiles = []

    def __init__(self, x, y, w, h, speed):
        super().__init__(x, y, w, h)

        self.dir = -1 * (x/abs(x)) # Calucates required sprite direction based on input value
        self.speed = speed * self.dir
        projectiles.append(self)

    def update(self):
        self.x += self.speed

    @classmethod
    def update_all(self):
        valid_projectiles = [projectile for projectile in projectiles if (projectile.x > 0 - projectile.width and projectile.dir == -1) or (projectile.x < win_size[0] and projectile.dir == 1)] # Projectiles within bounds
        invalid_projectiles = [projectiles.pop(projectiles.index(projectile)) for projectile in projectiles if (projectile.x < 0 - projectile.width and projectile.dir == -1) or (projectile.x > win_size[0] and projectile.dir == 1)] # Projectiles NOT within bounds
        del invalid_projectiles

        # Running all projectile methods
        for projectile in valid_projectiles:
            projectile.update()
            projectile.draw()

# Defines Player class (child of Object)
class Player(Object):
    def __init__(self, x, y, w, h, speed):
        super().__init__(x, y, w, h)
        self.speed = speed
        self.touched = False

    def draw(self):
        pygame.draw.rect(window, "blue", (self.x, self.y, self.width, self.height)) # Player draw method

    def update(self):
        self.keys = pygame.key.get_pressed()
        
        self.x += self.speed * (self.keys[pygame.K_RIGHT] - self.keys[pygame.K_LEFT]) # Left/Right Key Input
        self.y += self.speed * (self.keys[pygame.K_DOWN] - self.keys[pygame.K_UP]) # Down/Up Key Input

    def set_bounds(self):

        # Restricts player to screen dimensions
        if self.x < 0:
            self.x = 0
        elif self.x > win_size[0] - self.width:
            self.x = win_size[0] - self.width
        if self.y < 0:
            self.y = 0
        elif self.y > win_size[1] - self.height:
            self.y = win_size[1] - self.height
            self.yvel = 0

    def collision(self):

        # Defines collisions w/ Projectiles
        for projectile in projectiles:
            if aabb(player, projectile):
                return True
        return False
    
    def update_all(self):

        # Running Player methods
        self.update()
        self.set_bounds()
        self.draw()

player = Player(win_size[0]/2 - 16, win_size[1]/2 - 16, 32, 32, 5) # Defines player

while running: # Game loop
    window.fill("white") # Clears screen

    timer += 1

    if timer % 10 == 0: # New Projectile object every 10 frames
        Projectile(np.random.choice((-32, win_size[0])), np.random.randint(0, win_size[1] - 32), 32, 16, projectile_speed - np.random.random())

    if timer % 60 == 0: # Score increments by 1 every 60 frames
        score += 1
        projectile_speed += 0.2 # Increases every second: very basic difficulty scaling
        if projectile_speed > 14: # Caps off bullet speed
            projectile_speed = 14

    player.update_all() # Updates Player methods
    Projectile.update_all() # Updates Projectile methods

    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = False
            pygame.quit()
            sys.exit()

    title_text = score_font.render("Score: ", True, (45, 45, 45))
    score_text = score_font.render(str(score), True, (110, 110, 110))

    window.blit(title_text, (0, 0))
    window.blit(score_text, (title_text.get_width(), 0))

    title_text = score_font.render("Highscore: ", True, (45, 45, 45))
    score_text = score_font.render(str(high_score), True, (110, 110, 110))

    window.blit(title_text, (0, title_text.get_height()))
    window.blit(score_text, (title_text.get_width(), title_text.get_height()))  

    title_text = score_font.render(str(np.int64(clock.get_fps())), True, (45, 45, 45))
    window.blit(title_text, (0, win_size[1] - title_text.get_height()))

    running = game_over(player.collision()) 
    # Constantly calls game_over function
    # Returns false when player.collision() is true

    clock.tick(60)
    pygame.display.update()

scores = np.append(scores, score)
np.savetxt("scores.txt", scores)

pygame.quit()
sys.exit()

Answer 1

imports

Pep8 asks that you organize imports in the usual way. Don't bother learning the rules. Just use "$ isort *.py" and be done with it, move on to more pressing concerns.

comment on the why, not the how

# Defines window size
win_size = (900, 600)

You are saying what the code already eloquently states. Just elide this comment.

If you feel "win" is too cryptic, maybe spell out window_size, but the identifier is fine as-is. Your identifiers like game_over_font are great, BTW.

A valuable comment would explain that this is the right size for use on a certain display in the living room, or so an Emacs buffer will also comfortably fit on the screen without overlap. Or that going any bigger makes it hard to maintain a 60 FPS frame rate.

Similarly here:

projectile_speed = 7  # Defines projectile speed

pathlib

scores = np.loadtxt(str(Path().absolute()) + "/scores.txt"). ...

That would be more naturally expressed as
....loadtxt(Path("scores.txt").resolve())
It is seldom that we want .absolute() -- prefer .resolve().

docstring

# AABB Collision implementation
def aabb(r1, r2):

This is a helpful comment.

It would have been better as a """docstring""", on the line following def.

It would have been much better if it spelled out that we're doing axis-aligned bounding-box collision detection, perhaps citing the URL of a webpage that explains OBB vs AABB.

    return r1.x < r2.x + r2.width and r1.x + r1.width > r2.x and r1.y < r2.y + r2.height and r1.y + r1.height > r2.y

This is illegible as written. Maybe you can discern its meaning, but I can't. Use black to format your code:

    return (
        r1.x < r2.x + r2.width
        and r1.x + r1.width > r2.x
        and r1.y < r2.y + r2.height
        and r1.y + r1.height > r2.y
    )

Now it's transparently obvious what's going on here.

meaningful identifier

# Displays game over content if input(collision) is true
def game_over(val):
    if val:

Thank you for the valuable comment.

But why did we even need one? Couldn't the code have said that? The name val conveys nothing to the reader.

When I read the signature, I was looking for a type annotation that would help explain the nature of val. But then we see if val: and it seems def game_over(val: bool) -> None: could be a plausible signature.

But let's think about the Public API we're defining here. Should the whole body be indented one level? Maybe. Or perhaps we shouldn't even call game_over() until the game is over.

Consider ditching the if, unconditionally returning False, and alter the caller in this way:

    running = not(player.collision()) or game_over()

We rely on short-circuite evaluation of or. As long as we're collision free, we never evaluate the "game over" disjunct.

But as long as we're changing the design, why have "over" in the middle of the display loop at all? Just assign
running = not player.collision()
and put the game over call after the while running: loop.

tuple unpack

These expressions are less convenient than they could be:

        window.blit(... , (win_size[0]/2 - game_over_text.get_width()/2, win_size[1]/2 - game_over_text.get_height()/2))

The [0] and [1] subscripts are needlessly cryptic. Define a pair of width and height temp vars:

        win_w, win_h = win_size

We don't have to, but while we're at it we might shrink the spelling of that other expression, and go for parallel construction:

        txt_w, txt_h = game_over_text.get_width(), game_over_text.get_height()

color names

        ....render("GAME OVER!", True, (0, 0, 0))

This is not terrible; it seems fairly obvious we're rendering in black.

        ....render("New Highscore!", True, (45, 45, 45))
        ....render(str(score), True, (110, 110, 110))

Ok, now we've gone too far. Define the first magic number as BLACK, and invent two new names, maybe LIGHT_GREY and DARK_GREY.

Also define PAUSE = 1000, as we use it twice and should slow down by similar amounts if we ever increase the delay.

object vs Object

I guess this is kind of OK? I mean, they are distinct identifiers. But it seems too generic, too close to python's core object class.

class Object:
    global objects
    objects = []

I would prefer to see it phrased:

objects = []

class Object:

and we don't even need a global declaration for objects.append().

A better name for the class might be Drawable. In most game applications the conventional name would be Sprite. Oh, lookit dat, you even mention sprite in the Projectile ctor.

Also, do we really need objects at module scope? Couldn't the class hold it?

sgn()

Python is slightly odd among languages in that it lacks a sgn(y) function, for historic reasons. (There are corner cases, and consensus did not emerge for a universally useful way to handle them for all use cases.)

        self.dir = -1 * (x/abs(x)) # Calucates required sprite direction based on input value

You should use copysign:

        self.dir = -1 * math.copysign(1, x)

This more clearly conveys Author's Intent, without danger of exploding with ZeroDivisionError.

extract helper

This is an unfortunate one-liner:

    def update_all(self):
        valid_projectiles = [projectile for projectile in projectiles if (projectile.x > 0 - projectile.width and projectile.dir == -1) or (projectile.x < win_size[0] and projectile.dir == 1)] # Projectiles within bounds

Not even black can save it:

    valid_projectiles = [
        projectile
        for projectile in projectiles
        if (projectile.x > 0 - projectile.width and projectile.dir == -1)
        or (projectile.x < win_size[0] and projectile.dir == 1)
    ]

Package up a helper predicate:

def is_valid(projectile) -> bool:
    return (projectile.x > 0 - projectile.width and projectile.dir == -1) or (
        projectile.x < win_size[0] and projectile.dir == 1
    )

Now the caller can simply say:

        for projectile in filter(is_valid, projectiles):

Also, creating a list so we can have a .pop() side effect is not the best way to reveal Author's Intent. Prefer an explicit for loop to produce the side effect, rather than a list comprehension.

choose specific names

    def set_bounds(self):

This is a pretty good identifier.

But it sounds like it's filling in the size of a wall or something.

If it was named enforce_bounds it would be a little clearer that it's about how sprites interact with walls.

Consider rephrasing it in terms of min() and max(), e.g.
self.x = max(0, self.x)

---

I had many comments. I always do. But on the whole this game is written in a clear and straightforward style.

Keep up the good work!