# Simple top down shooter game

This one of my first Python games in PyGame I decided to make for fun. Basically, you control a from top down perspective and shoot people in a maze.

This is the rewrite of the original code, and I'm aware that it is still probably very inefficient and has does many things frowned upon by many such as many repeated if statements, etc (that I don't know how to fix).

I would like to know your suggestions on improvements I can make, as well as point out my bad habits and tell me how to fix them.

import pygame
import random

pygame.init()

black = [0, 0, 0]
white = [255, 255, 255]
red = [255, 0, 0]
orange = [255, 102, 0]
yellow = [255, 255, 0]
green = [0, 255, 0]
blue = [0, 0, 255]
purple = [128, 0, 128]

FPS = 15
clock = pygame.time.Clock()

block_s = 30
screen_w, screen_h = 900, 600
assert screen_w % block_s == 0, 'Window width must be multiple of tile side.'
assert screen_h % block_s == 0, 'Window height must be multiple of tile side.'
screen = pygame.display.set_mode((screen_w, screen_h), 0, 32)
pygame.display.set_caption('Communism Simulator 2')

font = pygame.font.SysFont(None, 24)

def playMusic(path):
song = pygame.mixer.Sound(random.choice(path))
song.play()

def message(msg, color, x, y):
text = font.render(msg, True, color)
screen.blit(text, [x, y])

wall_coords = []
enemy_coords = []
file = open(path, 'r')
file = file.split('\n')
for row in range(len(file)):
for column in range(len(file[row])):
if file[row][column] == '@':
if file[row][column] == '#':
wall_coords.append([column*block_s, row*block_s])
if file[row][column] == 'e':
enemy_coords.append([column*block_s, row*block_s])

def mainLoop():
enemy_dir = enemyup

bullet_coords = []
bullet_dir = bulletup
bulletMotion = False

gameOver = False
while True:
while gameOver == True:
screen.blit(gameover, (0, 0))
pygame.display.update()
for event in pygame.event.get():
if event.type == pygame.KEYDOWN:
if event.key == pygame.K_n:
pygame.quit()
quit()
elif event.key == pygame.K_y:
mainLoop()

for event in pygame.event.get():
if event.type == pygame.QUIT:
pygame.quit()
quit()
if event.type == pygame.KEYDOWN:
if event.key == pygame.K_LEFT:
elif event.key == pygame.K_RIGHT:
elif event.key == pygame.K_UP:
elif event.key == pygame.K_DOWN:

if event.key == pygame.K_z and bulletMotion == False:
bullet_dir = bulletup

bullet_dir = bulletdown

bullet_dir = bulletright

bullet_dir = bulletleft
bulletMotion = True

if event.type == pygame.KEYUP:
if event.key == pygame.K_LEFT:
elif event.key == pygame.K_RIGHT:
elif event.key == pygame.K_UP:
elif event.key == pygame.K_DOWN:

if bulletMotion == True:
if bullet_dir == bulletup:
bullet_coords[1] -= block_s
elif bullet_dir == bulletdown:
bullet_coords[1] += block_s
elif bullet_dir == bulletright:
bullet_coords[0] += block_s
elif bullet_dir == bulletleft:
bullet_coords[0] -= block_s

if bullet_coords in wall_coords:
bullet_coords = []
bulletMotion = False
elif bullet_coords in enemy_coords:
enemy_coords.remove(bullet_coords)

for coordinate in enemy_coords:
direction = random.choice(['X', 'Y'])
if direction == 'X':
if [coordinate[0] - block_s, coordinate[1]] in wall_coords:
continue
else:
enemy_dir = enemyleft
coordinate[0] -= block_s

if [coordinate[0] + block_s, coordinate[1]] in wall_coords:
continue
else:
enemy_dir = enemyright
coordinate[0] += block_s

elif direction == 'Y':
if [coordinate[0], coordinate[1] - block_s] in wall_coords:
continue
else:
enemy_dir = enemyup
coordinate[1] -= block_s

if [coordinate[0], coordinate[1] + block_s] in wall_coords:
continue
else:
enemy_dir = enemydown
coordinate[1] += block_s

gameOver = True

screen.blit(background, (0, 0))
for i in wall_coords:
screen.blit(wall, (i[0], i[1]))
for i in enemy_coords:
screen.blit(enemy_dir, (i[0], i[1]))
if bulletMotion == True:
screen.blit(bullet_dir, (bullet_coords[0], bullet_coords[1]))

pygame.display.update()
clock.tick(FPS)
mainLoop()

• Welcome to Code Review! Good job on your first question. – Phrancis Mar 4 '16 at 1:35
• Very nice first question! The code is obviously written in a procedural style (as opposed to OOP, for example). I am interested if this was intended/desired? Obviously there are MANY recommendations that could be made about making this code more OOP-like, but procedural code is perfectly valid in Python. – Tersosauros Mar 4 '16 at 2:19
• @Tersosauros The procedural style was not particularly desired, as I'm not familiar with OOP. If OOP is better organized/more efficient in this scenario, etc, then examples on how to do so would be appreciated. – user99331 Mar 4 '16 at 2:25

