HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Pong in Python and Pygame

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pythonpygameandpong

Problem

I'm new to programming. This is my first project where I tried to use object-oriented programming. I'm looking for all kinds of comments.

```
import pygame, sys
from pygame.locals import *
from random import choice, random

INDIAN_RED = (205, 92, 92)
DARK_SALMON = (233, 150, 122)
DIM_GRAY = (105, 105, 105)
BLACK = (0, 0, 0)

SCREEN_HEIGHT = 300
SCREEN_WIDTH = 400

DISPLAYSURF = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT), 0)

PADDLE_HEIGHT = 50
PADDLE_WIDTH = 4

PADDLE_VEL = 7

# enum values
UP = "gora"
DOWN = "dol"

points_r = 0
points_l = 0

class Paddle:
def __init__(self, x, y, clr, vel):
self.x = x
self.y = y
self.height = PADDLE_HEIGHT
self.width = PADDLE_WIDTH
self.color = clr
self.vel = vel

self.is_moving = False
self.direction = None

def draw(self):
leftx = self.x - self.width/2
topy = self.y - self.height/2
coords = pygame.Rect(leftx, topy, self.width, self.height)
pygame.draw.rect(DISPLAYSURF, self.color, coords, 0)

def move(self):
if not self.is_moving:
return

if self.direction == UP and self.y - self.height/2 > 0:
self.y -= self.vel
elif self.direction == DOWN and self.y + self.height/2 SCREEN_HEIGHT:
self.vely = - self.vely

if self.x - self.r SCREEN_WIDTH:
if collide(paddle_r, self):
self.velx = - self.velx
else:
self.reset()
global points_l
points_l += 1

def reset(self):
self.velx = choice(range(self.vel//2, self.vel))
self.vely = choice(range(self.vel//2, self.vel))

if random() > 0.5:
self.velx *= -1
if random() > 0.5:
self.vely *= -1

self.x = SCREEN_WIDTH//2
self.y = SCREEN_HEIGHT//2

def collide(paddle, ball):
if abs(ball.y - paddle.y) < paddle.height/2 + ball.r:
return True
el

Solution

Generally looks pretty good. A few things stick out:

if abs(ball.y - paddle.y) < paddle.height/2 + ball.r:
    return True
else:
    return False


is clearer as just:

return abs(ball.y-paddle.y) < paddle.height/2 + ball.r


Also, lots of repetition is a code smell, and

if event.type == pygame.KEYDOWN:
        if event.key == pygame.K_UP:
            paddle_r.is_moving = True
            paddle_r.direction = UP
        elif event.key == pygame.K_DOWN:
            paddle_r.is_moving = True
            paddle_r.direction = DOWN
        if event.key == pygame.K_w:
            paddle_l.is_moving = True
            paddle_l.direction = UP
        elif event.key == pygame.K_s:
            paddle_l.is_moving = True
            paddle_l.direction = DOWN


is the same as:

MOVEKEYS = { pygame.K_UP  : (paddle_r, UP), 
             pygame.K_w   : (paddle_l, UP),
             pygame.K_DOWN: (paddle_r, DOWN),
             pygame.K_s   : (paddle_l, DOWN) }

if event.type == pygame.KEYDOWN and event.key in MOVEKEYS:
    paddle, direction = MOVEKEYS[event.key]
    paddle.is_moving = True
    paddle.direction = direction


Note: fewer if statements and a lot less repetition. It makes the KEY_UP block easier too:

elif event.type == pygame.KEYUP and event.key in MOVEKEYS:
    paddle, _ = MOVEKEYS[event.key]
    paddle.is_moving = False

Code Snippets

if abs(ball.y - paddle.y) < paddle.height/2 + ball.r:
    return True
else:
    return False
return abs(ball.y-paddle.y) < paddle.height/2 + ball.r
if event.type == pygame.KEYDOWN:
        if event.key == pygame.K_UP:
            paddle_r.is_moving = True
            paddle_r.direction = UP
        elif event.key == pygame.K_DOWN:
            paddle_r.is_moving = True
            paddle_r.direction = DOWN
        if event.key == pygame.K_w:
            paddle_l.is_moving = True
            paddle_l.direction = UP
        elif event.key == pygame.K_s:
            paddle_l.is_moving = True
            paddle_l.direction = DOWN
MOVEKEYS = { pygame.K_UP  : (paddle_r, UP), 
             pygame.K_w   : (paddle_l, UP),
             pygame.K_DOWN: (paddle_r, DOWN),
             pygame.K_s   : (paddle_l, DOWN) }

if event.type == pygame.KEYDOWN and event.key in MOVEKEYS:
    paddle, direction = MOVEKEYS[event.key]
    paddle.is_moving = True
    paddle.direction = direction
elif event.type == pygame.KEYUP and event.key in MOVEKEYS:
    paddle, _ = MOVEKEYS[event.key]
    paddle.is_moving = False

Context

StackExchange Code Review Q#139211, answer score: 4

Revisions (0)

No revisions yet.