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

Simple 'Space Invaders' clone

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

Problem

This is my second game, not yet finished but working. I've put so much time into it I wanted to get feedback before continuing. I always learn so much from those that comment.

Whilst my first (still a work in progress), is object orientated, this isn't. Given that though, I certainly appreciate the OO method more now.

Please try the game out and post your feedback:

```
# Space Invaders
# By Dave
#
# A simple Space Invaders clone
#
# Planned features not yet implemented:
# - bonus dropped if strobing invader hit
# - high scores tracking using pickled data
# - explosion effects (drawn using colored pixels?)
# - more....

import math
import pygame
import random
import sys
from itertools import cycle
from datetime import datetime
from pygame import gfxdraw
from pygame.locals import *

def print_text(surface, font, text, surf_rect, x = 0, y = 0, center = False,\
color = (255, 255, 255)):
"""
Draws text onto a surface. If center, text is centered on screen at y
"""
if not center:
textimage = font.render(text, True, color)
surface.blit(textimage, (x, y))
else:
textimage = font.render(text, True, color)
text_rect = textimage.get_rect()
x = (surf_rect.width // 2) - (text_rect.width // 2 )
surface.blit(textimage, (x, y))

def game_is_over(surface, font, ticks):
timer = ticks
surf_rect = surface.get_rect()
surf_height = surf_rect.height
surf_width = surf_rect.width
print_text(screen, font, "G A M E O V E R", surf_rect, y = 260,\
center = True)
pygame.display.update()
while True:
ticks = pygame.time.get_ticks()
for event in pygame.event.get():
if event.type == QUIT:
pygame.quit()
sys.exit()
if ticks > timer + 3000:
break

def next_level(level):
level += 1
if level > 6:
level = 6
return level

def load_level(level):
# create and populate(not all)

Solution

Just a minor improvements.

1) If you count something manually or repeat exact lines one after other probably you do something wrong. Use iteration if possible

def draw_bonus_invader(i, bonus_color, bx, bonus_x):
    x, y = bonus_invader.x, bonus_invader.y
    pygame.draw.circle(backbuffer, bonus_color, (x+bx, y+7), 2)
    if i == 5:
        bx = next(bonus_x)


2) Try not to use \ to split logical line. You don’t need it anyway if splitting inside (). Also I'm not familiar with PyGame, so I can guess what the parameters are but not always:

# wrong
pygame.draw.line(backbuffer, (150, a, b),(rect.x+115, rect.y),\
                     (rect.x+50, rect.y-55),2 )

# better
x, y = rect.x, rect.y
pygame.draw.line(Surface=backbuffer,   color=(150, a, b), 
                 start_pos=(x+115, y), end_pos=(x+50, y-55), width=2)


3) # create user events and # event timers looks a bit painful. You should try to use dictionaries more often for creating special options and then just iterate over them.

Also I'd create list of strings ['bonus', 'invader_shoot', ...] and just use it to create dictionary

events_map = {'bonus':         (event, opt_1, opt_2), 
              'invader_shoot': (event, opt_1, opt_2),
              ...}


But that requires replacing many variables like this:

bonus -->     event = events_map['bonus'][0]              
all param --> event, opt_1, opt_2 = events_map['bonus']


Also note that I try to sort functions and variables in alphabetic order if possible.

# create user events

bonus                  = pygame.USEREVENT + 5
invader_shoot          = pygame.USEREVENT + 4
move_invaders_down     = pygame.USEREVENT + 2
move_invaders_sideways = pygame.USEREVENT + 1
reload                 = pygame.USEREVENT + 3

# event timers

options_map = {bonus:                  BONUS_FREQ,
               invader_shoot:          INV_SHOOT_FREQ,
               move_invaders_down:     0,
               move_invaders_sideways: MOVE_SIDEWAYS,
               reload:                 RELOAD_SPEED
               }
for (event, option) in event_map.items():
    pygame.time.set_timer(event, option)


4) Multiple if statements are bad, since they all will be processed one after another every time code is executed. Use elif or better: dictionaries. Again:

# Assigned functions aren’t executed. You just assign names (if without ())
event_map = {bonus: bonus_foo, invader_shoot: invader_shoot_foo,
             KEYUP: KEYUP_foo, reload: reload_foo,  ...}

# event loop
while True:
    ticks = pygame.time.get_ticks()
    for event in pygame.event.get():
        if event.type == QUIT:
            pygame.quit()
            sys.exit()
        elif not game_over:
            # Here is where execution begins
            event_map[event.type]()


5) Try to organise code in general:
  • Defined functions are in random order. Use alphabetic. It will be easier to find and edit each.
  • Constants are everywhere. Constants block are in non-clear order. And they are somewhat mixed with code. Define them at the place they are needed if possible. If used in many situations, place somewhere at the beginning or use separate module just for the constants and functions.



Edit:

If you don’t want to introduce classes you should probably stick with elif statements. You can try to modify global variables, but very carefully since they can mess code logic easily and usually this a bad practice to use many global variables:

reloaded = False

def reload_f():
    global reloaded
    reloaded = True        

print(reloaded) #--> False
reload_f()
print(reloaded) #--> True


On the other hand with classes you can edit class parameters anywhere in the code as well as obtain parameter values:

class MyShip(object):

    def __init__(self):
        object.__init__(self)
        self.isReloaded = False
        self.bullets = 0

    def reload(self):
        self.isReloaded = True
        self.bullets = 10

player = MyShip()
print(player.isReloaded, player.bullets) #--> False, 0

bullets = player.reload()
print(player.isReloaded, player.bullets) #-->True, 10


So instead of elif statement:

# define some classes with useful methods
player = MyShip()
invaders = InvaderShips()

for event in pygame.event.get():
    if event.type == QUIT:
        pygame.quit()
        sys.exit()
    elif not game_over:
        event_map = {reload: player.reload, 
                     move_invaders_sideways: invaders.move_sideways,
                     ...}

        if event.type in event_map:
            event_map[event.type]

Code Snippets

def draw_bonus_invader(i, bonus_color, bx, bonus_x):
    x, y = bonus_invader.x, bonus_invader.y
    pygame.draw.circle(backbuffer, bonus_color, (x+bx, y+7), 2)
    if i == 5:
        bx = next(bonus_x)
# wrong
pygame.draw.line(backbuffer, (150, a, b),(rect.x+115, rect.y),\
                     (rect.x+50, rect.y-55),2 )

# better
x, y = rect.x, rect.y
pygame.draw.line(Surface=backbuffer,   color=(150, a, b), 
                 start_pos=(x+115, y), end_pos=(x+50, y-55), width=2)
events_map = {'bonus':         (event, opt_1, opt_2), 
              'invader_shoot': (event, opt_1, opt_2),
              ...}
bonus -->     event = events_map['bonus'][0]              
all param --> event, opt_1, opt_2 = events_map['bonus']
# create user events

bonus                  = pygame.USEREVENT + 5
invader_shoot          = pygame.USEREVENT + 4
move_invaders_down     = pygame.USEREVENT + 2
move_invaders_sideways = pygame.USEREVENT + 1
reload                 = pygame.USEREVENT + 3

# event timers

options_map = {bonus:                  BONUS_FREQ,
               invader_shoot:          INV_SHOOT_FREQ,
               move_invaders_down:     0,
               move_invaders_sideways: MOVE_SIDEWAYS,
               reload:                 RELOAD_SPEED
               }
for (event, option) in event_map.items():
    pygame.time.set_timer(event, option)

Context

StackExchange Code Review Q#142892, answer score: 6

Revisions (0)

No revisions yet.