Noughts and Crosses in Python using the pygame module

Question

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():
    '''Shuts down the game if it's closed'''
    for event in pygame.event.get():
        if event.type == QUIT:
            print('Quit the game...')
            pygame.quit()
            sys.exit()
        if event.type == KEYDOWN:
            if event.key == K_ESCAPE:
                print('Quit the game...')
                pygame.quit()
                sys.exit()

def wait(seconds):
    '''Pauses the program for the specified amout of time'''
    for i in range(int(seconds * FPS)):
        Quit()
        pygame.display.update()
        CLOCK.tick(FPS)

def reset_grid():
    '''Resets the playing grid'''
    global grid
    grid = [0, 0, 0,
            0, 0, 0,
            0, 0, 0]

def test_for_win():
    '''Tests to see if one of the players has won'''
    global grid
    for i in winning_lines:
        if grid[i[0]] == grid[i[1]] == grid[i[2]]:
            if grid[i[0]] == 0: continue
            elif grid[i[0]] == 'X':
                print_to_screen('Winner!', ( 15, 450), False, 80, p1_colour, FONT)
                wins[0] += 1
            elif grid[i[0]] == 'O':
                print_to_screen('Winner!', (570, 450), False, 80, p2_colour, FONT)
                wins[1] += 1
            wait(2)
            reset_grid()

def test_for_draw():
    '''Tests to see if the game results in a tie'''
    global grid
    finish = 0
    for i in grid:
        if i == 0: finish += 1

    if finish == 0:
        print_to_screen('It\'s a draw!', (400, 550), True, 100, OLIVE, FONT)
        wait(2)
        reset_grid()

def main():
    global turn, grid, wins
    clicked = False #Used to stop holding the mouse down from affecting the program

    #Create the game board using the custom sprite class
    boards = [classes.sprite.Sprite(x = x, y = y) for x, y in [[250, 200], [350, 200], [450, 200],
                                                               [250, 300], [350, 300], [450, 300],
                                                               [250, 400], [350, 400], [450, 400]]]

    #Set the image to be shown for each tile (image size: 100px, 100px)
    for board in boards: board.set_image('board.png')

    #Custom sprite group class, allow all the sprites to be rendered in one method
    board_group = classes.sprite.SpriteGroup(boards)

    #Creates instance of the sprite class to show the logo
    logo = classes.sprite.Sprite(100, 20)
    logo.set_image('logo.png')

    while True: #Main loop
        SCREEN.fill(MAROON)
        #Closes the game if the 'X' is clicked or is ESC is pressed
        Quit()

        #Changes the colour of the lettering depending on the turn
        if turn == 0:
            p1_colour = (237,  28,  36)
            p2_colour = ( 36,  44, 134)
        else:
            p1_colour = (157,  13,  20)
            p2_colour = ( 62,  72, 204)

        #Updates the image of a tile if the mouse hovers over it
        i = 0
        for board in boards:
            if board.mouse_hover() and grid[i] == 0:
                if   turn == 0: board.set_image('cross.png')
                elif turn == 1: board.set_image('nought.png')
            else:
                if grid[i] == 0:  board.set_image('board.png')
            i += 1

        #Updates grid with player's turn when tile is clicked
        if pygame.mouse.get_pressed()[0]:
            if clicked == False:
                clicked = True

                i = 0
                for board in boards:
                    if board.mouse_click() and grid[i] == 0:
                        if  turn == 0:
                            board.set_image('cross.png')
                            grid[i] = 'X'
                            turn = 1
                        elif turn == 1:
                            board.set_image('nought.png')
                            grid[i] = 'O'
                            turn = 0
                    i += 1

        else: clicked = False

        #Prints whose turn it is and scores to the screen
        print_to_screen('P1', ( 40, 200), False, 250, p1_colour, FONT)
        print_to_screen('P2', (570, 200), False, 250, p2_colour, FONT)
        print_to_screen(wins[0], (110, 550), True, 100, p1_colour, FONT)
        print_to_screen(wins[1], (670, 550), True, 100, p2_colour, FONT)

        #Renders the board tiles and the logo
        board_group.draw()
        logo.render()

        test_for_win()
        test_for_draw()

        #Update the display
        pygame.display.update()
        CLOCK.tick(FPS)

if __name__ == '__main__':
    main()

Here's an image of what the game looks like when played:

Answer 1

