I just completed my second python project which is a chess game. At the moment, I have included a mechanism for pieces to lock onto a square based on the nearest square when moved. And, I have introduced a mechanism to take pieces. I haven't yet checked for legal moves, but I plan on doing that soon.
If anyone could suggest ways I could make the code neater/more efficient, that would be greatly appreciated. I've been programming in C++ for around one and a half years, so a few things I tried in python that I can do in C++ didn't work, which caused me to do some unorthodox methods.
If you want to run the project, here are the images I used as a rar
file: https://gofile.io/d/Ss0vPc
import pygame
import python_math as math
whiteC = (200,200,200)
blackC = (55, 55, 55)
class Coordinates: #convering pixel values to alpha and numerical values that you would see on a chess board
# x values
a = 0 * 100
b = 1 * 100
c = 2 * 100
d = 3 * 100
e = 4 * 100
f = 5 * 100
g = 6 * 100
h = 7 * 100
i = 8 * 100
j = 9 * 100
k = 10 * 100
l = 11 * 100
m = 12 * 100
n = 13 * 100
# y values
one = 7 * 100
two = 6 * 100
three = 5 * 100
four = 4 * 100
five = 3 * 100
six = 2 * 100
seven = 1 * 100
eight = 0 * 100
c = Coordinates
whiteTAC = [(c.i, c.eight), (c.j, c.eight) , (c.k, c.eight), (c.l, c.eight), (c.m, c.eight), (c.n, c.eight), (c.i, c.seven),
(c.j, c.seven), (c.k, c.seven), (c.l, c.seven), (c.m, c.seven), (c.n, c.seven), (c.i, c.six), (c.j, c.six),
(c.k, c.six), (c.l, c.six)] # when we take a piece, these are the coordinates in which the taken piece will go to
blackTAC = [(c.i, c.one), (c.j, c.one) , (c.k, c.one), (c.l, c.one), (c.m, c.one), (c.n, c.one), (c.i, c.two),
(c.j, c.two), (c.k, c.two), (c.l, c.two) , (c.m, c.two), (c.n, c.two), (c.i, c.three), (c.j, c.three),
(c.k, c.three), (c.l, c.three)]
def round100bottom(x):
return int(math.math.floor(x / 100.0)) * 100
def round100top(x):
return int(math.math.ceil(x / 100.0)) * 100
class game_logic:
currentMove = "white"
wI = 0 #indexs used for moving a piece when its taken away, used for arrays whiteTAC and blackTAC
bI = 0
GL = game_logic
class Piece:
side = None
image = None
coords = None
heldDown = False
tempCoords = coords
tempBool = False
finalRoundOne = 0
finalRoundTwo = 0
justMoved = False
def __init__(self, image, coords, side):
self.image = pygame.image.load(image)
self.coords = coords
self.side = side
def listenForMovement(self, event):
print(self.side)
if event.type == pygame.MOUSEBUTTONUP:
if self.justMoved:
print("called")
if GL.currentMove == "white":
GL.currentMove = "black"
else:
GL.currentMove = "white"
self.justMoved = False
self.heldDown = False
self.tempBool = False
if ((self.coords[0] - round100bottom(self.coords[0])) < 50): # tests to see which square is the closest by rounding the top and the bottom and seeing which is smallest
self.finalRoundOne = round100bottom(self.coords[0])
else:
self.finalRoundOne = round100top(self.coords[0])
if ((self.coords[1] - round100bottom(self.coords[1])) < 50):
self.finalRoundTwo = round100bottom(self.coords[1])
else:
self.finalRoundTwo = round100top(self.coords[1])
self.coords = (self.finalRoundOne, self.finalRoundTwo)
TakePiece(self.coords, self.side)
if ((event.type == pygame.MOUSEBUTTONDOWN or self.heldDown == True) and pygame.mouse.get_pos()[0] > self.coords[0] and pygame.mouse.get_pos()[0] < self.coords[0] + 100 and pygame.mouse.get_pos()[1] > self.coords[1] and pygame.mouse.get_pos()[1] < self.coords[1] + 100 and GL.currentMove == self.side):
self.heldDown = True
if self.tempBool != True:
self.tempCoords = (pygame.mouse.get_pos()[0] - self.coords[0], pygame.mouse.get_pos()[1] - self.coords[1])
self.tempBool = True
self.coords = (pygame.mouse.get_pos()[0] - self.tempCoords[0], pygame.mouse.get_pos()[1] - self.tempCoords[1])
self.justMoved = True
class White:
king = Piece(r'C:\sprites\wking.png', (c.e, c.one), "white")
queen = Piece(r'C:\sprites\wqueen.png', (c.d, c.one), "white")
bishop1 = Piece(r'C:\sprites\wbishop.png', (c.c, c.one), "white")
bishop2 = Piece(r'C:\sprites\wbishop.png', (c.f, c.one), "white")
knight1 = Piece(r'C:\sprites\wknight.png', (c.b, c.one), "white")
knight2 = Piece(r'C:\sprites\wknight.png', (c.g, c.one), "white")
castle1 = Piece(r'C:\sprites\wcastle.png', (c.a, c.one), "white")
castle2 = Piece(r'C:\sprites\wcastle.png', (c.h, c.one), "white")
pawn1 = Piece(r'C:\sprites\wpawn.png', (c.a, c.two), "white")
pawn2 = Piece(r'C:\sprites\wpawn.png', (c.b, c.two), "white")
pawn3 = Piece(r'C:\sprites\wpawn.png', (c.c, c.two), "white")
pawn4 = Piece(r'C:\sprites\wpawn.png', (c.d, c.two), "white")
pawn5 = Piece(r'C:\sprites\wpawn.png', (c.e, c.two), "white")
pawn6 = Piece(r'C:\sprites\wpawn.png', (c.f, c.two), "white");
pawn7 = Piece(r'C:\sprites\wpawn.png', (c.g, c.two), "white")
pawn8 = Piece(r'C:\sprites\wpawn.png', (c.h, c.two), "white")
class Black:
king = Piece(r'C:\sprites\bking.png', (c.e, c.eight), "black")
queen = Piece(r'C:\sprites\bqueen.png', (c.d, c.eight), "black")
bishop1 = Piece(r'C:\sprites\bbishop.png', (c.c, c.eight), "black")
bishop2 = Piece(r'C:\sprites\bbishop.png', (c.f, c.eight), "black")
knight1 = Piece(r'C:\sprites\bknight.png', (c.b, c.eight), "black")
knight2 = Piece(r'C:\sprites\bknight.png', (c.g, c.eight), "black")
castle1 = Piece(r'C:\sprites\bcastle.png', (c.a, c.eight), "black")
castle2 = Piece(r'C:\sprites\bcastle.png', (c.h, c.eight), "black")
pawn1 = Piece(r'C:\sprites\bpawn.png', (c.a, c.seven), "black")
pawn2 = Piece(r'C:\sprites\bpawn.png', (c.b, c.seven), "black")
pawn3 = Piece(r'C:\sprites\bpawn.png', (c.c, c.seven), "black")
pawn4 = Piece(r'C:\sprites\bpawn.png', (c.d, c.seven), "black")
pawn5 = Piece(r'C:\sprites\bpawn.png', (c.e, c.seven), "black")
pawn6 = Piece(r'C:\sprites\bpawn.png', (c.f, c.seven), "black")
pawn7 = Piece(r'C:\sprites\bpawn.png', (c.g, c.seven), "black")
pawn8 = Piece(r'C:\sprites\bpawn.png', (c.h, c.seven), "black")
pygame.init()
dis = pygame.display.set_mode((1400, 800))
pygame.display.update()
white = White
black = Black
def draw_pieces():
# dis.blit(white.king.image, white.king.coords)
dis.blit(white.pawn1.image, white.pawn1.coords)
dis.blit(white.pawn2.image, white.pawn2.coords)
dis.blit(white.pawn3.image, white.pawn3.coords)
dis.blit(white.pawn4.image, white.pawn4.coords)
dis.blit(white.pawn5.image, white.pawn5.coords)
dis.blit(white.pawn6.image, white.pawn6.coords)
dis.blit(white.pawn7.image, white.pawn7.coords)
dis.blit(white.pawn8.image, white.pawn8.coords)
dis.blit(white.king.image, white.king.coords)
dis.blit(white.queen.image, white.queen.coords)
dis.blit(white.bishop1.image, white.bishop1.coords)
dis.blit(white.bishop2.image, white.bishop2.coords)
dis.blit(white.knight1.image, white.knight1.coords)
dis.blit(white.knight2.image, white.knight2.coords)
dis.blit(white.castle1.image, white.castle1.coords)
dis.blit(white.castle2.image, white.castle2.coords)
dis.blit(black.pawn1.image, black.pawn1.coords)
dis.blit(black.pawn2.image, black.pawn2.coords)
dis.blit(black.pawn3.image, black.pawn3.coords)
dis.blit(black.pawn4.image, black.pawn4.coords)
dis.blit(black.pawn5.image, black.pawn5.coords)
dis.blit(black.pawn6.image, black.pawn6.coords)
dis.blit(black.pawn7.image, black.pawn7.coords)
dis.blit(black.pawn8.image, black.pawn8.coords)
dis.blit(black.king.image, black.king.coords)
dis.blit(black.queen.image, black.queen.coords)
dis.blit(black.bishop1.image, black.bishop1.coords)
dis.blit(black.bishop2.image, black.bishop2.coords)
dis.blit(black.knight1.image, black.knight1.coords)
dis.blit(black.knight2.image, black.knight2.coords)
dis.blit(black.castle1.image, black.castle1.coords)
dis.blit(black.castle2.image, black.castle2.coords)
for event in pygame.event.get():
white.king.listenForMovement(event)
white.queen.listenForMovement(event)
white.bishop1.listenForMovement(event)
white.bishop2.listenForMovement(event)
white.castle1.listenForMovement(event)
white.castle2.listenForMovement(event)
white.knight1.listenForMovement(event)
white.knight2.listenForMovement(event)
white.pawn1.listenForMovement(event)
white.pawn2.listenForMovement(event)
white.pawn3.listenForMovement(event)
white.pawn4.listenForMovement(event)
white.pawn5.listenForMovement(event)
white.pawn6.listenForMovement(event)
white.pawn7.listenForMovement(event)
white.pawn8.listenForMovement(event)
black.king.listenForMovement(event)
black.queen.listenForMovement(event)
black.bishop1.listenForMovement(event)
black.bishop2.listenForMovement(event)
black.castle1.listenForMovement(event)
black.castle2.listenForMovement(event)
black.knight1.listenForMovement(event)
black.knight2.listenForMovement(event)
black.pawn1.listenForMovement(event)
black.pawn2.listenForMovement(event)
black.pawn3.listenForMovement(event)
black.pawn4.listenForMovement(event)
black.pawn5.listenForMovement(event)
black.pawn6.listenForMovement(event)
black.pawn7.listenForMovement(event)
black.pawn8.listenForMovement(event)
def TakePiece(coords, side):
if side == "black":
if white.king.coords == coords:
white.king.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.queen.coords == coords:
white.queen.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.king.coords == coords:
white.king.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.queen.coords == coords:
white.queen.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.castle1.coords == coords:
white.castle1.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.castle2.coords == coords:
white.castle2.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.knight1.coords == coords:
white.knight1.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.knight2.coords == coords:
white.knight2.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.bishop1.coords == coords:
white.bishop1.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.bishop2.coords == coords:
white.bishop2.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn1.coords == coords:
white.pawn1.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn2.coords == coords:
white.pawn2.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn3.coords == coords:
white.pawn3.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn4.coords == coords:
white.pawn4.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn5.coords == coords:
white.pawn5.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn6.coords == coords:
white.pawn6.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn7.coords == coords:
white.pawn7.coords = whiteTAC[GL.wI]
GL.wI += 1
if white.pawn8.coords == coords:
white.pawn8.coords = whiteTAC[GL.wI]
GL.wI += 1
if side == "white":
if black.king.coords == coords:
black.king.coords = blackTAC[GL.bI]
GL.bI += 1
if black.queen.coords == coords:
black.queen.coords = blackTAC[GL.bI]
GL.bI += 1
if black.king.coords == coords:
black.king.coords = blackTAC[GL.bI]
GL.bI += 1
if black.queen.coords == coords:
black.queen.coords = blackTAC[GL.bI]
GL.bI += 1
if black.castle1.coords == coords:
black.castle1.coords = blackTAC[GL.bI]
GL.bI += 1
if black.castle2.coords == coords:
black.castle2.coords = blackTAC[GL.bI]
GL.bI += 1
if black.knight1.coords == coords:
black.knight1.coords = blackTAC[GL.bI]
GL.bI += 1
if black.knight2.coords == coords:
black.knight2.coords = blackTAC[GL.bI]
GL.bI += 1
if black.bishop1.coords == coords:
black.bishop1.coords = blackTAC[GL.bI]
GL.bI += 1
if black.bishop2.coords == coords:
black.bishop2.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn1.coords == coords:
black.pawn1.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn2.coords == coords:
black.pawn2.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn3.coords == coords:
black.pawn3.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn4.coords == coords:
black.pawn4.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn5.coords == coords:
black.pawn5.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn6.coords == coords:
black.pawn6.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn7.coords == coords:
black.pawn7.coords = blackTAC[GL.bI]
GL.bI += 1
if black.pawn8.coords == coords:
black.pawn8.coords = blackTAC[GL.bI]
GL.bI += 1
def draw_background():
pygame.draw.rect(dis, (180, 180, 180), [800, 0, 600, 800])
for i in range(0, 8):
for j in range(0, 8):
if (i + j) % 2 != 0:
pygame.draw.rect(dis, blackC, [i * 100, j * 100, 100, 100 ])
else:
pygame.draw.rect(dis, whiteC, [i * 100, j * 100, 100, 100 ])
def main():
while True:
draw_background()
draw_pieces()
pygame.display.update()
main()
1 Answer 1
Layout
Classes and functions should be at the top after the import
lines.
Having them intermixed with executable lines 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.
For example, you can convert this comment:
class Coordinates: #convering pixel values to alpha and numerical values that you would see on a chess board
into a docstring:
class Coordinates:
""" Converting pixel values to alpha and numerical values that you would see on a chess board """
I also fixed a typo ("convering").
Naming
The naming style you use for function names is a little inconsistent. The PEP-8 guide recommends using snake_case for functions. For example:
def TakePiece(coords, side):
would be:
def take_piece(coords, side):
In general, many of your variables and constants could use more descriptive names.
For example:
# x values
a = 0 * 100
b = 1 * 100
c = 2 * 100
The single letter names (a
through n
) don't convey much meaning.
The comment gives us some idea, but it could be expanded to describe
why you chose those letters.
While we're looking at those lines, the number 100
is repeated many times.
It would be better to replace the "magic number" with a constant which has a name
describing the significance of the value. For example:
PIXEL_WIDTH = 100
a = 0 * PIXEL_WIDTH
b = 1 * PIXEL_WIDTH
Refer again to the PEP-8 guide for recommendations on variable and constant names.
For a name like whiteTAC
, it would be better to either expand the "TAC" acronym
or at least add a comment describing what it means.
wI
would be better as white_index
.
dis
better as display
.
DRY
This is where the most benefits would come into play. There is a lot of repeated code.
For example:
king = Piece(r'C:\sprites\wking.png', (c.e, c.one), "white")
queen = Piece(r'C:\sprites\wqueen.png', (c.d, c.one), "white")
bishop1 = Piece(r'C:\sprites\wbishop.png', (c.c, c.one), "white")
The string "C:\sprites" is repeated 32 times: once for each chess piece. This can be assigned to a constant, which makes the code more flexible if you change the directory path to those PNG files.
You can also use a constant for the color.
Consider using an array for the 8 pawns:
pawn1 = Piece(r'C:\sprites\wpawn.png', (c.a, c.two), "white")
pawn2 = Piece(r'C:\sprites\wpawn.png', (c.b, c.two), "white")
pawn3 = Piece(r'C:\sprites\wpawn.png', (c.c, c.two), "white")
pawn4 = Piece(r'C:\sprites\wpawn.png', (c.d, c.two), "white")
pawn5 = Piece(r'C:\sprites\wpawn.png', (c.e, c.two), "white")
pawn6 = Piece(r'C:\sprites\wpawn.png', (c.f, c.two), "white");
pawn7 = Piece(r'C:\sprites\wpawn.png', (c.g, c.two), "white")
pawn8 = Piece(r'C:\sprites\wpawn.png', (c.h, c.two), "white")
You could use a loop to make the array assignments.
This could greatly reduce the line count in the draw_pieces
and TakePiece
functions as well.
Tools
Since you were new to Python when this question was posted, perhaps these code development tools could be of use.
The black program can be used to automatically add more consistency to the formatting of the code (reduce long lines, etc.).
The lint tools can help with adehering to common coding style.