5
\$\begingroup\$

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()
toolic
14.4k5 gold badges29 silver badges201 bronze badges
asked Jun 22, 2021 at 8:35
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

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.

answered Dec 20, 2024 at 10:10
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.