Pong using pygame module

Question

I made a game in Python where you play Pong against an AI. As I am quite new to pygame, I would be grateful to hear any possible improvements.

This is my code:

import random

import pygame

from pygame.locals import (
    K_UP, K_DOWN,
    K_w, K_s,
    QUIT
)

BG_COLOR = (255,) * 3
FG_COLOR = (0,) * 3

SCREEN_WIDTH = 800
SCREEN_HEIGHT = 500

PADDLE_X = SCREEN_WIDTH / 15
PADDLE_Y = SCREEN_HEIGHT / 2

PADDLE_WIDTH = SCREEN_WIDTH / 30
PADDLE_HEIGHT = SCREEN_HEIGHT / 5

SPEED_MULTIPLIER = 3


class Paddle(pygame.sprite.Sprite):
    def __init__(self, x, y, width, height, *groups):
        super().__init__(*groups)

        self.image = pygame.Surface((width, height))
        self.image.fill(FG_COLOR)

        self.rect = self.image.get_rect()
        self.rect.x = x
        self.rect.y = y

    def update(self):
        if self.rect.collidepoint(self.rect.x, 0):
            self.rect.y += SPEED_MULTIPLIER

        elif self.rect.collidepoint(self.rect.x, SCREEN_HEIGHT):
            self.rect.y -= SPEED_MULTIPLIER


class Ball(pygame.sprite.Sprite):
    def __init__(self, x, y, r, *groups):
        super().__init__(*groups)

        self.image = pygame.Surface((r * 2,) * 2, pygame.SRCALPHA)
        self.image = self.image.convert_alpha()

        pygame.draw.circle(self.image, FG_COLOR, (r, r), r)

        self.rect = self.image.get_rect()

        self.rect.x = x
        self.rect.y = y

        self.dx = random.choice((-1, 1))
        self.dy = random.choice((random.uniform(1, -0.7), random.uniform(0.7, 1)))

        self.radius = r

    def update(self):
        self.rect.x += self.dx * SPEED_MULTIPLIER
        self.rect.y += self.dy * SPEED_MULTIPLIER


pygame.init()

screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))

clock = pygame.time.Clock()

sprites = pygame.sprite.Group()

computer_paddle = Paddle(
    PADDLE_X, PADDLE_Y,
    PADDLE_WIDTH, PADDLE_HEIGHT,
    sprites
)
human_paddle = Paddle(
    SCREEN_WIDTH - 2 * PADDLE_X, PADDLE_Y,
    PADDLE_WIDTH, PADDLE_HEIGHT,
    sprites
)

ball = Ball(
    SCREEN_WIDTH / 2, SCREEN_HEIGHT / 2,
    SCREEN_WIDTH / 40,
    sprites
)

while True:
    if any([event.type == QUIT for event in pygame.event.get()]):
        break

    screen.fill(BG_COLOR)

    keys_pressed = pygame.key.get_pressed()

    # right paddle controls
    if keys_pressed[K_UP] or keys_pressed[K_w]:
        human_paddle.rect.y -= SPEED_MULTIPLIER

    if keys_pressed[K_DOWN] or keys_pressed[K_s]:
        human_paddle.rect.y += SPEED_MULTIPLIER

    # computer logic
    if ball.rect.centerx < SCREEN_WIDTH / 2:
        if ball.rect.centery > computer_paddle.rect.centery:
            computer_paddle.rect.centery += SPEED_MULTIPLIER
        else:
            computer_paddle.rect.centery -= SPEED_MULTIPLIER

    elif abs(computer_paddle.rect.centery - SCREEN_HEIGHT / 2) > SCREEN_HEIGHT / 10:
        if computer_paddle.rect.centery > SCREEN_HEIGHT / 2:
            computer_paddle.rect.centery -= SPEED_MULTIPLIER
        else:
            computer_paddle.rect.centery += SPEED_MULTIPLIER

    # move the ball
    if ball.rect.colliderect(computer_paddle.rect) \
            or ball.rect.colliderect(human_paddle.rect):
        ball.dx *= -1
        
        # while ball isn't touching either paddle
        while not (ball.rect.colliderect(computer_paddle.rect)
                   or ball.rect.colliderect(human_paddle.rect)):
            ball.rect.x += ball.dx

        ball.dy += random.uniform(-0.2, 0.2)  # random bounces

    if not (0 < ball.rect.y and ball.rect.y + ball.rect.height < SCREEN_HEIGHT):
        ball.dy *= random.uniform(-1.2, -0.8) # random bounces

    # check if game is over
    if not 0 < ball.rect.x < SCREEN_WIDTH:
        break

    sprites.update()
    sprites.draw(screen)
    pygame.display.flip()

    clock.tick(120)

pygame.quit()

Game window:

Answer 1

Nice effort, this is some good looking code!

---

init bug

        self.dy = random.choice((random.uniform(1, -0.7), random.uniform(0.7, 1)))

That doesn't make sense to me. During play we observe the initial vector is sometimes nearly horizontal. It appears you wanted

        self.dy = random.choice((random.uniform(-1, -0.7), random.uniform(0.7, 1)))

Such a "give magic number twice" bug would have been unlikely if you'd instead phrased it

        self.dy = random.choice((-1, 1)) * random.uniform(0.7, 1)

---

symmetry

Some of your magic numbers are just fine as-is. For example, starting the ball at center of screen seems a perfectly natural choice.

This expression seems weird:

human_paddle = Paddle(
    SCREEN_WIDTH - 2 * PADDLE_X, ...

I would have expected SCREEN_WIDTH - PADDLE_X - PADDLE_WIDTH, for symmetry with computer_paddle.

---

extract helper

There's not a lot to DRY up here, but to make the logic more readable I do recommend you define this predicate:

def is_colliding() -> bool:
    return (ball.rect.colliderect(computer_paddle.rect)
         or ball.rect.colliderect(human_paddle.rect))

Consider defining an is_vertically_in_bounds predicate, just to improve legibility of not is_vertically_in_bounds(), as the current code doesn't exactly read like an English sentence. No biggie. Or consider applying De Morgan to the bounds check. Why? The computer doesn't care which way you phrase it, but humans do better when reasoning about the positive instead of the negative.

---

bounds check

OTOH, the horizontally in bounds test appears to be buggy. On the left, it's fine. But on the right you want to pay the same careful attention to detail as when you were doing the vertical check. That is, the ball.rect.x + ball.rect.width right-hand side is what matters, rather than the left-hand side that you currently check.

During play we occasionally see glitches where ball bounces behind the human's paddle and re-enters play.

The ball's horizontal movement usually happens, very nicely, in this way:

    def update(self):
        self.rect.x += self.dx * SPEED_MULTIPLIER

I don't understand this business:

    # move the ball
    if is_colliding():
        ball.dx *= -1
        
        # while ball isn't touching either paddle
        while not is_colliding():
            ball.rect.x += ball.dx

(I paraphrased slightly, to simplify.)

I assume the goal was to ensure that we are not is_colliding() when we emerge from that code. So the negation of while not doesn't make sense -- surely we wanted the positive instead?

You would better reveal Author's Intent if you appended

        assert not is_colliding()

after that code. Plus, if there are glitches that would help you chase them down.

---

refactor

The main loop is a little on the long side -- we have to scroll vertically to read all of it.

Consider breaking out helpers:

• def read_keyboard(),
• def move_paddles(), and maybe even
• def adjust_ball_speed().

---

This code achieves most of its design goals.

I would be willing to delegate or accept maintenance tasks on it.