Tic-tac-toe game using Pygame

Question

I programmed a tic-tac-toe game using pygame. It's the first game I make. I'd really appreaciate any comments you have about the coding style and any improvements that could be made.

import pygame
import graphics # External module graphics.py
import logic # External module logic.py
# Setting up window
HEIGHT = 600
WIDTH = 480
FPS = 30
TITLE = "Tic-tac-toe"
# Defining colors
BLACK = (0, 0, 0)
GREEN = (0, 255, 0)
# Initializing pygame and create window
pygame.init()
pygame.mixer.init()
screen = pygame.display.set_mode((WIDTH, HEIGHT))
pygame.display.set_caption(TITLE)
clock = pygame.time.Clock()
# Game settings
TOP = 100
SIZE = 450
CELL = SIZE // 3
CPU_SPEED = FPS // 2
class TicTacToe():
 """Main tic-tac-toe game object.
 Attributes
 ----------
 board : graphics.Board
 pygame.Sprite object. Consists mainly of the lines that make
 up the grid.
 board_logic : logic.Board
 Object that defines the game logic and AI.
 cells : 3 x 3 nested list.
 List containing individual graphics.Cell objects.
 Each `Cell` object is a cell sprite.
 player_score : graphics.Text
 Sprite object that shows the player score.
 cpu_score : graphics.Text
 Sprite object that shows the cpu score.
 end_message : graphics.Text
 Sprite object that shows a message when the game ends:
 win/lose/tie.
 player_play : bool
 Determine when it's the player's turn to play.
 check : bool
 Determine when the check the end of the game.
 Set to `True` after each player or cpu move.
 reset : bool
 Determines when to reset the board after the end of game.
 timer : int
 Counter to limit cpu moves speed.
 timer2 : int
 Counter to limit end game reactions speed.
 Methods
 -------
 add_to_group(sprites_group)
 Add all of the sprite objects of the `TicTacToe` object
 to `sprites_group`.
 create_cells(CELL)
 Create 9 `graphics.Cell` objects of size `SIZE` x `SIZE`
 and append them to `self.cells`.
 update()
 Check if the game has ended or a player has won. Calls
 methods accordingly. Verify when to check with `self.check`
 and limit frequency using `self.timer2`.
 Verifies it's the computer's turn to play by checking
 `self.player_turn`. Calls `computer_turn` method when
 corresponds. Limits computer's play speed using
 `self.timer`.
 computer_turn()
 Use `graphics.board_logic.computer_turn()` method to determine
 next case to fill.
 Fill cell accordingly and update to check board end game
 conditions and player's turn.
 reset_game()
 Reset `graphics.Board` to delete extra lines on the board.
 Reset all `graphics.Cell` sprites to a blank surface.
 Reset `self.end_message` to show an empty string.
 update_score()
 Add one point to the corresponding current player score.
 Either `self.cpu_score` or `self.player_score`.
 win_message()
 Modify `self.end_message` to show a win or lose message.
 tie_message()
 Modify `self.end_message` to show a tied game message.
 get_cells():
 Return `self.cells` containing all the `graphics.Cell`
 sprite objects.
 is_player_turn():
 Return `True` when player's turn conditions are met.
 `False` otherwise.
 to_reset():
 Return `self.reset` attribute. Use to determine when
 to reset the game board.
 """
 def __init__(self):
 self.board = graphics.Board()
 self.board_logic = logic.Board()
 self.cells = self.create_cells(CELL)
 self.player_score = graphics.Text('P1 : 0',
 (WIDTH * 4 // 5, TOP * 1 // 3))
 self.cpu_score = graphics.Text('CPU : 0',
 (WIDTH * 4 // 5, TOP * 2 // 3))
 self.end_message = graphics.Text('',
 (WIDTH * 2 // 5, TOP // 2))
 self.player_play = False
 self.check = False
 self.reset = False
 self.timer = 0
 self.timer2 = 0
 def add_to_group(self, sprites_group):
 sprites_group.add(self.player_score)
 sprites_group.add(self.cpu_score)
 sprites_group.add(self.end_message)
 for cell in self.cells:
 sprites_group.add(cell)
 sprites_group.add(self.board)
 def create_cells(self, CELL=CELL):
 cells = []
 for i in range(3):
 row = []
 for j in range(3):
 pos = (self.board.rect.left + j * CELL,
 self.board.rect.top + i * CELL)
 row.append(graphics.Cell(pos))
 cells.append(row)
 return cells
 def update(self):
 if self.check:
 if self.timer2 > CPU_SPEED // 2:
 self.timer2 = 0
 self.check = False
 if self.board_logic.check_winner():
 self.board.draw_triple(self.board_logic.win_line_pos())
 self.win_message()
 self.update_score()
 self.reset = True
 elif self.board_logic.endgame():
 self.tie_message()
 self.reset = True
 else:
 self.timer2 += 1
 if self.timer < CPU_SPEED:
 self.timer += 1
 else:
 self.timer = 0
 if not self.is_player_turn() and not self.to_reset():
 self.computer_turn()
 def computer_turn(self):
 i, j = self.board_logic.computer_turn()
 # print(self.board_logic)
 self.cells[i][j].computer_fill()
 self.check = True
 self.player_play = True
 def player_turn(self, ij):
 self.timer = 0
 self.board_logic.user_turn(ij)
 i, j = ij
 self.cells[i][j].player_fill()
 self.player_play = False
 self.check = True
 def reset_game(self):
 self.board.new_board()
 self.board_logic.reset()
 self.end_message.write('')
 for i in range(3):
 for j in range(3):
 self.cells[i][j].reset()
 self.reset = False
 def update_score(self):
 if self.board_logic.current_player() == 'o':
 self.player_score.add1()
 else:
 self.cpu_score.add1()
 def win_message(self):
 if self.board_logic.current_player() == 'o':
 self.end_message.write('You win!')
 else:
 self.end_message.write('You lose!')
 def tie_message(self):
 self.end_message.write(' Tie!')
 def get_cells(self):
 return self.cells
 def is_player_turn(self):
 return self.player_play and not self.board_logic.check_winner()
 def to_reset(self):
 return self.reset
all_sprites = pygame.sprite.Group()
game = TicTacToe()
game.add_to_group(all_sprites)
# Game loop
running = True
while running:
 clock.tick(FPS)
 # Process input
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 running = False
 if event.type == pygame.MOUSEBUTTONUP:
 if game.to_reset():
 game.reset_game()
 elif game.is_player_turn():
 pos = pygame.mouse.get_pos()
 cells = game.get_cells()
 for i in range(3):
 for j in range(3):
 if cells[i][j].hits(pos):
 game.player_turn((i, j))
 game.update()
 # Draw / render
 screen.fill(BLACK)
 all_sprites.draw(screen)
 pygame.display.flip()
pygame.quit()

I separated the code in different modules. The graphics module includes the classes to create the different sprites used by the game (the game board, the board cells and the text messages). The logic module is used for the computer AI.

I'm sharing only this "game part" of the code because this is where I'd appreciate your comments. If you'd like to check the other modules or get them in order to play the game, you can do it here.

Thank you very much for your time.

Answer 1

Static Code Analysis

Let me walk you through some of my favorite static code analysis tools:

• Type annotations : You don't use any. They are awesome and everybody should use them for Python 3.6+. As nobody should use Python versions below 3.6, just everybody should use them^^
• black and isort: For general formatting, I like to use the autoformatter black. Your code is already good when it comes to this / PEP8
• Flake8: May style checks. For example, the parameter to create_cells should be lowercase (cell, not CELL).
• McCabe: To check if there are parts which are hard to understand, I use python -m mccabe --min 5 yourscript.py. It only complains about the game loop.

Other comments

main function

I usually like to put everything in functions. You could put the game loop in a main function and then add:

if __name__ == "__main__":
 main()

This has the advantage that you can import parts of this file without executing it. This makes testing sometimes way easier.

Properties and YAGNI

Python has the property decorator. In your case, however, I would simply remove get_cells and access .cells directly. It makes the code easier to read and if you need it, you can still introduce the property later.

There is YAGNI - you ain't gonna need it. Don't create abstractions if you don't need them. The method to_reset seems to be a good example.

Docstring Style

Your docstring style is very similar to numpydoc; maybe you can change it to be consistent with that. This especially means that you don't need to document all methods within the class docstring. Instead, you document the method within the method. Many editors can then also use this to show help about the method.

Naming

Good names are hard to find. We want names that are clear in the context, but also not Java-style overly long names.

Here are some examples which I find hard to understand:

• timer2: Usually when you have to add numbers, that is a sign that it's not a good name
• player_play: Should be called is_players_turn, but that is already a method
• is_player_turn(): I'm not sure what self.board_logic.check_winner is doing, so I'm uncertain what a good name would be. But as this method is only called once, I'm wondering if the method itself is really necessary.

Enums

I don't know what self.board_logic.current_player() == "o" is doing, but maybe the method could return an enum and you could compare against an enum? String comparisons are prone to typo errors

• fabrizzio_gz: Thanks a lot for your review. I agree with most of your observations and I think they will improve the code. About getting rid of unnecessary methods (YAGNI), I'm following a recommendation of not accessing class attributes from outside the class. So to access self.cells, I use the self.get_cells() method. What's your take on this?
• Martin Thoma: "I'm following a recommendation of not accessing class attributes from outside the class" - I only see instance attributes and no class attributes in your example. I guess you mean instance attribute. Where does the recommendation come from? Why is it recommended? Have you read the article about the property decorator I've shared above?
• fabrizzio_gz: Yes, I meant instance attributes. The recommendation came from a MIT course. They suggested using getters and setters for attributes, I assume for encapsulation purposes. Similar to the java coding style the decorator article criticizes, actually. Maybe the best way to do that is using decorators, as the article suggests. I'm not very familiar with them but I will try looking into it. I also found a bit burdensome to create methods just to retrieve attributes.
• Martin Thoma: A lot of teachers come from the Java world and they are not aware of important differences to Python. For example, it is crucial to keep interfaces of a library constant (in any language**). Still, you want to be able to change something**. The only solution to that (in Java) is to make methods (getters and setters). In Python, you can use the property decorator. You can re-define what happens at attribute access. This removes the need for a getter / setter.
• fabrizzio_gz: I agree. It seems the pythonic way of doing it. You have given me many resources to learn more and improve. Thank you very much.