5
\$\begingroup\$

This is my first python/pygame program. Instead of using images I tried drawing with pygame.draw.line() and pygame.draw.arc() with time.sleep() for animation, but this adds delay.

I have commented that code before the while loop in main method. Problem is, it still takes time to render as I have added pygame.time.Clock().tick(16.67) to maintain 60fps.(Maybe this is not needed at all)

I want animation (by using drawGridLinesAnim() and adding delay to drawX() and drawO() methods) and also want to execute event handling in its own thread. I tried threading.Thread(target=fucn, args=(tuple)).start() but that keeps adding latency every move.

How can I go about this? Also, this is the first time I am posting to code review. So any tips/suggestions regarding, code style, code design, etc. will be appreciated.

""" Tic Tac Toe game in python using pygame  """

import sys, pygame, time, threading, numpy as np
pygame.init()

size = width, height = 800, 600
screen = pygame.display.set_mode(size)

#resources
surface_tile_bg = pygame.image.load("tile_bg.png").convert_alpha()
surface_tile_x = pygame.image.load("tile_x1.png").convert_alpha()
surface_tile_o = pygame.image.load("tile_o1.png").convert_alpha()
surface_board = pygame.Surface((300, 300))

color_grid_bg = pygame.color.Color("#14bdac")
color_grid_line = pygame.color.Color("#0da192")
color_symbol_x = pygame.color.Color("#545454")
color_symbol_o = pygame.color.Color("#F2EBD3")
color_white = [255,255,255]

#checks whether all items in a list are exactly same
def all_same(items):
    return all(x == items[0] for x in items)

#draws grid background using surface_tile_bg
def drawBackgroundUsingImage():
    for i in range (0,3):
        for j in range (0,3):
            surface_board.blit(surface_tile_bg, ((i*100, j*100), (100, 100)))
    pygame.display.update()

#draws grid lines with animation    
def drawGridLinesAnim():
    for i in range(0, 301, 10):
        pygame.draw.line(surface_board, color_grid_line, (0, 100), (i,100), 10)
        pygame.draw.line(surface_board, color_grid_line, (0, 200), (i,200), 10)
        pygame.draw.line(surface_board, color_grid_line, (100, 0), (100,i), 10)
        pygame.draw.line(surface_board, color_grid_line, (200, 0), (200,i), 10)
        screen.blit(surface_board, (100, 100))
        pygame.display.update()
        time.sleep(0.005)

#draws grid background using color
def drawBackground():
    surface_board.fill(color_grid_bg)

#draws grid lines using color
def drawGridLines():
    pygame.draw.line(surface_board, color_grid_line, (0, 100), (300,100), 10)
    pygame.draw.line(surface_board, color_grid_line, (0, 200), (300,200), 10)
    pygame.draw.line(surface_board, color_grid_line, (100, 0), (100,300), 10)
    pygame.draw.line(surface_board, color_grid_line, (200, 0), (200,300), 10)

#draws X at given offset on surface        
def drawX(x, y):
    for i in range(0, 70, 10):
        pygame.draw.line(surface_board, color_symbol_x, (x + 20, y + 20), (x + 20 + i, y + 20 + i), 20)
        screen.blit(surface_board, (100, 100))
    for i in range(0, 70, 10):
        pygame.draw.line(surface_board, color_symbol_x, (x + 75, y + 20), (x + 75 - i, y + 20 + i), 20)
        screen.blit(surface_board, (100, 100))

#draws O at given offset on surface
def drawO(x, y):
    for i in range(0, 360, 5):
        rect = ((x+15, y+15), (70,70))
        pygame.draw.arc(surface_board, color_symbol_o, rect, 0, i, 10)
        screen.blit(surface_board, (100, 100))

#draws gameboard depending on contents of TicTacToe object's _board data
def drawBoard(TTTGameObject):
    rows = len(TTTGameObject.board)
    cols = len(TTTGameObject.board[0])
    for i in range (0,rows):
        for j in range (0, cols):
            if TTTGameObject.isPositionX(i,j):
                drawX(j*100, i*100)
            elif TTTGameObject.isPositionO(i,j):
                drawO(j*100, i*100)


