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

A small Bejeweled-like game in Pygame

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

Problem

I would really appreciate a review of my first game. It is a Bejeweled-like game written in Python with Pygame:

```
import pygame, random, time, sys
from pygame.locals import *

pygame.init()

NUM_SHAPES = 7 #7
PUZZLE_COLUMNS = 6 #6
PUZZLE_ROWS = 12 #12
SHAPE_WIDTH = 50 #50
SHAPE_HEIGHT = 50 #50

FPS = 15
WINDOW_WIDTH = PUZZLE_COLUMNS * SHAPE_WIDTH
WINDOW_HEIGHT = PUZZLE_ROWS * SHAPE_HEIGHT + 75

BACKGROUND = pygame.image.load("images/bg.png")

CIRCLE = pygame.image.load("images/circle.png")
DIAMOND = pygame.image.load("images/diamond.png")
HEXAGON = pygame.image.load("images/hexagon.png")
SQUARE = pygame.image.load("images/square.png")
STAR = pygame.image.load("images/star.png")
STAR2 = pygame.image.load("images/star2.png")
TRIANGLE = pygame.image.load("images/triangle.png")
SHAPES_LIST = [CIRCLE, DIAMOND, HEXAGON, SQUARE, STAR, STAR2, TRIANGLE]
for x in xrange(len(SHAPES_LIST) - NUM_SHAPES):
del(SHAPES_LIST[0])

EXPLOSION_1 = pygame.image.load("images/explosion1.png")
EXPLOSION_2 = pygame.image.load("images/explosion2.png")
EXPLOSION_3 = pygame.image.load("images/explosion3.png")
EXPLOSION_4 = pygame.image.load("images/explosion4.png")
EXPLOSION_5 = pygame.image.load("images/explosion5.png")
EXPLOSION_6 = pygame.image.load("images/explosion6.png")
EXPLOSION_LIST = [EXPLOSION_1, EXPLOSION_2, EXPLOSION_3, EXPLOSION_4, EXPLOSION_5, EXPLOSION_6]

BLANK = pygame.image.load("images/blank.png")
WHITE = (255, 255, 255)
BLACK = (0, 0, 0)

FONT_SIZE = 36
TEXT_OFFSET = 5

MINIMUM_MATCH = 3
SINGLE_POINTS = .9
DOUBLE_POINTS = 3
TRIPLE_POINTS = 9
EXTRA_LENGTH_POINTS = .1
RANDOM_POINTS = .3
DELAY_PENALTY_SECONDS = 10
DELAY_PENALTY_POINTS = .5

FPS_CLOCK = pygame.time.Clock()
DISPLAY_SURFACE = pygame.display.set_mode((WINDOW_WIDTH, WINDOW_HEIGHT), DOUBLEBUF)
pygame.display.set_caption("Bilging Puzzle")

def main():
global score
global selector

bilgeBoard = generate_random_board()
selector = (0, 0)
score = 0.0
lastMoveTime

Solution


  1. Introduction



The code seems pretty good to me overall. I had no difficulty reading it or understanding what it did, and it seems to work well enough. So take all the points below (especially those in section 3) with a pinch of salt. I would have written it differently, but that doesn't mean that every change I have made is an improvement.

  1. Major points



-
You implement your explosions by writing a little loop that updates the board to put each animation frame in place, and then waits for a frame to pass:

def explosion_animation(board, matches):
    for frame in EXPLOSION_LIST:
        # ...
        blit_board(board)
        pygame.display.update()
        FPS_CLOCK.tick(FPS)


The problem with this approach is that no other events can happen during this loop: the score can't update, the player can't move the cursor, and nothing else can animate. For this particular game you only have one thing happening at a time, so you can get away with this approach. But what if you wanted to have several different things happening at the same time?

The usual way to handle many objects which all need to move or animate at the same time is to give each object a tick function that is responsible for updating its internal state based on the amount of time that has passed since the last frame. This means, of course, that you will have to re-organize your code so that you record the set of matches, and keep track of time from when the matches occurred so that you can select the correct frame from the explosion animation.

I've put some revised code at the end of this answer showing one way in which you might go about this. Note that a benefit of this approach is that you can decouple the game's framerate from your animation framerate, so that you can change one without having to change the other.

-
The scoring system is both hard to understand (there's no clear relationship between what I do and the score I get) and very harsh. From reading the code, it seems that I lose a point every time I move (and a further half point for every 10 seconds I take) but only gain 0.9 points when I make a match. No wonder I quickly racked up loads of negative points!

The usual way to indicate the relationship between what the player does and the score they get is to draw the score on the board in a position close to the action happened. If there's a bonus, there needs to be some text of the form "BONUS" or "+100" or whatever.

The harshness of the scoring system is really up to you as game designer. But look at it like this: it costs you nothing to hand out points by the millions, and there are people out there who will enjoy the game more if you do.

-
The game never ends! It just carries on forever, with no possibility of winning or losing.

  1. Minor points



-
Code that maintains some kind of persistent state and manipulates it is often most clearly written in the form of classes and methods. This is particularly so for games, which typically implement some kind of persistent world. It would seem sensible here to have a Game class to represent the global game state, a Board class to represent the playing area, and a Cell class to represent a square on the board.

-
The thing you call a selector is more commonly known as a cursor.

-
The term blit is short for block transfer and means roughly "to copy blocks of pixels from one part of memory to another." Unless you're specifically writing about blitting, I'd use a more general term like draw.

-
If you made selector a (mutable) list rather than an immutable tuple, you'd be able to update its x and y component independently:

selector = [0, 0]
# ...
selector[0] += 1  # Move right


-
You can use the function random.choice to simplify

SHAPES_LIST[random.randrange(0, len(SHAPES_LIST))]


to random.choice(SHAPES_LIST).

-
It's conventional to use _ for a loop variable that you don't actually use:

[[random.choice(SHAPES_LIST) for _ in range(PUZZLE_COLUMNS)] for _ in range(PUZZLE_ROWS)]


-
The code could be simplified if you kept the board in a single list of length width × height (rather than a list of lists). You could replace each pair of nested loops with a single loop, and the matches test would also be simplified.

-
When iterating over the elements of a sequence, if you also want the index of each element, use the function enumerate. So instead of:

rowNum = 0
for row in board:
    columnNum = 0
    for shape in row:
        DISPLAY_SURFACE.blit(shape, (SHAPE_WIDTH * columnNum, SHAPE_HEIGHT * rowNum))
        columnNum += 1
    rowNum += 1


write:

for j, row in enumerate(board):
    for i, shape = enumerate(row):
        DISPLAY_SURFACE.blit(shape, (SHAPE_WIDTH * i, SHAPE_HEIGHT * j))


-
The line

selector = (0, 0) #So subsequent matches won't be counted as player matches


seems wrong: if there are subsequent matches at (0,0) then these would be incorrectly counted as player matches. It would be

Code Snippets

def explosion_animation(board, matches):
    for frame in EXPLOSION_LIST:
        # ...
        blit_board(board)
        pygame.display.update()
        FPS_CLOCK.tick(FPS)
selector = [0, 0]
# ...
selector[0] += 1  # Move right
SHAPES_LIST[random.randrange(0, len(SHAPES_LIST))]
[[random.choice(SHAPES_LIST) for _ in range(PUZZLE_COLUMNS)] for _ in range(PUZZLE_ROWS)]
rowNum = 0
for row in board:
    columnNum = 0
    for shape in row:
        DISPLAY_SURFACE.blit(shape, (SHAPE_WIDTH * columnNum, SHAPE_HEIGHT * rowNum))
        columnNum += 1
    rowNum += 1

Context

StackExchange Code Review Q#15873, answer score: 15

Revisions (0)

No revisions yet.