4
\$\begingroup\$

I've been learning to code in my spare time for a couple of years. I have covered object oriented stuff before and can usually understand other people's object oriented code. This is the first time I've written an object oriented program from scratch.

It has all the functionality that I want and (hopefully) no mistakes - I have stress tested it.

However, I still feel the code is a bit dirty, so I'd like some suggested improvements. Some areas that I feel I've gone wrong:

1, I have put too much functionality into the board class and not enough into the player class. Is this an issue?

2, It was relatively streamlined to begin with, but then I started adding stuff to "make it look cool" such as a coin toss to decide who goes first and drawing the winning line on the board/displaying a victory message. This is all great, but my code seems to have gotten less clean as the game loop function is a bit messy - any recommendations for how to repackage it?

3, Any other comments?

4, Any recommendations for a next project that would be slightly harder but still manageable for someone of my level.

My code:

import random
import pygame
import itertools
# Initailize PyGame object
pygame.init()
DISPLAY_WIDTH = 600
DISPLAY_HEIGHT = 600
BLOCK_SIZE = 200
# Defining colors (rgb values)
BACKGROUND_COLOR = (0, 0, 0)
white = (255, 255, 255)
black = (0, 0, 0)
red = (255, 0, 0)
blue = (0, 0, 255)
green = (0, 255, 0)
pink = (255, 20, 147)
cyan = (0, 255, 255)
gray = (128, 128, 128)
# Draw black rectangle to cover square numbers once played
def draw_rect(x, y, color = black, size=30, width = 0):
 pygame.draw.polygon(gameDisplay, color, [(x-size, y-size), (x-size, y+size), (x+size, y+size), (x+size, y-size)], width)
def draw_wide_rect(x, y, color = black, size=30, width = 0):
 pygame.draw.polygon(gameDisplay, color, [(x-3*size, y-2*size), (x-3*size, y+2*size), (x+3*size, y+2*size), (x+3*size, y-2*size)], width)
# How to draw naughts and crosses
def draw_naught(x, y, color, radius = 40, thickness = 20):
 pygame.draw.circle(gameDisplay, color, (x,y), radius, thickness)
def draw_cross(x, y, color, closed = False, thickness = 20):
 pygame.draw.lines(gameDisplay, color, closed, [(x-30, y-30), (x+30, y+30)], thickness)
 pygame.draw.lines(gameDisplay, color, closed , [(x+30, y-30), (x-30, y+30)], thickness)
largefont = pygame.font.SysFont(None, 80)
player_font = pygame.font.SysFont('Comic Sans MS', 40)
def display_text(text, x, y, color):
 text = largefont.render("{}".format(str(text)), True, color)
 gameDisplay.blit(text, [x, y])
# Set up the display
gameDisplay = pygame.display.set_mode((DISPLAY_WIDTH, DISPLAY_HEIGHT + 100))
pygame.display.set_caption("Naughts and Crosses")
clock = pygame.time.Clock()
# Draw the background grid 
def drawGrid(w, rows, surface):
 sizeBtwn = w // rows
 
 x = 0
 y = 0
 pygame.draw.line(gameDisplay, white, (x,0),(x,w), 10)
 pygame.draw.line(gameDisplay, white, (0,y),(w,y), 10)
 for l in range(rows):
 x = x + sizeBtwn
 y = y + sizeBtwn
 
 pygame.draw.line(gameDisplay, white, (x,0),(x,w), 10)
 pygame.draw.line(gameDisplay, white, (0,y),(w,y), 10)