#mark position if empty with player's symbol
def handleInput(TTTGameObject, player):
    did_user_quit = False
    symbol_val = player.symbol_val
    events = pygame.event.get()
    for event in events:
        if event.type == pygame.QUIT:
            did_user_quit = True
        elif event.type == pygame.KEYDOWN:
            if event.key == pygame.K_KP1:
                if TTTGameObject.isPositionEmpty(2,0):
                    TTTGameObject.markPosition(2,0,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP2:
                if TTTGameObject.isPositionEmpty(2,1):
                    TTTGameObject.markPosition(2,1,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP3:
                if TTTGameObject.isPositionEmpty(2,2):
                    TTTGameObject.markPosition(2,2,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP4:
                if TTTGameObject.isPositionEmpty(1,0):
                    TTTGameObject.markPosition(1,0,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP5:
                if TTTGameObject.isPositionEmpty(1,1):
                    TTTGameObject.markPosition(1,1,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP6:
                if TTTGameObject.isPositionEmpty(1,2):
                    TTTGameObject.markPosition(1,2,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP7:
                if TTTGameObject.isPositionEmpty(0,0):
                    TTTGameObject.markPosition(0,0,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP8:
                if TTTGameObject.isPositionEmpty(0,1):
                    TTTGameObject.markPosition(0,1,symbol_val)
                    TTTGameObject.turn *= -1
            elif event.key == pygame.K_KP9:
                if TTTGameObject.isPositionEmpty(0,2):
                    TTTGameObject.markPosition(0,2,symbol_val)
                    TTTGameObject.turn *= -1
    return did_user_quit


#player class
class Player:

    def __init__(self):
        self._name = ""
        self._symbol_val = 0

    @property
    def name(self):
        return self._name

    @name.setter
    def name(self, pname):
        self._name = pname

    @property
    def symbol_val(self):
        return self._symbol_val

    @symbol_val.setter
    def symbol_val(self, pSymbVal):
        self._symbol_val = pSymbVal



#tictactoe class
class TicTacToe:

    def __init__(self):
        #set-up board
        self._board = np.array([[0,0,0],
                                 [0,0,0],
                                 [0,0,0]])
        #1 for player 1; -1 for player 2        
        self._turn = 1

    @property
    def board(self):
        return self._board

    @property
    def turn(self):
        return self._turn

    @turn.setter
    def turn(self, val):
        self._turn = val


    #positions will always be marked by 0,1 and 2 for empty, x and o respectively
    def isPositionEmpty(self, x, y):
        return True if self._board[x][y] == 0 else False

    def isPositionX(self, x, y):
        return True if self._board[x][y] == 1 else False

    def isPositionO(self, x, y):
        return True if self._board[x][y] == 2 else False

    #mark 1(x) or 2(0) at specified board position
    def markPosition(self, x, y, val):
        self._board[x][y] = val

    #game is over if board is full or winning combination found
    def isGameOver(self):
        gameOver = False;
        if self.isBoardFull() or not np.array_equal(self.getWinningCombination(), [0,0,0]):
            gameOver = True
        return gameOver

    #check whether all positions filled
    def isBoardFull(self):
        boardFull = True;
        for i in range (0,3):
            for j in range (0,3):
                if self._board[i][j] == 0:
                    boardFull = False
                    break
        return boardFull

    #return [0,0,0] if no winning combination present; else return winning combination
    def getWinningCombination(self):
        winningCombination = [0,0,0]
        if all_same(self._board[0]): winningCombination = self._board[0]
        if all_same(self._board[1]): winningCombination = self._board[1]
        if all_same(self._board[2]): winningCombination = self._board[2]
        if all_same(self._board[:,0]): winningCombination = self._board[:,0]
        if all_same(self._board[:,1]): winningCombination = self._board[:,1]
        if all_same(self._board[:,2]): winningCombination = self._board[:,2]
        if all_same(np.diag(self._board)): winningCombination = np.diag(self._board)
        #temp board since fliplr() alters array
        temp_check_board = np.copy(self._board)
        temp_alter_board = np.copy(self._board)
        if all_same(np.diag(np.fliplr(temp_check_board))): winningCombination = np.diag(np.fliplr(temp_alter_board))
        return winningCombination        



def main():
    tttGameObject = TicTacToe()    
    p1 = Player()
    p2 = Player()

    p1.symbol_val = 1
    p2.symbol_val = 2

    turn = 1

    #drawBackgroundUsingImage()
    #drawGridLinesAnim()
    user_quits = False
    while not tttGameObject.isGameOver() and not user_quits:
        drawBackground()
        drawGridLines()
        curr_player = p1 if tttGameObject.turn == 1 else p2
        user_quits = handleInput(tttGameObject, curr_player)
        drawBoard(tttGameObject)
        pygame.display.update()
        pygame.time.Clock().tick(16.67)
        surface_board.fill(color_white)

main()
pygame.quit()
\$\endgroup\$
  • 2
    \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Peilonrayz Feb 3 '17 at 10:25
  • \$\begingroup\$ I will keep that in mind. Thanks for reverting back to original question. \$\endgroup\$ – Chandan Pednekar Feb 5 '17 at 15:33
2
\$\begingroup\$

There's quite a lot going on here, beside the style issues (use snake_case instead of camelCase, for example).

  • The first big issue is that you're checking who won on every frame. You only need to check that when there's an input from either player, because the board will not change in any other case.

  • You also don't need numpy for such a simple task, if you have a 3x3 grid, you can easily get the two diagonals.

For example:

grid = [[1, 2, 3],[4, 5, 6],[7, 8, 9]]

diag = [grid[i][i] for i in range(0, 3)]
print diag

diag = [grid[2-i][i] for i in range(2, -1, -1)]
print diag

You also don't need the all_same check, what you can do is use 1 for a player and -1 for the other player and check if the sum in any of the rows, columns or diagonals is either 3 or -3.

This also means that you can build an array containing the sums and check if one of the -3, 3 is in there.

For example:

grid = [[-1, 0, 1],[-1, 1, 1],[-1, 0, 0]]

diag19 = [grid[i][i] for i in range(0, 3)]
diag37 = [grid[2-i][i] for i in range(2, -1, -1)]

sums_rows = [sum(row) for row in grid]
sums_cols = [sum(col) for col in zip(*grid)]
sums_diag = [sum(diag) for diag in [diag19, diag37]]

winning_list = [status for status in [sums_rows, sums_cols, sums_diag] if 3 in status or -3 in status]

print winning_list

Of course:

  • Only the player with the current turn can win
  • The board can be full only after nine moves, so no need to check the content, just check if it's the ninth move.

Also, you're making the same check a lot of times when the user presses a key. It's much easier if you move that check into the markPosition function and maybe rename that to mark_position_if_free.

The same goes for changing the turn, you do that in every if, so just put it again in the mark_position_if_free function.

That leaves you with:

# TODO: rename everything to be snake_case
def markPosition(self, x, y, val):
    if TTTGameObject.isPositionEmpty(x, y):
        self._board[x][y] = val
        self.turn *= -1

def handleInput(TTTGameObject, player):
    did_user_quit = False
    symbol_val = player.symbol_val
    events = pygame.event.get()
    for event in events:
        if event.type == pygame.QUIT:
            did_user_quit = True
        elif event.type == pygame.KEYDOWN:
            if event.key == pygame.K_KP1:
                TTTGameObject.markPosition(2,0,symbol_val)
            elif event.key == pygame.K_KP2:
                TTTGameObject.markPosition(2,1,symbol_val)
            elif event.key == pygame.K_KP3:
                TTTGameObject.markPosition(2,2,symbol_val)
            elif event.key == pygame.K_KP4:
                TTTGameObject.markPosition(1,0,symbol_val)
            elif event.key == pygame.K_KP5:
                TTTGameObject.markPosition(1,1,symbol_val)
            elif event.key == pygame.K_KP6:
                TTTGameObject.markPosition(1,2,symbol_val)
            elif event.key == pygame.K_KP7:
                TTTGameObject.markPosition(0,0,symbol_val)
            elif event.key == pygame.K_KP8:
                TTTGameObject.markPosition(0,1,symbol_val)
            elif event.key == pygame.K_KP9:
                TTTGameObject.markPosition(0,2,symbol_val)
    return did_user_quit

But that really looks a lot like a lookup table, basically a dictionary. So maybe you could use something like:

# TODO: rename everything to be snake_case
keys_to_coord = {
    pygame.K_KP1: [2, 0],
    pygame.K_KP2: [2, 1],
    pygame.K_KP3: [2, 2],
    pygame.K_KP4: [1, 0],
    pygame.K_KP5: [1, 1],
    pygame.K_KP6: [1, 2],
    pygame.K_KP7: [0, 0],
    pygame.K_KP8: [0, 1],
    pygame.K_KP9: [0, 2]
}

def handleInput(TTTGameObject, player):
    did_user_quit = False
    symbol_val = player.symbol_val
    events = pygame.event.get()
    for event in events:
        if event.type == pygame.QUIT:
            did_user_quit = True
        elif event.type == pygame.KEYDOWN:
            TTTGameObject.markPosition(keys_to_coord[event.key][0],keys_to_coord[event.key][1],symbol_val)
    return did_user_quit

I don't think this is all, but start by simplifying your code and other things will pop-up (how often are you actually drawing X and O, for example).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.