I'd remove those comments, because they don't really add anything (having something like 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 like

path = 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.

Answer 2

In response to your comment that formatting for pep8 takes a long time, Here is a pep8 line length version of your code, that I did in about 3 minutes. (including the cut and paste time)

I use a tool called pycharm. It will auto format to pep8 quite quickly. I only needed to touch a couple of things by hand, primarily comments. There are plenty of equivalent tools, I am not suggesting it is the best, or only way, I am only suggesting that finding and using the best tools you can afford is a really good idea.

import os, sys, pygame

# Imports the necessary modules as well a custom sprite class
import classes.sprite
from pygame.locals import *

pygame.init()  # Initialises the pygame module

path, fname = os.path.split(__file__)
# Defines the path that stores all of the data (Images)
path = os.path.join(path, 'data')  

# 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():
    '''Shuts down the game if it's closed'''
    for event in pygame.event.get():
        if event.type == QUIT:
            print('Quit the game...')
            pygame.quit()
            sys.exit()
        if event.type == KEYDOWN:
            if event.key == K_ESCAPE:
                print('Quit the game...')
                pygame.quit()
                sys.exit()


def wait(seconds):
    '''Pauses the program for the specified amout of time'''
    for i in range(int(seconds * FPS)):
        Quit()
        pygame.display.update()
        CLOCK.tick(FPS)


def reset_grid():
    '''Resets the playing grid'''
    global grid
    grid = [0, 0, 0,
            0, 0, 0,
            0, 0, 0]


def test_for_win():
    '''Tests to see if one of the players has won'''
    global grid
    for i in winning_lines:
        if grid[i[0]] == grid[i[1]] == grid[i[2]]:
            if grid[i[0]] == 0:
                continue
            elif grid[i[0]] == 'X':
                print_to_screen('Winner!', (15, 450), False, 80, p1_colour,
                                FONT)
                wins[0] += 1
            elif grid[i[0]] == 'O':
                print_to_screen('Winner!', (570, 450), False, 80, p2_colour,
                                FONT)
                wins[1] += 1
            wait(2)
            reset_grid()


def test_for_draw():
    '''Tests to see if the game results in a tie'''
    global grid
    finish = 0
    for i in grid:
        if i == 0: finish += 1

    if finish == 0:
        print_to_screen('It\'s a draw!', (400, 550), True, 100, OLIVE, FONT)
        wait(2)
        reset_grid()


def main():
    global turn, grid, wins
    # Used to stop holding the mouse down from affecting the program
    clicked = False  

    # Create the game board using the custom sprite class
    boards = [classes.sprite.Sprite(x=x, y=y) for x, y in
              [[250, 200], [350, 200], [450, 200],
               [250, 300], [350, 300], [450, 300],
               [250, 400], [350, 400], [450, 400]]]

    # Set the image to be shown for each tile (image size: 100px, 100px)
    for board in boards: board.set_image('board.png')

    # Custom sprite group class, allow all the sprites to be rendered
    # in one method
    board_group = classes.sprite.SpriteGroup(boards)

    # Creates instance of the sprite class to show the logo
    logo = classes.sprite.Sprite(100, 20)
    logo.set_image('logo.png')

    while True:  # Main loop
        SCREEN.fill(MAROON)
        # Closes the game if the 'X' is clicked or is ESC is pressed
        Quit()

        # Changes the colour of the lettering depending on the turn
        if turn == 0:
            p1_colour = (237, 28, 36)
            p2_colour = (36, 44, 134)
        else:
            p1_colour = (157, 13, 20)
            p2_colour = (62, 72, 204)

        # Updates the image of a tile if the mouse hovers over it
        i = 0
        for board in boards:
            if board.mouse_hover() and grid[i] == 0:
                if turn == 0:
                    board.set_image('cross.png')
                elif turn == 1:
                    board.set_image('nought.png')
            else:
                if grid[i] == 0:  board.set_image('board.png')
            i += 1

        # Updates grid with player's turn when tile is clicked
        if pygame.mouse.get_pressed()[0]:
            if clicked == False:
                clicked = True

                i = 0
                for board in boards:
                    if board.mouse_click() and grid[i] == 0:
                        if turn == 0:
                            board.set_image('cross.png')
                            grid[i] = 'X'
                            turn = 1
                        elif turn == 1:
                            board.set_image('nought.png')
                            grid[i] = 'O'
                            turn = 0
                    i += 1

        else:
            clicked = False

        # Prints whose turn it is and scores to the screen
        print_to_screen('P1', (40, 200), False, 250, p1_colour, FONT)
        print_to_screen('P2', (570, 200), False, 250, p2_colour, FONT)
        print_to_screen(wins[0], (110, 550), True, 100, p1_colour, FONT)
        print_to_screen(wins[1], (670, 550), True, 100, p2_colour, FONT)

        # Renders the board tiles and the logo
        board_group.draw()
        logo.render()

        test_for_win()
        test_for_draw()

        # Update the display
        pygame.display.update()
        CLOCK.tick(FPS)


if __name__ == '__main__':
    main()

Answer 3

Since chatterone beat me to the punch with lots of good points, and to keep this review a digestible length, I will comment just on test_for_draw() to illustrate a few things.

Starting with:

def test_for_draw():
    """Tests to see if the game results in a tie"""
    global grid
    finish = 0
    for i in grid:
        if i == 0: finish += 1

    if finish == 0:
        # process finish
        ...

Test Directly if Possible:

The finish variable above is used to test if i == 0, be we can do that directly.

def test_for_draw():
    """Tests to see if the game results in a tie"""
    global grid
    for i in grid:
        if i == 0: 
            # process finish
            ...

            # leave the loop
            break

Globals are Bad:

Globals are usually best avoided. The scope being broader can makes things harder to maintain and debug. In this case it wasn't really needed, just make the test for draw function accept a grid to evaluate.

def test_for_draw(grid):
    """Tests to see if the game results in a tie"""
    for i in grid:
        if i == 0:
            # process finish
            ...

            # we're done, leave the loop
            break

Python is Cool!

So python has a ton of slick shorthands that make common tasks syntactically easy. In this example let's use the in operator to test for presence in a list. And test_for_draw ends up as:

def test_for_draw(grid):
    """Tests to see if the game results in a tie"""
    if 0 in grid:
        # process finish
        ...

So we managed to compress 5 lines of code into a single test.

Answer 4

Major issues

Regarding the images, it seems you're loading them again and again when you switch them, but that's slow and consumes extra memory (that's not problematic for a game like tic-tac-toe). Load all images when you start the program and also convert them with the .convert() or .convert_alpha() methods to improve the blit performance. Then just reuse these image variables in the rest of your program instead of reloading them, and the board tiles can share the same images.

CROSS_IMG = pygame.image.load(os.path.join('data', 'cross.png')).convert_alpha()

It's also inefficient to create new font objects the whole time in the print_to_screen function. Define them once as global constants and then reuse them.

FONT = pygame.font.SysFont('Courier New', 60)
FONT2 = pygame.font.SysFont('Courier New', 100)

---

The functions do too much, they should ideally do only one thing. I'll show you how you can turn test_for_win into a pure function which doesn't modify the game state. It can just take the grid as an argument, check who has won and return this player (e.g. 1, 2 or 0 if no one has won yet). In the main loop you can check what the function has returned and change the game state. Pure functions are usually easier to test.

You can get rid of the test_draw function and just test 0 not in grid in the main loop.

I'd also remove the Quit function and put the event loop back into the main function, because you can also check if event.type == pygame.MOUSEBUTTONDOWN: there and remove the mouse.get_pressed check and the clicked variable.

It looks to me like you can simplify your sprite class as well. In my example below I just use a mouse_hover method in which I check if its rect collides with the mouse, so you can probably remove the mouse_click method. Then use this method in both the hover and the click loop. Also, you should give it the initial image in the __init__ method instead of assigning it later.

Minor issues

enumerate helps if you need the item and the index.

for i, board in enumerate(boards):
    if board.mouse_hover() and grid[i] == 0:
        if turn == 0:
            board.set_image(cross_img)
        elif turn == 1:
            board.set_image(nought_img)
    else:
        if grid[i] == 0:
            board.set_image(board_img)

Replace the wait function with pygame.time.wait(2000) or a use timer variable. CLOCK.tick(FPS) returns the time since the last tick which you can use to increase or decrease a timer.

if centered == True: can be simplified to if centered:.

The logo doesn't have to be a sprite. Just blit the image.

---

Example

Here's a complete example with the mentioned changes.

import sys
import pygame


pygame.init()

# Load images once and then reuse them.
# CROSS_IMG = pygame.image.load(os.path.join('data', 'cross.png')).convert_alpha()

# Replaced images with pygame.Surfaces, so that I could run the game.
board_img = pygame.Surface((50, 50))
board_img.fill((90, 90, 90))
logo_img = pygame.Surface((30, 30))
logo_img.fill((80, 90, 170))
pygame.draw.circle(logo_img, (170, 120, 9), [15, 15], 11, 3)
cross_img = pygame.Surface((50, 50), pygame.SRCALPHA)
pygame.draw.line(cross_img, (237, 28, 36), (0, 0), (50, 50), 4)
pygame.draw.line(cross_img, (237, 28, 36), (50, 0), (0, 50), 4)
nought_img = pygame.Surface((50, 50), pygame.SRCALPHA)
pygame.draw.circle(nought_img, (62, 72, 204), (25, 25), 20, 4)

SCREEN = pygame.display.set_mode((800, 600))
CLOCK = pygame.time.Clock()
FPS = 30
FONT = pygame.font.SysFont('Courier New', 60)
FONT2 = pygame.font.SysFont('Courier New', 100)
MAROON = pygame.Color(89, 27, 34)
OLIVE = pygame.Color(30, 140, 10)
ICON = pygame.Surface((32, 32))
ICON.fill((90, 33, 140))

pygame.display.set_icon(ICON)
pygame.display.set_caption('Noughts and Crosses')

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]
    ]


