6
\$\begingroup\$

I have been studying Python for a few months and just recently decided to stop avoiding OOP and reworked a Pong game to a more object oriented style. Please tell me how I should improve my code.

Some of the code is an adaptation from this Stack Overflow question about menu state.

There is a known problem with the MenuScene.handle_events() function, such that I need to press the space or return key multiple times to it catch the event, but I don't consider it to be a significant bug.

main.py

from __future__ import absolute_import
import pygame   
from core.config import Colors, Globals
from core.scene import GameScene, SceneManager

def main():
    screen = pygame.display.set_mode((Globals.win_width,Globals.win_height))
    pygame.display.set_caption("Pong")
    clock = pygame.time.Clock()
    manager = SceneManager()
    running= True
    pygame.init()
    while running:
        clock.tick(120)
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                running = False
        screen.fill(Colors.black)
        manager.scene.render(screen)
        manager.scene.handle_events(pygame.event.get())
        manager.scene.update()
        pygame.display.flip()

    pygame.quit()

if __name__ == "__main__":
    main()

scene.py

from .actors import Player,Enemy,Ball
from .config import Colors, Globals
import pygame

class SceneManager(object):
    def __init__(self):
        self.go_to(MenuScene())

    def go_to(self, scene):
        self.scene = scene
        self.scene.manager = self

class Scene(object):
    def __init__(self):
        pass
    def render(self, screen):
        raise NotImplementedError
    def update(self):
        raise NotImplementedError
    def handle_events(self, events):
        raise NotImplementedError

class MenuScene(Scene):
    def __init__(self):
        super(MenuScene,self).__init__()
        pygame.font.init()
        self.font = pygame.font.SysFont('Arial', 56)
        self.sfont = pygame.font.SysFont('Arial', 32)
        pass
    def render(self, screen):      
        screen.fill(Colors.green)
        text1 = self.font.render('Pong Rework', True, (255, 255, 255))
        text2 = self.sfont.render('> press SPACE to start <', True, (255, 255, 255))
        screen.blit(text1, (200, 50))
        screen.blit(text2, (200, 350))
    def update(self):
        pass
    def handle_events(self,events):
        for e in events:
            if e.type == pygame.KEYDOWN and (e.key == pygame.K_SPACE or e.key == pygame.K_RETURN):
                self.manager.go_to(GameScene()) 

class GameScene(Scene):
    def __init__(self):
        super(GameScene, self).__init__()
        pygame.font.init()
        self.font = pygame.font.SysFont("Comic Sans MS", 30)
        self.player = Player()
        self.enemy = Enemy()
        self.points ={"player": 0, "enemy": 0}
        self.player_score=self.font.render("{}".format(self.points["player"]),1,Colors.white)
        self.enemy_score=self.font.render("{}".format(self.points["enemy"]),1, Colors.white)       
        self.ball = Ball()

    def render(self,screen):
        screen.blit(self.player_score,(150,100))
        screen.blit(self.enemy_score,(630,100))
        pygame.draw.rect(screen,Colors.white,self.player)
        pygame.draw.rect(screen,Colors.white,self.enemy)
        pygame.draw.rect(screen,Colors.white,self.ball)

    def update(self):
        pressed = pygame.key.get_pressed()
        up,down = [pressed[key] for key in (pygame.K_UP, pygame.K_DOWN)]
        self.handle_point()
        self.player.update(up,down)
        self.enemy.update(self.ball.y)
        self.ball.update(self.player,self.enemy)
        return

    def handle_events(self, events):
        for event in events:
            if event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE:
                pass

    def handle_point(self):

        def update_points(key) :
            self.points[key] +=1
            self.player_score=self.font.render("{}".format(self.points["player"]),1,Colors.white)
            self.enemy_score=self.font.render("{}".format(self.points["enemy"]),1, Colors.white)

        if self.ball.x <= self.ball.width:
            update_points("enemy")
            self.ball.reset()                     
        if self.ball.x >= (Globals.win_width + self.ball.width):
            update_points("player")
            self.ball.reset()
            self.ball.dir_x *= -1  

actors.py

import pygame
from .config import Globals, Colors
from math import cos,sin,radians,pi
class Player(pygame.Rect):
    def __init__(self):
        super(Player,self).__init__(20,225,20,150)
        self.velocity = 5
        print("player class initated")
    def update(self,up,down):
        if up and self.y >= 10:
            self.y -= self.velocity
        if down and self.y <= Globals.win_height - (self.height +10):
            self.y += self.velocity
        pass

