patternpythonMinor
Noughts and Crosses in Python using the pygame module
Viewed 0 times
themodulepygameusingpythonandnoughtscrosses
Problem
I've created a Noughts and Crosses game in Python using pygame, and I'm quite pleased with the result that I've produced. I've managed to reduce the code to what it is now, as it used to be much longer, and I'd like to submit it here for review. I know that it's common practice to not have more than 79 characters on any one line, but I'm not too bothered about this. I don't know what sort of things could possibly be done to improve the code further, or to correct any other things that I've done that go against the PEP 8 style guide.
```
import os, sys, pygame, classes.sprite #Imports the necessary modules as well a custom sprite class
from pygame.locals import *
pygame.init() #Initialises the pygame module
path, fname = os.path.split(__file__)
path = os.path.join(path, 'data') #Defines the path that stores all of the data (Images)
#Define some constants
WIDTH = 800
HEIGHT = 600
SCREEN = pygame.display.set_mode((WIDTH,HEIGHT))
CLOCK = pygame.time.Clock()
FPS = 30
FONT = 'LovelyK'
ICON = pygame.transform.scale(pygame.image.load(os.path.join(path, 'icon.png')).convert_alpha(), (32, 32))
tick = 0
#Initialise the custom sprite module
classes.sprite.init(SCREEN, FPS)
#Sets the game's icon and caption
pygame.display.set_icon(ICON)
pygame.display.set_caption('Noughts and Crosses')
#Some variables needed for the game
turn = 0
wins = [0, 0]
grid = [0, 0, 0,
0, 0, 0,
0, 0, 0]
winning_lines = [[0, 1, 2], [3, 4, 5], [6, 7, 8],
[0, 3, 6], [1, 4, 7], [2, 5, 8],
[0, 4, 8], [2, 4, 6]]
p1_colour, p2_colour = (237, 28, 36), ( 62, 72, 204)
def print_to_screen(text, position = (0, 0), centered = False, size = 30, colour = (255, 255, 255), font = 'Courier New'):
'''Prints text to the screen'''
FONT = pygame.font.SysFont(font, size)
label = FONT.render(str(text), 1, colour)
if centered == True:
position = label.get_rect(center = position)
SCREEN.blit(label, position)
def Quit()
```
import os, sys, pygame, classes.sprite #Imports the necessary modules as well a custom sprite class
from pygame.locals import *
pygame.init() #Initialises the pygame module
path, fname = os.path.split(__file__)
path = os.path.join(path, 'data') #Defines the path that stores all of the data (Images)
#Define some constants
WIDTH = 800
HEIGHT = 600
SCREEN = pygame.display.set_mode((WIDTH,HEIGHT))
CLOCK = pygame.time.Clock()
FPS = 30
FONT = 'LovelyK'
ICON = pygame.transform.scale(pygame.image.load(os.path.join(path, 'icon.png')).convert_alpha(), (32, 32))
tick = 0
#Initialise the custom sprite module
classes.sprite.init(SCREEN, FPS)
#Sets the game's icon and caption
pygame.display.set_icon(ICON)
pygame.display.set_caption('Noughts and Crosses')
#Some variables needed for the game
turn = 0
wins = [0, 0]
grid = [0, 0, 0,
0, 0, 0,
0, 0, 0]
winning_lines = [[0, 1, 2], [3, 4, 5], [6, 7, 8],
[0, 3, 6], [1, 4, 7], [2, 5, 8],
[0, 4, 8], [2, 4, 6]]
p1_colour, p2_colour = (237, 28, 36), ( 62, 72, 204)
def print_to_screen(text, position = (0, 0), centered = False, size = 30, colour = (255, 255, 255), font = 'Courier New'):
'''Prints text to the screen'''
FONT = pygame.font.SysFont(font, size)
label = FONT.render(str(text), 1, colour)
if centered == True:
position = label.get_rect(center = position)
SCREEN.blit(label, position)
def Quit()
Solution
I'd remove those comments, because they don't really add anything (having something like
The constants for
You're not using
The line
And later you do:
So there's no need for the
The
The function
When the game ends you're resetting the grid (which you can do with a
Checking the win/loss can be done differently (maybe with sums, or concatenated strings) but even the way it's now, I think it's good enough.
Initialises the pygame module near pygame.init() is not really helpful). The only one I'd keep is clicked = False # Used to stop holding the mouse down from affecting the program because you're actually explaining why you're doing something, not what.The constants for
WIDTH and HEIGHT are used only once, so there's no real need for them.You're not using
fname anywhere, you can just remove it and use something likepath = os.path.split(__file__)[0]The line
p1_colour, p2_colour = (237, 28, 36), (62, 72, 204) suggests that you want an array or list, so just use it. If you want to change based on both turn and player, you can use lists.colours = [[(237, 28, 36), (36, 44, 134)], [(62, 72, 204), (36, 44, 134)]]And later you do:
print_to_screen('P1', (40, 200), False, 250, colours[0][turn], FONT)
print_to_screen('P2', (570, 200), False, 250, colours[1][turn], FONT)
print_to_screen(wins[0], (110, 550), True, 100, colours[0][turn], FONT)
print_to_screen(wins[1], (670, 550), True, 100, colours[1][turn], FONT)So there's no need for the
if turn == 0: when setting colours.The
test_for_draw() is useless, you can just keep track of how many turns are happening, because the only possible draw is when the players made 9 moves in total.The function
Quit() has a bad name, because it looks as if you want to quit the game, while in reality you're checking. Maybe rename it quit_if_needed() ?When the game ends you're resetting the grid (which you can do with a
grid = [0] * 9), but you're not resetting the turn, so you don't know which player will be the first, next time (you could say it's the loser of the previous game, but what if it's a draw?).Checking the win/loss can be done differently (maybe with sums, or concatenated strings) but even the way it's now, I think it's good enough.
Code Snippets
print_to_screen('P1', (40, 200), False, 250, colours[0][turn], FONT)
print_to_screen('P2', (570, 200), False, 250, colours[1][turn], FONT)
print_to_screen(wins[0], (110, 550), True, 100, colours[0][turn], FONT)
print_to_screen(wins[1], (670, 550), True, 100, colours[1][turn], FONT)Context
StackExchange Code Review Q#157869, answer score: 6
Revisions (0)
No revisions yet.