Alien Assault game

Question

I know this code is pretty awful, but, by any chance, could someone point out all the flaws you can find and tell me them? It's Python 3.2.3, by the way.

# Alien Assualt
# A bullet hell alien invasion game.
# Bobby Clarke

#Imports
import pygame, sys, os, random, math, easygui
from pygame.locals import *
pygame.init()

screen = pygame.display.set_mode((600, 450))
pygame.display.set_caption("Alien Assualt")

clock = pygame.time.Clock()

# Glitchy Lazer Mode
#pygame.key.set_repeat(1, 1)


def adv_range(limit1, limit2 = None, increment = 1.):
    """
    Range function that accepts floats (and integers).

    Usage:
    adv_range(-2, 2, 0.1)
    adv_range(10)
    adv_range(10, increment = 0.5)

    The returned value is an iterator.  Use list(adv_range) for a list.
    """

    if limit2 is None:
        limit2, limit1 = limit1, 0.
    else:
        limit1 = float(limit1)

    count = int(math.ceil(limit2 - limit1)/increment)
    return (limit1 + n*increment for n in range(count))

def setup(num_of_enemies, enemyclass):
    enemies = []
    for i in range(0, num_of_enemies):
        distance = i * round(400 / num_of_enemies)
        direction = random.choice((True, False)) #True is right, left is false
        enemies.append(enemyclass(x = random.randint(100, 440), y = distance , right = direction))

    return enemies

class player():
    """The player"""
    def __init__(self, x = screen.get_width() / 2 -  35, y = screen.get_height() - 45, img = "Images/player.png", speed = 3):
        self.x = x
        self.y = y
        self.img = pygame.image.load(img)
        self.rightkeys = (K_RIGHT, K_d, K_KP6) #Keys making it move left
        self.leftkeys = (K_LEFT, K_a, K_KP4) #Keys making it move right
        self.left = False
        self.right = False
        self.speed = speed

    def move(self):
        if self.right and self.x <= screen.get_width() - 60:
            self.x += self.speed # Move right
        if self.left and self.x >= 0:
            self.x -= self.speed # Move left

    def fire(self, shot, shots):
        shots.append(shot(self))
        return shots

class shot():
    def __init__(self, player, enemy = False):
        self.x = player.x + 25
        self.y = player.y

        if enemy:
            img = "Images/enemyshot.png"
        else:
            img = "Images/shot.png"

        self.img = pygame.image.load(img)
        self.enemy = enemy
        self.delete = False

    def move(self, speed = 5):
        if self.enemy:
            self.y += speed
        else:
            self.y -= speed

        if self.y <= 0 or self.y > screen.get_height():
            self.delete = True



class enemy():
    def __init__(self, x = random.randint(0, screen.get_width() - 30), y = 0, img = "Images/enemy.png", left = True, right = False, speed = random.randint(4, 7)):
        self.x = x
        self.y = y
        self.img = pygame.image.load(img)
        self.speed = speed

        if right:
            left = False

        self.left =  left
        self.right = right

    def move(self):
        if self.left:
            self.x -= self.speed
        elif self.right:
            self.x += self.speed
        if self.x < 0:
            self.right = True
            self.left = False
        elif self.x > screen.get_width() - 60:
            self.right = False
            self.left = True

    def fire(self, shot, shots):
        shots.append(shot(self, enemy = True))
        return shots

