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 it turns out 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.