patternpythonMinor
First chess game
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,
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
This could be simplified even further if every piece had for example
With this it would become:
In addition to this, you should work on your style. Python has an official style-guide, PEP8. It recommends using
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:
What would make it even better is if the image was only loaded once per piece:
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
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
Instead of any of these:
you should just use:
If you really need the index, use
I will leave the rest for somebody else...
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.