class MySprite(pygame.sprite.Sprite):

    def __init__(self, x, y):
        super().__init__()
        self.image = board_img
        self.rect = self.image.get_rect(topleft=(x, y))

    def mouse_hover(self):
        return self.rect.collidepoint(pygame.mouse.get_pos())

    def set_image(self, img):
        self.image = img


def print_to_screen(text, position=(0, 0), centered=False, colour=(255, 255, 255),
                    font=FONT):
    """Prints text to the screen"""
    label = font.render(str(text), 1, colour)
    if centered:
        position = label.get_rect(center=position)
    SCREEN.blit(label, position)


def test_for_win(grid):
    """Tests to see if one of the players has won.

    Returns:
        int: 1 if player 1 is the winner, 2 for player 2 otherwise 0.
    """
    for i in WINNING_LINES:
        if grid[i[0]] == grid[i[1]] == grid[i[2]]:
            if grid[i[0]] == 0:
                continue
            elif grid[i[0]] == 'X':
                return 1
            elif grid[i[0]] == 'O':
                return 2
    return 0


def main():
    turn = 0
    wins = [0, 0]
    grid = [0] * 9
    boards = [MySprite(x, y)
              for x, y in [[250, 200], [350, 200], [450, 200],
                           [250, 300], [350, 300], [450, 300],
                           [250, 400], [350, 400], [450, 400]]]
    board_group = pygame.sprite.Group(boards)
    colours = [[(237, 28, 36), (36, 44, 134)], [(157, 13, 20), (62, 72, 204)]]

    winner = 0
    game_over = False
    done = False

    while not done:
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                done = True
            if event.type == pygame.KEYDOWN:
                if event.key == pygame.K_ESCAPE:
                    done = True
            if event.type == pygame.MOUSEBUTTONDOWN:
                # Updates grid with player's turn when tile is clicked.
                for i, board in enumerate(boards):
                    if board.mouse_hover() and grid[i] == 0:
                        if turn == 0:
                            board.set_image(cross_img)
                            grid[i] = 'X'
                            turn = 1
                        elif turn == 1:
                            board.set_image(nought_img)
                            grid[i] = 'O'
                            turn = 0

        # === Game logic ===
        if game_over:
            if winner == 1:
                wins[0] += 1
            elif winner == 2:
                wins[1] += 1
            pygame.time.wait(2000)  # Pause the game before reset.
            grid = [0] * 9  # Reset the game.
            game_over = False

        # Updates the image of a tile if the mouse hovers over it.
        for i, board in enumerate(boards):
            if board.mouse_hover() and grid[i] == 0:
                if turn == 0:
                    board.set_image(cross_img)
                elif turn == 1:
                    board.set_image(nought_img)
            else:
                if grid[i] == 0:
                    board.set_image(board_img)

        winner = test_for_win(grid)
        draw = 0 not in grid
        if winner in (1, 2) or draw:
            game_over = True

        # Changes the colour of the lettering depending on the turn.
        p1_colour, p2_colour = colours[turn]

        # === Draw everything ===
        SCREEN.fill(MAROON)

        # Prints whose turn it is and scores to the screen.
        print_to_screen('P1', (40, 200), False, p1_colour, FONT)
        print_to_screen('P2', (570, 200), False, p2_colour, FONT)
        print_to_screen(wins[0], (110, 550), True, p1_colour, FONT2)
        print_to_screen(wins[1], (670, 550), True, p2_colour, FONT2)

        board_group.draw(SCREEN)

        if winner == 1:
            print_to_screen('Winner!', (15, 450), False, p1_colour, FONT)
        elif winner == 2:
            print_to_screen('Winner!', (570, 450), False, p2_colour, FONT)
        elif draw:
            print_to_screen("It's a draw!", (400, 550), True, OLIVE, FONT)

        SCREEN.blit(logo_img, (100, 20))

        pygame.display.update()
        CLOCK.tick(FPS)


if __name__ == '__main__':
    main()
    print('Quit the game...')
    pygame.quit()
    sys.exit()