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)
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Feb 7, 2019 at 14:00
\$\endgroup\$
2
  • \$\begingroup\$ If there is a significant known bug, then you should fix it before asking for a review. \$\endgroup\$ Commented Feb 7, 2019 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\$ Commented Feb 7, 2019 at 14:07

1 Answer 1

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.

answered Feb 8, 2019 at 15:31
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.