7
\$\begingroup\$

Recently discovered PyQt5 and decided to make this as a 'hello world' for GUIs

Would like input on the design, best practices, as well as idiomatic nature of the application and its structure.

Game.py

import collections
from PyQt5.QtWidgets import QMainWindow, QGridLayout, QPushButton, QDialog, QLabel, QLineEdit, QWidget, QAction, QDesktopWidget

# TODO-P: Maybe add color for symbol. Alt: Use stylesheets.
Player = collections.namedtuple('Player', ['name', 'symbol'])


class TicTacToe(QMainWindow):

    def __init__(self):
        super().__init__()
        self.xPlayer = Player('X Player', 'X')
        self.oPlayer = Player('O Player', 'O')
        self.currentPlayer = self.xPlayer
        self.turnIndicator = f"{self.currentPlayer.name}'s Turn"
        self.initUI()

    def initUI(self):
        self.board = TicTacToeBoard(self)
        self.setCentralWidget(self.board)
        self.initElements()
        self.setWindowTitle('Tic Tac Toe')
        self.resize(1200, 1200)
        self.center()
        self.show()

    def initElements(self):
        self.initStatusBar()
        self.initMenu()

    def initStatusBar(self):
        self.statusBar = self.statusBar()
        self.statusBar.showMessage(self.turnIndicator)

    def initMenu(self):
        gameMenu = self.menuBar().addMenu('Game')
        gameMenu.addAction(self.resetAction())
        settingsMenu = self.menuBar().addMenu('Settings')
        settingsMenu.addAction(self.setNameAction())
        settingsMenu.addAction(self.turnIndicatorToggle())

    def resetAction(self):
        action = QAction('Reset', self)
        action.setShortcut('Ctrl+R')
        action.setToolTip('Reset Game')
        action.triggered.connect(self.reset)
        return action

    def reset(self):
        self.board.reset()
        self.currentPlayer = self.xPlayer
        self.updateIndicator()

    def updateIndicator(self):
        # TODO-Q: Does PyQt have parameter binding? Can I also use signals / slots to 'auto-update' variables / displayed text?
        self.turnIndicator = f"{self.currentPlayer.name}'s Turn"
        self.statusBar.showMessage(self.turnIndicator)

    def setNameAction(self):
        action = QAction('Set Name(s)', self)
        action.setShortcut('Ctrl+A')
        action.setStatusTip('Set Player Name(s)')
        action.triggered.connect(self.setNameDialog)
        return action

    def setNameDialog(self):
        dialog = QDialog(self)
        dialog.setWindowTitle('Set Player Name(s)')
        dialog.resize(600, 200)
        layout = QGridLayout()
        xText = QLineEdit()
        oText = QLineEdit()
        layout.addWidget(QLabel(f'<b>{self.xPlayer.name}:</b>'), 0, 0)
        layout.addWidget(xText, 0, 1)
        layout.addWidget(QLabel(f'<b>{self.oPlayer.name}:</b>'), 1, 0)
        layout.addWidget(oText, 1, 1)
        dialog.setLayout(layout)
        enterButton = QPushButton('Enter')
        enterButton.clicked.connect(lambda: self.setName(xText.text(), oText.text()))
        enterButton.clicked.connect(dialog.accept)
        layout.addWidget(enterButton, 2, 0)
        dialog.exec_()

    def setName(self, xName, oName):
        if xName:
            self.xPlayer = self.xPlayer._replace(name=xName)
        if oName:
            self.oPlayer = self.oPlayer._replace(name=oName)
        self.currentPlayer = self.xPlayer if self.currentPlayer is self.xPlayer else self.oPlayer
        self.updateIndicator()

    def buttonClicked(self):
        button = self.sender()
        button.capture(self.currentPlayer.symbol)
        self.togglePlayer()

    def togglePlayer(self):
        self.currentPlayer = self.oPlayer if self.currentPlayer is self.xPlayer else self.xPlayer
        self.updateIndicator()

    def turnIndicatorToggle(self):
        action = QAction('Turn Indicator', self, checkable=True)
        action.setShortcut('Ctrl+T')
        action.setChecked(True)
        action.setStatusTip('Toggle Status Indicator')
        action.triggered.connect(self.toggleTurnIndicator)
        return action

    def toggleTurnIndicator(self, state):
        if state:
            self.statusBar.show()
            self.statusBar.showMessage(self.turnIndicator)
        else:
            self.statusBar.hide()

    def showVictory(self):
        winner = self.oPlayer if self.currentPlayer is self.xPlayer else self.xPlayer
        self.statusBar.showMessage(f'{winner.name} wins!')

    def center(self):
        qr = self.frameGeometry()
        qr.moveCenter(QDesktopWidget().availableGeometry().center())
        self.move(qr.topLeft())