def play(player, enemies, shotclass, secs):
    shots = []
    key = None
    playing = True

    while playing:
        clock.tick(60)
        screen.fill((0, 0, 0)) # Black

        player.move()

        for shot1 in shots:
            shot1.move()

            for shot2 in shots:
                if shot1.x in adv_range(shot2.x, shot2.x + 10) and shot1.y in adv_range(shot2.y, shot2.y + 10) or shot2.x in adv_range(shot1.x, shot1.x + 10) and shot2.y in adv_range(shot1.y, shot1.y + 10):
                    if not shot1 == shot2 and(shot1.enemy == True and shot2.enemy == False) or (shot1.enemy == False and shot2.enemy == True):
                        shot1.delete = True
                        shot2.delete = True

        #Blits

        screen.blit(player.img, (player.x, player.y))

        for shot in shots:
            for enemy in enemies:
                if shot.x in adv_range(enemy.x, enemy.x + 60) and shot.y in adv_range(enemy.y, enemy.y + 20) and not shot.enemy:
                    enemies.remove(enemy)
                    try:
                        shots.remove(shot)
                    except:
                        pass

            if shot.enemy and shot.x in adv_range(player.x, player.x + 60) and shot.y in adv_range(player.y, player.y + 45):
                return False, secs

            if shot.delete:
                try:
                    shots.remove(shot)
                except:
                    pass
            screen.blit(shot.img, (shot.x, shot.y))

        for enemy in enemies:
            screen.blit(enemy.img, (enemy.x, enemy.y))
            enemy.move()
            if random.randint(1, 10 * len(enemies)) == 1:
                shots = enemy.fire(shotclass, shots)

        #   #   #   #   #

        pygame.display.update()

        if not enemies:
            return True, secs

        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                playing = False

            elif event.type == KEYDOWN:
                if event.key in player.rightkeys:
                    player.right = True
                elif event.key in player.leftkeys:
                    player.left = True
                elif event.key == K_SPACE:
                    shots = player.fire(shotclass, shots)

            elif event.type == KEYUP:
                if event.key in player.rightkeys:
                    player.right = False
                elif event.key in player.leftkeys:
                    player.left = False

            elif event.type == USEREVENT:
                secs += 1

def main(playerclass, enemyclass, shotclass):
    win = False
    enemynum = 0
    deaths = 0
    secs = 0

    while enemynum not in range(5, 21):
        enemynum = easygui.enterbox("How many enemies do you want? (5 - 20)", "Alien Assualt")

        try:
            enemynum = int(enemynum)
        except ValueError:
            enemynum = 0
        except TypeError:
            sys.exit()

    pygame.time.set_timer(USEREVENT, 1000) #Fire a user event every second, for the timer

    while not win:
        win, secs = play(playerclass(), setup(enemynum, enemyclass), shotclass, secs)
        deaths += 1

    if secs < 60:
        time = "{0} seconds".format(secs)
    else:
        time = "{0} minute(s) and {1} second(s)".format(math.floor(secs / 60), secs % 60)

    easygui.msgbox("You won in {0} and died {1} times.".format(time, deaths))

if __name__ == "__main__":
    main(player, enemy, shot)

Answer 1

Some problems:

• Classes uncapitalised
• Default parameter equal signs without space (def foo(a=0))
• Too long lines (you can break them!)
• Built-in paths ("Images/player.png")
• Use of too many arguments (use **kwargs instead)

• Return value instead of in-place operations on lists

  def fire(self, shot, shots):
      #You are performing this operation on the mutable object you get
      shots.append(shot(self, enemy = True))
      #Therefore the next line is unnecessary
      return shots
  

You code is actually not that bad, you just have to read PEP8 and use pylint or something :)

Answer 2

I'm not sure the logic of adv_range is doing what you intended:

>>> list(adv_range(4.0, 5.0, 0.3))
[4.0, 4.3, 4.6]
>>> list(adv_range(4.0, 5.1, 0.3))
[4.0, 4.3, 4.6, 4.9, 5.2, 5.5]

Normally you would expect a function like this to consistently stop one step before it reaches limit2, or one step after. You should specify in the function's documentation exactly which numbers you expect it to return and test that it does this (you could even throw in some doctests). Note that the line limit1 = float(limit1) won't have any effect unless float1 was some unusual type like Decimal.

You could also look at numpy if you are going to do a lot of numerical work - it has functions like numpy.arange and numpy.linspace that do essentially what adv_range does.

I haven't looked at this in detail but the way you keep storing directions as a pair of bools (left and right) looks a bit awkward. If you just store the direction as an integer, e.g., +1 for right, -1 for left, 0 for stationary, it could simplify things - you could do self.x += self.speed * self.direction instead of testing both self.left and self.right.