HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

First chess game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
chessgamefirst

Problem

I am just new to programming I just have a 2 month experience in programming and a 2 week experience with python.

This is my chess game written in python with pygame.

Can some one help me how can I improve this program ?

p.s. this is not a AI chess game it is a multiplayer game; also the checkmate condition is not yet implemented, I'd like to clean up what I have (which works as intented) before I go and implement it.

```
import pygame
import time

pygame.init()

# defines the width and height of the display
display_width = 600
display_height = 680

# defines block width and height
block_height = 50 * 1.5
block_width = 50 * 1.5

factor = 25 * 1.5

# defines colours
white = (255, 255, 255)
d_white = (250, 250, 250)
black = (0, 0, 0)
teal = (0, 128, 128)
blue_black = (50, 50, 50)

game_display = pygame.display.set_mode((display_width, display_height))
pygame.display.update()
clock = pygame.time.Clock()

selected_family = "black"

class piece:
x = 0 # x coordinate
y = 0 # y coordinate
rank = "" # rank of the piece
life = True # is the piece dead or alive
family = "" # colour of the piece (black or white)
pic = "" # photo of the piece

def __init__(self, x_position, y_position, p_rank, p_family):
self.x = x_position
self.y = y_position
self.rank = p_rank
self.family = p_family

selected_piece = piece
end_piece = piece
pie = [piece(3, 7, "q", "black"), piece(0, 6, "p", "black"), piece(1, 6, "p", "black"), piece(2, 6, "p", "black"),
piece(2, 0, "b", "white"), piece(5, 0, "b", "white"), piece(0, 0, "r", "white"), piece(7, 0, "r", "white"),
piece(3, 6, "p", "black"), piece(4, 6, "p", "black"), piece(5, 6, "p", "black"), piece(6, 6, "p", "black"),
piece(7, 6, "p", "black"), piece(1, 0, "k", "white"), piece(6, 0, "k", "white"), piece(4, 0, "king", "white"),
piece(0, 1, "p", "white"), piece(1, 1, "p", "white"), piece(2, 1, "p", "white"), piece(3, 1, "p", "white"),
piece(4,

Solution

Your function initialize_piece could be way simpler if you used a dictionary to map from the piece rank to the file name and used str.format to supply the family into the string:

file_names = {"p": "pawn_{}.png", ...}            

def initialize_piece():
    for piece in pie:
        if piece.life:
            img = pygame.image.load(file_names[piece.rank].format(piece.family)
            game_display.blit(img, ((2 * piece.x) * factor, ((2 * piece.y) * factor)))


This could be simplified even further if every piece had for example "pawn" as rank, so that you could add a file_name property (and a position property):

class piece:
    x = 0  # x coordinate
    y = 0  # y coordinate
    rank = ""  # rank of the piece, e.g. "pawn"
    life = True  # is the piece dead or alive
    family = ""  # colour of the piece ("black" or "white")
    pic = ""  # photo of the piece

    def __init__(self, x_position, y_position, p_rank, p_family):
        self.x = x_position
        self.y = y_position
        self.rank = p_rank
        self.family = p_family

    @property
    def file_name(self):
        return "{self.rank}_{self.family}.png".format(self=self)

    @property
    def position(self):
        return ((2 * self.x) * factor, ((2 * self.y) * factor))


With this it would become:

def initialize_piece():
    for piece in pie:
        if piece.life:
            img = pygame.image.load(piece.file_name)
            game_display.blit(img, piece.position)


In addition to this, you should work on your style. Python has an official style-guide, PEP8. It recommends using CAPITAL_LETTERS for constants, lower_case for variables and functions and PascalCase for classes.

In addition, you should get rid of your global variables as much as possible, they make it very hard to follow the program flow. Instead, pass the variables as arguments where necessary:

def initialize_piece(pieces):
    for piece in pieces:
        if piece.life:
            img = pygame.image.load(piece.file_name)
            game_display.blit(img, piece.position)


What would make it even better is if the image was only loaded once per piece:

class Piece:

    def __init__(self, x, y, rank, family):
        self.x, self.y = x, y
        # rank of the piece, e.g. "pawn"
        self.rank = rank
        # colour of the piece ("black" or "white")
        self.family = family
        self.file_name = "{}_{}.png".format(rank, family)
        self.img = pygame.image.load(self.file_name) 

    @property
    def position(self):
        return ((2 * self.x) * factor, ((2 * self.y) * factor))

def initialize_piece(pieces):
    for piece in pieces:
        if piece.life:
            game_display.blit(piece.img, piece.position)


Here I also removed the unnecessary initializations in the class, you don't need to initialize variables in Python. The comments could be moved to a docstring, for documentation purposes.

Finally, something I have already done above, you should learn how to iterate properly. In Python iterating over the indices of a list (and even worse, using a while loop where a for loop would be a lot easier) is frowned upon.

Instead of any of these:

l = [1, 2, 3, 4, 5]

i = 0
while i < len(l):
    do_something(l[i])
    i += 1

for i in range(len(l)):
    do_something(l[i])


you should just use:

for x in l:
    do_something(x)


If you really need the index, use enumerate:

for i, x in enumerate(l):
    do_something(i, x)


I will leave the rest for somebody else...

Code Snippets

file_names = {"p": "pawn_{}.png", ...}            

def initialize_piece():
    for piece in pie:
        if piece.life:
            img = pygame.image.load(file_names[piece.rank].format(piece.family)
            game_display.blit(img, ((2 * piece.x) * factor, ((2 * piece.y) * factor)))
class piece:
    x = 0  # x coordinate
    y = 0  # y coordinate
    rank = ""  # rank of the piece, e.g. "pawn"
    life = True  # is the piece dead or alive
    family = ""  # colour of the piece ("black" or "white")
    pic = ""  # photo of the piece

    def __init__(self, x_position, y_position, p_rank, p_family):
        self.x = x_position
        self.y = y_position
        self.rank = p_rank
        self.family = p_family

    @property
    def file_name(self):
        return "{self.rank}_{self.family}.png".format(self=self)

    @property
    def position(self):
        return ((2 * self.x) * factor, ((2 * self.y) * factor))
def initialize_piece():
    for piece in pie:
        if piece.life:
            img = pygame.image.load(piece.file_name)
            game_display.blit(img, piece.position)
def initialize_piece(pieces):
    for piece in pieces:
        if piece.life:
            img = pygame.image.load(piece.file_name)
            game_display.blit(img, piece.position)
class Piece:

    def __init__(self, x, y, rank, family):
        self.x, self.y = x, y
        # rank of the piece, e.g. "pawn"
        self.rank = rank
        # colour of the piece ("black" or "white")
        self.family = family
        self.file_name = "{}_{}.png".format(rank, family)
        self.img = pygame.image.load(self.file_name) 

    @property
    def position(self):
        return ((2 * self.x) * factor, ((2 * self.y) * factor))


def initialize_piece(pieces):
    for piece in pieces:
        if piece.life:
            game_display.blit(piece.img, piece.position)

Context

StackExchange Code Review Q#160905, answer score: 8

Revisions (0)

No revisions yet.