Alright, this may be something where you need/desire/end up with multiple reviews - since changing this from it's current (procedural) style to OOP is (very) unlikely to be something someone new to OOP gets right first time.

But, there are certain aspects of this code that could be made better even in a procedural style - so I'll start with these...

## Decomposition

Decomposition, or "functional decomposition" is important in all structured programming paradigms (which includes both OOP and procedural programming). One of the most elementary steps in this process is to construct your programs using modularity and hierarchies.

Structurally, using different scopes for different things is important in creating conceptually seperated 'modules'. For example, in your program the variable purple (a color, obviously), is in the same scope as the variable vodka (in image for a power-up item). Logically speaking, an image and a color do not belong in the same place - they should be separated (into 'modules', of some logical nature).

black = [0, 0, 0]
white = [255, 255, 255]
red = [255, 0, 0]
orange = [255, 102, 0]
yellow = [255, 255, 0]
green = [0, 255, 0]
blue = [0, 0, 255]
purple = [128, 0, 128]


(which are ALL in the __main__ scope,) you could have used a dictionary (Python's dict) data structure, like this:

colors={
"black" : [0, 0, 0],
"white" : [255, 255, 255],
"red" : [255, 0, 0],
"orange" : [255, 102, 0],
"yellow" : [255, 255, 0],
"green" : [0, 255, 0],
"blue" : [0, 0, 255],
"purple" : [128, 0, 128]
}


This would mean, instead of using yellow, you would now use colors["yellow"]. This not only helps you structure your program's code, but also makes it clearer what something is. This is probably not a big deal for "yellow", but when scrolling through and seeing vodka, rather than images["vodka"], it can create confusion.

Note:   in an object oriented design, you may wish to decompose these to a class or subclass rather than using a dictionary.

## Don't Repeat Yourself (DRY)

Some of your code, for example the following block of sprite definitions, is VERY repetitive...

gameover = loadSprite('images/gameover.png', screen_w, screen_h)



Generally, any time your have many lines like this that are largely the same, you're better off using some sort of loop to construct a data structure to contain them. For example:

img_dir = "images/"
screens = {
"gameover" : 'gameover.png',
"background" : 'background.png',
}
for screen in screens.keys():

sprites = {
"item" : 'item.png', "vodka" : 'powerup.png', "wall" : 'wall.png',
"enemyup" : 'enemyup.png', "enemydown" : 'enemydown.png', "enemyright" : 'enemyright.png', "enemyleft" : 'enemyleft.png',
"bulletup" : 'bulletup.png', "bulletdown" : 'bulletdown.png', "bulletright" : 'bulletright.png', "bulletleft" : 'bulletleft.png'
}
for sprite in sprites.keys():


1. Each thing is specified once. Imagine if you moved our images to imgs/ rather than images/ - this way you need only change 1 variable (img_dir) on 1 line (rather than editing 20 lines).

2. You change the name/signature of the loadSprite function. 2 lines to edit rather than 20, etc.

It is fairly apparent that your game world is using square tiles, thus the x and y of the sprites (as oppose to the screens, such as 'background" You may also wish to alter your loadSprite function from:

def loadSprite(path, x, y):


to something which either uses default arguments (x and y defaulting to block_s), or which allows either a tile size (square) OR x and Y (rectangle) to be supplied. For example:

 def loadSprite(path, x, y=None):
return pygame.transform.scale((pygame.image.load(path).convert_alpha()),(x,x if y is None else y))


## On to the OOP!

Your program currently has a number (7, actually) of global variables:

• FPS
• clock
• block_s
• screen_w
• screen_h
• screen
• font

(That is, excluding the 8 colors and 18 sprites that I already mentioned).

These pretty much ALL belong in some sort of class. Probably named something like Game, World, Engine, etc. This would provide a place for all of the data (variables) and behavior (functions, called methods once they're in a class) of the game itself.

Using the hierarchies idea from the decomposition concept, this Game class could hierarchically contain (i.e. be the parent of,) the sprites (or images) and colors data structures created earlier.

The line pygame.init() would go into the constructor, rather than being in the main program body.

Each of the 3 global functions; message, loadMap, and mainLoop could then be made into methods of that class.

Note: the playMusic and loadSprite global functions may or may not be promoted to methods. They seem to me more like helper functions for PyGame, rather than explicit behaviors of your Game/World.

## Your main()... monster?

This function is 153 lines of code, with a cyclomatic complexity of around 54! I really don't even want to begin on it until the program structure (everything else I've talked about) is improved.

However, I will just say that you CLEARLY need decomposition here! Methods should always have a complexity less than 7, so this function would need to be decomposed into around 8 other (smaller) functions. Although, from scanning over it and looking at all your loop conditions (like while True, etc) I think you can probably made it less complex.

As I said at the start of this answer, this may be something where you need/desire/end up with multiple reviews. You can refactor your code and post another question with the new (OOP structured) version (then put a comment on this question pointing to it, etc).

• What an explanation! It deserves a lot of upvotes – Iván Rodríguez Torres Mar 4 '16 at 10:53