class TicTacToeBoard(QWidget):  

    def __init__(self, parent):
        super().__init__(parent)
        self.game = parent
        layout = QGridLayout()
        self.setLayout(layout)
        # TODO-Q: How to implement __mul__ so this works -> [TicTacToeCell(self) * 3] * 3
        self.grid = [[TicTacToeCell(self), TicTacToeCell(self), TicTacToeCell(self)], [TicTacToeCell(self), TicTacToeCell(self), TicTacToeCell(self)], [TicTacToeCell(self), TicTacToeCell(self), TicTacToeCell(self)]]
        positions = [(i, j) for i in range(3) for j in range(3)]
        for position in positions:
            x, y = position
            button = self.grid[x][y]
            button.clicked.connect(parent.buttonClicked)
            button.clicked.connect(self.evaluateBoard)
            layout.addWidget(button, *position)

    def reset(self):
        for row in self.grid:
            for cell in row:
                cell.reset()

    def disable(self):
        for row in self.grid:
            for cell in row:
                cell.setEnabled(False)

    def evaluateBoard(self):
        self.evaluateCells(self.grid[0][0], self.grid[1][1], self.grid[2][2])
        self.evaluateCells(self.grid[0][2], self.grid[1][1], self.grid[2][0])
        for i in range(3):
            self.evaluateCells(self.grid[i][0], self.grid[i][1], self.grid[i][2])
            self.evaluateCells(self.grid[0][i], self.grid[1][i], self.grid[2][i])

    def evaluateCells(self, a, b, c):
        if a.captured() and a.text is b.text is c.text:
            self.disable()
            self.game.showVictory()


class TicTacToeCell(QPushButton):

    def __init__(self, parent):
        super().__init__(parent)
        self.setMinimumSize(400, 400)
        self.text = ''

    def captured(self):
        return self.text

    def capture(self, symbol):
        self.text = symbol
        self.setText(symbol)
        self.setEnabled(False)

    def reset(self):
        self.text = ''
        self.setText(self.text)
        self.setEnabled(True)

    def __repr__(self):
        return f'[{self.text}]'

App.py

import sys
from PyQt5.QtWidgets import QApplication
from Game import TicTacToe

if __name__ == '__main__':
    app = QApplication(sys.argv)
    tictactoe = TicTacToe()
    sys.exit(app.exec_())
\$\endgroup\$
  • 2
    \$\begingroup\$ Looks good. You don't need to construct TicTacToeCell widgets with a parent because adding them to the layout sets the parent automatically. Doing so actually places them on the parent widget at coordinates (0, 0), until the layout repositions them. The TicTacToeCell constructor can be changed to def __init__(self, parent=None) and you can do this self.grid = [[TicTacToeCell() for i in range(3)] for j in range(3)] \$\endgroup\$ – alec Mar 10 at 9:20
  • 1
    \$\begingroup\$ @alec The "add an answer" button is just a little lower than the "add comment" button. Comments should be used to ask clarifying questions, not to give review feedback. \$\endgroup\$ – AJNeufeld Mar 10 at 13:39
  • \$\begingroup\$ Oh ok, but it wasn't a complete answer addressing all the questions so I thought I should comment it. \$\endgroup\$ – alec Mar 10 at 21:22
2
\$\begingroup\$

The code generally looks good, but is a little rough to read (in part due to length but also with method names like turnIndicatorToggle(self) and toggleTurnIndicator(self)). But when I try to run your program I run into a few issues; your main window is fairly huge (bigger than my screen!) and can't be resized, and your indicators for tics and tacs (x and o) don't appear after the first. I think there's also supposed to be text somewhere but I can't see it (which may be because the window is too big).

Since you're asking for Qt advice, I think that you should look at decorators for your slots; it increases both readability and performance. And since you ask for best practices, you should get used to snake_case instead of camelCase for your variables and functions, and lower case names for your files, and it would also be nice with type hinting and some docstrings as well as some additional whitespace in your functions. You may also want to consider separating the "back-end" of the game and putting that functionality in its own class.

Finally, I'll add some minor practical tips:

  • you could simplify lines 136–137 to for x, y in positions (and you could have a tuple of tuples instead of a list of tuples) (don't use memory you don't need to use.)
  • You can simplify it even more with for x, y in itertools.product(range(3), range(3)): (if there's a function that can do what you want to do, use it.)
  • your capture and reset functions in TicTacToeCell can merged with a default argument: def capture(self, symbol=''): (look for repetition)
| improve this answer | |
\$\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.