class Enemy(pygame.Rect):
    def __init__(self):
        super(Enemy,self).__init__(760,225,20,150)
        self.velocity = 3
        print("enemy class initated")
    def update(self,ballYpos):
        middle = self.y + self.height /2
        if ballYpos != middle:
            if ballYpos > middle and self.y <= Globals.win_height- (self.height+self.velocity):
                self.y += self.velocity
            if ballYpos < middle and self.y >= self.velocity*2:
                self.y -= self.velocity


class Ball(pygame.Rect):
    def __init__(self):
        super(Ball,self).__init__(400,300,20,20)
        self.velocity = 5
        self.angle = radians(0)
        self.dir_x = cos(self.angle)
        self.dir_y = -sin(self.angle)
        print("Ball class instancieted")
    def reset(self):
        self.x =400
        self.y = 300
        self.angle = radians(0)
        self.dir_x = cos(self.angle)
        self.dir_y = -sin(self.angle)
    def update(self,player,enemy):       
        self.x += self.dir_x * self.velocity
        self.y += self.dir_y * self.velocity

        self.handle_bound_collision()
        self.handle_paddle_collision(player,enemy)

    def handle_bound_collision(self):
        if self.y <=  0 or self.y>= Globals.win_height - 10:
            self.dir_y*= -1.05

    def handle_paddle_collision(self,player,enemy):
        intersectY = self.y
        if self.colliderect(player):
            relativeIntersectY = (player.y + (player.height / 2) ) - intersectY
            normalizedRelativeIntersectY = relativeIntersectY / (player.height/2)
            self.angle = radians(normalizedRelativeIntersectY * 60)
            self.dir_x = cos(self.angle)
            self.dir_y = -sin(self.angle)

        if self.colliderect(enemy):
            relativeIntersectY = (enemy.y + (enemy.height/2)) - intersectY
            normalizedRelativeIntersectY = relativeIntersectY / (enemy.height/2) 
            self.angle = radians(normalizedRelativeIntersectY * 60)
            self.dir_x = -cos(self.angle)
            self.dir_y = sin(self.angle)

config.py

class Globals:
    win_width = 800
    win_height = 600

class Colors:
    white = (255,255,255)
    black = (0,0,0)
    red = (255,0,0)
    green = (0,255,0)
    blue = (0,0,255)
\$\endgroup\$
  • \$\begingroup\$ If there is a significant known bug, then you should fix it before asking for a review. \$\endgroup\$ – 200_success Feb 7 at 14:04
  • \$\begingroup\$ Sorry, the bug was not significant in my point of view as this is only a study case of oop, i shouldnt have mentioned it \$\endgroup\$ – Eduardo Gomero Feb 7 at 14:07
6
\$\begingroup\$

Gameplay:

By placing the paddle in a specific position I managed to break the game (see the image). I think you should add a random starting angle when the game restarts.

Also, as Wikipedia says the game should be finished once someone reaches eleven points.

I_broke_the_game.jpg

Code:

You violate some of the PEP 8 style recommendations, namely:


In some methods like GameScene.update or Player.update you have empty return or pass statements after code blocks. They are redundant and should be removed.


Consider changing the class Globals that consists of only width and height of the screen to a namedtuple:

import namedtuple

Size = namedtuple('Size', ['width', 'height'])
WINDOW_SIZE = Size(width=800, height=600)

So, you could use it in two different ways. As a tuple in main:

screen = pygame.display.set_mode(WINDOW_SIZE)

and as a class with width and height attributes, for example in Player.update:

if down and self.y <= WINDOW_SIZE.height - (self.height + 10):

In the config.py you have a class with colors, but pygame has a special class for that already: pygame.Color. For example, in main you would simply write:

screen.fill(pygame.Color('black'))

I think it would make sense to move lots of hardcoded values as fonts and dimensions of objects to the config.py file though. Also, be careful, some of your hardcoded values depend on each other, as in the Player class where you check if the paddle is going over the border. That 10 in if up and self.y >= 10: should be tied with the paddle's dimensions super(Player,self).__init__(20,225,20,150).

By the way, the last piece should be rewritten as super().__init__(20, 225, 20, 150). It's been like this since Python 3.0: PEP 3135 -- New Super.


In some places you convert integers to strings using format:

self.player_score=self.font.render("{}".format(self.points["player"]),1,Colors.white)

but it can be done by using str function:

self.player_score = self.font.render(str(self.points["player"]), 1, pygame.Color('white'))

Finally, don't print things like print("player class initated"). As these things are for debugging purposes, consider using logging module.


On overall, well done! I'm not a fan of OOP but it was easy to read and understand your code.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.