class Board:
 def __init__(self, current_player, next_player, naught_list = [], cross_list = []):
 self.naught_list = naught_list
 self.cross_list = cross_list
 self.empty_list = [1,2,3,4,5,6,7,8,9]
 self.current_player = current_player
 self.next_player = next_player
 self.naught_trebles = []
 self.naught_treble_sums = []
 self.cross_trebles = []
 self.cross_treble_sums = []
 self.magic_dic = {1: 6,
 2: 1,
 3: 8,
 4: 7,
 5: 5,
 6: 3,
 7: 2,
 8: 9,
 9: 4}
 def create_board(self):
 gameDisplay.fill(BACKGROUND_COLOR)
 drawGrid(DISPLAY_WIDTH, int(DISPLAY_WIDTH / BLOCK_SIZE), gameDisplay)
 
 for i in range(9):
 display_text(str(i+1), ((i)%3)*BLOCK_SIZE + BLOCK_SIZE/2.5, (i//3)*BLOCK_SIZE + BLOCK_SIZE/2.5, white)
 
 def cover_text(self, square_num):
 x = int((square_num%3)*BLOCK_SIZE + BLOCK_SIZE/2)
 y = int((square_num//3)*BLOCK_SIZE + BLOCK_SIZE/2)
 def play_move(self, marker, square_num, player):
 if player.marker == 'naughts':
 if int(square_num) in self.empty_list:
 idx = square_num - 1 # user numbering starts at 1 but Python starts at 0
 x = int((idx%3)*BLOCK_SIZE + BLOCK_SIZE/2)
 y = int((idx//3)*BLOCK_SIZE + BLOCK_SIZE/2)
 draw_rect(x, y)
 draw_naught(x, y, player.color)
 self.empty_list.remove(square_num)
 self.naught_list.append(square_num)
 tmp = player
 self.current_player = self.next_player
 self.next_player = tmp
 else:
 print("Invalid move - square already taken. Pick again please:")
 return 0 
 elif player.marker == 'crosses':
 if int(square_num) in self.empty_list:
 idx = square_num - 1 # user numbering starts at 1 but Python starts at 0
 x = int((idx%3)*BLOCK_SIZE + BLOCK_SIZE/2)
 y = int((idx//3)*BLOCK_SIZE + BLOCK_SIZE/2)
 draw_rect(x, y)
 draw_cross(x, y, player.color)
 self.empty_list.remove(square_num)
 self.cross_list.append(square_num)
 tmp = player
 self.current_player = self.next_player
 self.next_player = tmp
 else:
 print("Invalid move - square already taken. Pick again please:")
 return 0 
 else:
 print("Invalid marker for this player.")
 def victory_line(self, winning_list, color, closed = False, thickness = 20):
 # get the square numbers of the winning line. These will be in terms of magic square from magic_dic
 magic_list_0 = winning_list[0]
 magic_list_1 = winning_list[1]
 magic_list_2 = winning_list[2]
 # convert them back to the square numbers we see on the screen
 for key in self.magic_dic:
 if magic_list_0 == self.magic_dic[key]:
 list_0 = key
 for key in self.magic_dic:
 if magic_list_1 == self.magic_dic[key]:
 list_1 = key
 for key in self.magic_dic:
 if magic_list_2 == self.magic_dic[key]:
 list_2 = key
 non_magic_list = [list_0, list_1, list_2]
 
 # subtract 1 as pygame numbers squares from 0 and we number from 1
 left_idx = min(non_magic_list) - 1
 right_idx = max(non_magic_list) -1 
 
 # get coords
 left_x = int((left_idx%3)*BLOCK_SIZE + BLOCK_SIZE/2)
 left_y = int((left_idx//3)*BLOCK_SIZE + BLOCK_SIZE/2)
 right_x = int((right_idx%3)*BLOCK_SIZE + BLOCK_SIZE/2)
 right_y = int((right_idx//3)*BLOCK_SIZE + BLOCK_SIZE/2)
 # draw a line to show winning combination
 pygame.draw.lines(gameDisplay, color, closed , [(left_x, left_y), (right_x, right_y)], thickness)
 def magic_square_winner_check(self, player):
 # magic square has every row, column and diagonal sum to 15 meaning we can just check if any triple of used squares sums to 15
 # this won't be computationally challenging due to small board size. at most 5C3 = 10 combinations (no player can occupy more than 5 squares) 
 # current numbering is not magic so we must first use a dictionary to magic-ify the board
 if player.marker == 'naughts':
 updated_naught_list = [self.magic_dic[x] for x in self.naught_list]
 self.naught_trebles = list(itertools.combinations(updated_naught_list, 3))
 self.naught_treble_sums = [sum(x) for x in self.naught_trebles]
 if 15 in self.naught_treble_sums:
 print("Victory")
 return 1
 elif player.marker == 'crosses':
 updated_cross_list = [self.magic_dic[x] for x in self.cross_list]
 self.cross_trebles = list(itertools.combinations(updated_cross_list, 3))
 self.cross_treble_sums = [sum(x) for x in self.cross_trebles]
 if 15 in self.cross_treble_sums:
 print("Victory")
 return 1
 else:
 return 0
 def victory_message(self, msg, x = DISPLAY_WIDTH//5, y=DISPLAY_HEIGHT+20, color = cyan):
 text = largefont.render("{} wins!".format(str(msg)), True, color)
 gameDisplay.blit(text, [x, y])
 def draw_message(self, x = DISPLAY_WIDTH//6, y=DISPLAY_HEIGHT+20, color = cyan):
 text = largefont.render("A boring draw...", True, color)
 gameDisplay.blit(text, [x, y])
 def player1_text(self, color, x = 10, y=DISPLAY_HEIGHT+30):
 text = player_font.render("Player 1", True, color)
 gameDisplay.blit(text, [x, y])
 def player2_text(self, color, x = DISPLAY_WIDTH - 160, y= DISPLAY_HEIGHT+30):
 text = player_font.render("Player 2", True, color)
 gameDisplay.blit(text, [x, y])
class Player:
 def __init__(self, marker, coin_toss_victor, color, name):
 self.marker = marker # naughts or crosses
 self.coin_toss_victor = coin_toss_victor #boolean indication of who starts
 self.color = color
 self.name = name
 pass
 def request_move(self, board):
 move = input("Choose a square: ")
 tmp = move
 while move.isnumeric() == False:
 move = input("{} is not an int! Choose again: ".format(tmp))
 move = int(move)
 # check valid
 while move not in range(1,10):
 tmp = move
 move = int(input("{} is not a valid square. Please choose again: ".format(tmp)))
 while move not in board.empty_list:
 #board.square_taken_message()
 tmp = move 
 move = int(input("Square {} is already taken. Please choose again: ".format(tmp)))
 return move
"""<<<GAME LOOP>>>"""
def game_loop():
 # display screen asking for coin fkip guess
 gameDisplay.fill(BACKGROUND_COLOR)
 myfont = pygame.font.SysFont('Comic Sans MS', 50)
 textsurface = myfont.render("Heads or Tails?", True, white)
 gameDisplay.blit(textsurface, [60, 10])
 pygame.display.update()
 # receive coin guess
 coin = ["H", "T"]
 coin_toss = input("We will decide who goes first with a coin flip. Player 1 pease enter H or T: ")
 if coin_toss not in coin:
 coin_toss = input("Not a valid input. Please enter H or T: ")
 
 # dictionary for translating input to display words
 coin_dic = {"H" : "Heads", "T": "Tails"}
 # random toss. display the guess now. display the outcome after images
 random_toss = random.choice(coin)
 myfont = pygame.font.SysFont('Comic Sans MS', 50)
 textsurface2 = myfont.render("Your choice: {}".format(coin_dic[coin_toss]), True, white)
 gameDisplay.blit(textsurface2, [60, DISPLAY_HEIGHT - 140])
 
 # load two images of coin
 coin_a = pygame.image.load('coin_a.png')
 coin_b = pygame.image.load('coin_b.png')
 tosser = 0
 
 textsurface3 = myfont.render("Result: {}".format(coin_dic[random_toss]), True, white)
 if coin_toss == random_toss:
 textsurface4 = myfont.render("Win! You go first!", True, white)
 else:
 textsurface4 = myfont.render("Lose! You go second!", True, white)
 # change image 20 times for nice effect. display outcome of coin flip after 10 times.
 while tosser < 21:
 print(20 - tosser)
 gameDisplay.blit(coin_a, (60,DISPLAY_HEIGHT//3))
 pygame.display.update()
 pygame.time.wait(100)
 gameDisplay.blit(coin_b, (60, DISPLAY_HEIGHT//3))
 pygame.display.update()
 pygame.time.wait(100)
 tosser += 1
 if tosser > 10:
 gameDisplay.blit(textsurface3, [60, DISPLAY_HEIGHT - 70])
 gameDisplay.blit(textsurface4, [60, DISPLAY_HEIGHT])
 pygame.display.update()
 # short pause for user to read result before switching screen to tic-tac-toe
 pygame.time.wait(500)
 # instantiate the player objects according to the result of the coin toss
 while coin_toss not in coin:
 coin_toss = input("You didn't enter a valid input! Try again please: ")
 if coin_toss == random_toss:
 print("Coin toss result was {}".format(random_toss))
 print("Player 1 to go first!")
 p1 = Player('naughts', True, pink, 'Player 1')
 p2 = Player('crosses', False, green, 'Player 2')
 current_player = p1
 next_player = p2
 else:
 print("Coin toss result was {}".format(random_toss))
 print("Player 2 to go first!") 
 p1 = Player('naughts', False, pink, 'Player 1')
 p2 = Player('crosses', True, green, 'Player 2')
 current_player = p2
 next_player = p1
 # instantiate the board and display it
 b = Board(current_player, next_player)
 b.create_board()
 
 # display current player
 if current_player == p1:
 b.player1_text(white)
 b.player2_text(gray)
 elif current_player == p2:
 b.player1_text(gray)
 b.player2_text(white)
 pygame.display.update()
 # begin while loop - initialise all variables here so they're reset if we restart the game
 play_again = True
 end = False
 b.empty_list = [1,2,3,4,5,6,7,8,9]
 b.naught_list = []
 b.naught_trebles = []
 b.naught_treble_sums = []
 b.cross_list = []
 b.cross_trebles = []
 b.cross_treble_sums = []
 while play_again:
 
 # display current player
 if current_player == p1:
 b.player1_text(white)
 b.player2_text(gray)
 pygame.display.update()
 elif current_player == p2:
 b.player1_text(gray)
 b.player2_text(white)
 pygame.display.update()
 # allow window to be quit
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 play_again = False
 # print useful stuff to terminal
 print("-----Empty Squares-----")
 print(b.empty_list)
 print("-----Naughts-----")
 print(b.naught_list)
 print(b.naught_trebles)
 print(b.naught_treble_sums)
 print("-----Crosses-----")
 print(b.cross_list)
 print(b.cross_trebles)
 print(b.cross_treble_sums)
 
 # request player move and display it
 move = current_player.request_move(b)
 b.play_move(current_player.marker, move, current_player)
 # display current player
 if current_player == p1:
 b.player1_text(white)
 b.player2_text(gray)
 pygame.display.update()
 elif current_player == p2:
 b.player1_text(gray)
 b.player2_text(white)
 pygame.display.update()
 # check if current player has won and then switch player
 if current_player == p1 and end == False:
 if b.magic_square_winner_check(p1) == 1:
 draw_wide_rect(70, DISPLAY_HEIGHT+70)
 draw_wide_rect(DISPLAY_WIDTH - 80, DISPLAY_HEIGHT+70)
 b.victory_message(current_player.name)
 winning_idx = b.naught_treble_sums.index(15)
 winning_tuple = b.naught_trebles[winning_idx]
 winning_list = [x for x in winning_tuple]
 print(winning_list)
 b.victory_line(winning_list, p1.color)
 pygame.display.update()
 end = True
 else:
 current_player = p2
 elif current_player == p2 and end == False:
 if b.magic_square_winner_check(p2) == 1:
 draw_wide_rect(70, DISPLAY_HEIGHT+70)
 draw_wide_rect(DISPLAY_WIDTH - 80, DISPLAY_HEIGHT+70)
 b.victory_message(current_player.name)
 winning_idx = b.cross_treble_sums.index(15)
 winning_tuple = b.cross_trebles[winning_idx]
 winning_list = [x for x in winning_tuple]
 b.victory_line(winning_list, p2.color)
 pygame.display.update()
 end = True
 else:
 current_player = p1
 # check if game is a draw
 if len(b.empty_list) == 0 and end == False:
 b.draw_message()
 pygame.display.update()
 end = True
 # if game reaches a terminal state, return 1 (used as code in main loop to ask about restart)
 if end:
 return 1
 pygame.display.update()
def main():
 end_msg = game_loop()
 while end_msg == True:
 play_again = int(input("Play again? 1 = Yes, 0 = No : "))
 if play_again == 1:
 game_loop()
 if play_again == 0:
 break
if __name__ == '__main__':
 main()
toolic
14.4k5 gold badges29 silver badges201 bronze badges
asked Jan 13, 2019 at 11:22
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

UX

When I run the code in my shell, a GUI window immediately pops up, and it is in front of my shell. This means I can't see the the input prompts in my shell until I move the GUI window out of the way.

It is confusing for the user to interact with both a GUI and the shell. Consider one or the other. Since it is nice to have the GUI, perhaps the prompts can be added to the GUI instead of the shell.

Input

Since the code calls input several times, it is worth considering using pyinputplus. This offers many features for validating user input.

It is convenient that you allow the user to just type a a single letter (H or T) at the prompt:

coin_toss = input("We will decide who goes first with a coin flip. Player 1 pease enter H or T: ")

However, it would be even more convenient if you accepted case-insensitive input (h or t) using the upper function:

coin_toss = input("We will decide who goes first with a coin flip. Player 1 please enter H or T: ").upper()

Note that I fixed the typo "pease".

Layout

You should add blank lines after each function

def draw_rect(x, y, color = black, size=30, width = 0):
 pygame.draw.polygon(gameDisplay, color, [(x-size, y-size), (x-size, y+size), (x+size, y+size), (x+size, y-size)], width)
def draw_wide_rect(x, y, color = black, size=30, width = 0):

For example:

def draw_rect(x, y, color = black, size=30, width = 0):
 pygame.draw.polygon(gameDisplay, color, [(x-size, y-size), (x-size, y+size), (x+size, y+size), (x+size, y-size)], width)
def draw_wide_rect(x, y, color = black, size=30, width = 0):

Move the classes to the top after the import lines. Also move the other functions after the classes. Having them in the middle of the code interrupts the natural flow of the code (from a human readability standpoint).

Documentation

The PEP 8 style guide recommends adding docstrings for classes and functions. You can replace your comments with docstrings:

def draw_rect(x, y, color = black, size=30, width = 0):
 """ Draw black rectangle to cover square numbers once played """

Simpler

You can use f-strings to simplify the code:

move = int(input(f"{tmp} is not a valid square. Please choose again: "))

In the play_move function, there is no need for the tmp variable:

tmp = player
self.current_player = self.next_player
self.next_player = tmp

This is simpler:

self.current_player = self.next_player
self.next_player = player

The same can be done in the request_move function.

This line:

b.empty_list = [1,2,3,4,5,6,7,8,9]

is simpler using range:

b.empty_list = list(range(1, 10))

Since there are only 2 players, all the if/elif statements like this:

 if player.marker == 'naughts':
 elif player.marker == 'crosses':

can be simplified as if/else:

 if player.marker == 'naughts':
 else:

Partitioning

1, I have put too much functionality into the board class and not enough into the player class. Is this an issue?

It is not an issue, but there may be an opportunity to simplify the code here.

Naming

The PEP 8 style guide recommends snake_case for function and variable names.

For example, the function drawGrid would be draw_grid.

Also, lower-case l is a bad name for a variable because it is easily confused with the number 1, not to mention that it conveys little meaning:

for l in range(rows):

Since you don't even use the variable, it is better to use the _ placeholder instead:

for _ in range(rows):

While we're looking at this function, lines like this:

x = x + sizeBtwn

are simpler using the special assignment operators:

x += sizeBtwn

Tools

You could run code development tools to automatically find some style issues with your code.

ruff identifies a few that you can address.

answered Feb 13 at 10:54
\$\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.