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()
\$\endgroup\$

1 Answer 1

Reset to default
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.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.