patternpythonMinor
Space Shooter made using Pygame
Viewed 0 times
spacemadeusingpygameshooter
Problem
This is my first game made using Pygame of about 550 lines of code. Advice and suggestions are more than welcome.
```
## Game music Attribution
##Frozen Jam by tgfcoder licensed under CC-BY-3
from __future__ import division
import pygame
import random
from os import path
## assets folder
img_dir = path.join(path.dirname(__file__), 'assets')
sound_folder = path.join(path.dirname(__file__), 'sounds')
###############################
## to be placed in "constant.py" later
WIDTH = 480
HEIGHT = 600
FPS = 60
POWERUP_TIME = 5000
# Define Colors
WHITE = (255, 255, 255)
BLACK = (0, 0, 0)
RED = (255, 0, 0)
GREEN = (0, 255, 0)
BLUE = (0, 0, 255)
YELLOW = (255, 255, 0)
###############################
###############################
## to placed in "__init__.py" later
## initialize pygame and create window
pygame.init()
pygame.mixer.init() ## For sound
screen = pygame.display.set_mode((WIDTH, HEIGHT))
pygame.display.set_caption("Space Shooter")
clock = pygame.time.Clock() ## For syncing the FPS
###############################
font_name = pygame.font.match_font('arial')
def main_menu():
global screen
menu_song = pygame.mixer.music.load(path.join(sound_folder, "menu.ogg"))
pygame.mixer.music.play(-1)
title = pygame.image.load(path.join(img_dir, "main.png")).convert()
title = pygame.transform.scale(title, (WIDTH, HEIGHT), screen)
screen.blit(title, (0,0))
pygame.display.update()
while True:
ev = pygame.event.poll()
if ev.type == pygame.KEYDOWN:
if ev.key == pygame.K_RETURN:
break
elif ev.key == pygame.K_q:
pygame.quit()
quit()
else:
draw_text(screen, "Press [ENTER] To Begin", 30, WIDTH/2, HEIGHT/2)
draw_text(screen, "or [Q] To Quit", 30, WIDTH/2, (HEIGHT/2)+40)
pygame.display.update()
#pygame.mixer.music.stop()
ready = pygame.mixer.Sound(path.join(sound_folder,'getready.ogg'))
ready.play()
```
## Game music Attribution
##Frozen Jam by tgfcoder licensed under CC-BY-3
from __future__ import division
import pygame
import random
from os import path
## assets folder
img_dir = path.join(path.dirname(__file__), 'assets')
sound_folder = path.join(path.dirname(__file__), 'sounds')
###############################
## to be placed in "constant.py" later
WIDTH = 480
HEIGHT = 600
FPS = 60
POWERUP_TIME = 5000
# Define Colors
WHITE = (255, 255, 255)
BLACK = (0, 0, 0)
RED = (255, 0, 0)
GREEN = (0, 255, 0)
BLUE = (0, 0, 255)
YELLOW = (255, 255, 0)
###############################
###############################
## to placed in "__init__.py" later
## initialize pygame and create window
pygame.init()
pygame.mixer.init() ## For sound
screen = pygame.display.set_mode((WIDTH, HEIGHT))
pygame.display.set_caption("Space Shooter")
clock = pygame.time.Clock() ## For syncing the FPS
###############################
font_name = pygame.font.match_font('arial')
def main_menu():
global screen
menu_song = pygame.mixer.music.load(path.join(sound_folder, "menu.ogg"))
pygame.mixer.music.play(-1)
title = pygame.image.load(path.join(img_dir, "main.png")).convert()
title = pygame.transform.scale(title, (WIDTH, HEIGHT), screen)
screen.blit(title, (0,0))
pygame.display.update()
while True:
ev = pygame.event.poll()
if ev.type == pygame.KEYDOWN:
if ev.key == pygame.K_RETURN:
break
elif ev.key == pygame.K_q:
pygame.quit()
quit()
else:
draw_text(screen, "Press [ENTER] To Begin", 30, WIDTH/2, HEIGHT/2)
draw_text(screen, "or [Q] To Quit", 30, WIDTH/2, (HEIGHT/2)+40)
pygame.display.update()
#pygame.mixer.music.stop()
ready = pygame.mixer.Sound(path.join(sound_folder,'getready.ogg'))
ready.play()
Solution
You're following good style conventions, that's a good sign. A lot of people don't do this consistently. (Note I'm not sure if you're a beginner, you said this was your first game but that was all). You do occasionally let lines run too long or have the wrong spacing. It's best if you read the PEP0008, Python's official style guide. I have some general notes from reading through your code, but I've definitely missed things to be improved. On pep8online you can post your whole code in and get a breakdown of every part of your code that it finds in violation of PEP rules. It's not perfect, but will reveal some good rules for you to follow with tons of examples. I still recommend reading PEP0008 when you can.
You lay out colour constants at the start, but then you don't always use them. For instance, in
and not
At the risk of stating the obvious, constants are better to use as they're more readable to understand, and easier to change. If you decide that you want to adjust your colour palette to make the game look blue tinted, you could do that easily if all colours are set by your constant values.
In
I presume
Also you declare
In
Still on constants, you may like to use something called a Class constant. When you declare a class, you usually set its attributes in
This makes it easier to see that
You lay out colour constants at the start, but then you don't always use them. For instance, in
main_menu, why use screen.fill((0,0,0))and not
screen.fill(BLACK)At the risk of stating the obvious, constants are better to use as they're more readable to understand, and easier to change. If you decide that you want to adjust your colour palette to make the game look blue tinted, you could do that easily if all colours are set by your constant values.
In
draw_shield_bar you use an if statement to ensure that pct is always 0 at the lowest. An alternative is to use max, which will return the maximum of values passed to it. ie. if you pass max(pct, 0) then it returns pct if it's 0 or higher, but otherwise will return 0. Your mileage may vary on which is more readable but I prefer:pct = max(pct, 0)I presume
pct is percent, but it's not a clear name. Try to avoid such shortened names, they usually just add confusion. A clear full word makes your code a lot more readable, and shaving those characters doesn't really get you much.Also you declare
BAR_HEIGHT and BAR_LENGTH locally in the function but they're named as if they're constants, with all uppercase letters. You should either make them constants at the top level, alongside your other constants like the colours that will eventually be in constants.py or just declare them as normal values that happen to always be the same. ie. length = 100. I suggest the former, as long as you can name them clearly.In
draw_lives, you calculate img_rect.x by using 30. But since 30 isn't a variable, it's not at all clear what it corresponds to. This is known as a magic number, because the reason behind it is unknown to the reader. These should be avoided, instead you should have a named constant like LIFE_SPACING or something more appropriate so that it's much easier to follow what's happening.Still on constants, you may like to use something called a Class constant. When you declare a class, you usually set its attributes in
__init__, but you can also set constant attributes above __init__, in the class definition. Like this:class Explosion(pygame.sprite.Sprite):
FRAME_RATE = 75
def __init__(self, center, size):
...This makes it easier to see that
FRAME_RATE is constant across all Explosions. It can still be accessed with self.Explosion, and can be additionally accessed with Explosion.FRAME_RATE, which can be useful when you want to get the value from outside a class instance context.Code Snippets
screen.fill((0,0,0))screen.fill(BLACK)pct = max(pct, 0)class Explosion(pygame.sprite.Sprite):
FRAME_RATE = 75
def __init__(self, center, size):
...Context
StackExchange Code Review Q#117875, answer score: 5
Revisions (0)
No revisions yet.