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

Using the mouse to avoid asteroids and black holes

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

Problem

I have finished my first project for Python. I would be very grateful if you could check it and give me some feedback. Just simple game with possibility of replay. There are probably lots of mistakes and code could be half the size, but I just started with programming.

I tried this game on a few computers and unfortunately I was unable to run it on some older laptops. There are also issues with the resolution on some computers and I don't know how to fix this.

Is this good enough to show this as an example of my skills for very entry level job in programming?

You can find code, all needed resources and game on GitHub.

```
# SPACE GAME! :-)

def game():

# import libraries

import pygame
from random import randint, choice

# starting pygame

pygame.init()

# screen

size = (900, 700)
screen = pygame.display.set_mode(size)
pygame.display.set_caption("SPACEMAN")
background = pygame.image.load("images/background2.jpg").convert()

finish= False

# clock and colors

clock = pygame.time.Clock()
black = (0, 0, 0)
white = (255, 255, 255)

#loading sounds

explosion = pygame.mixer.Sound('sounds/Explosion.wav')
rock_sound = pygame.mixer.Sound('sounds/rock.wav')
hole_sound = pygame.mixer.Sound('sounds/hole.wav')
nyan_sound = pygame.mixer.Sound('sounds/nyan2.wav')
s1 = pygame.mixer.Sound('sounds/s1.wav')
s2 = pygame.mixer.Sound('sounds/s2.wav')
s3 = pygame.mixer.Sound('sounds/s3.wav')
s4 = pygame.mixer.Sound('sounds/s4.wav')
s5 = pygame.mixer.Sound('sounds/s5.wav')
s6 = pygame.mixer.Sound('sounds/s6.wav')
s7 = pygame.mixer.Sound('sounds/s7.wav')
s8 = pygame.mixer.Sound('sounds/s8.wav')
s9 = pygame.mixer.Sound('sounds/s9.wav')
s10 = pygame.mixer.Sound('sounds/s10.wav')
s11 = pygame.mixer.Sound('sounds/s11.wav')
s12 = pygame.mixer.Sound('sounds/s12.wav')
s13 = pygame.mixer.Sound('sounds/s13.wav')
s14 = pygame.mixer.Sound('sounds/s14.wav')

Solution

Is this good enough to show this as an example of my...skills for very entry level job in programming?

No

At least this was my reaction after reading the first few lines of code: you don't just make one giant blob of a mother of god function that does everything; it is just wrong.

After reading further on, the code feels better than expected. You seems to be missing some basics, though, and some discipline.

Functions

For the most part, your functions are dedicated to a single task, which is a good thing. But you relly so much on the scope to get your variables, it is pretty hard to follow what exactly is going on.

Your collision detection functions, for instance, rely on a global variable being set prior to their call so they can access them. It is extremely error-prone, especially since you're using them quite a few lines after their definition. You should at least pass the object to detect as a parameter. This is even more important because the comparisons are exactly the same, except for the "obstacle" variable.

But most of these functions that acts on existing objects would also be better as methods within a class. Letting inheritance aside for the various obstacles creation, you should at least implement the collision detection something like:

class ItemInSpace(pygame.sprite.Sprite):

    def __init__(self):
        self.image = 0
        self.start_list = []
        self.dx = 0
        self.dy = 0
        self.posx = 0
        self.posy = 0
        self.pos = [self.dx, self.dy]

    def collide(self, x, y):
        if x >= (self.posx - 50) and x = (self.posy - 50) and y = (self.posx - 50)) and (x = (self.posy)) and (y = (self.posx)) and (x = (self.posy + 50)) and (y = (self.posx)) and (x = (self.posy)) and (y <= (self.posy + 50)):
            return True
        else:
            return False


And you use it like:

for hole in hole_list:
    if hole.collide(pos[0], pos[1]):
        life = False
        explosion.play()
        play.rect = [-100, -100]

for rock in rock_list:
    if rock..collide(pos[0], pos[1]):
        life = False
        explosion.play()
        play.rect = [-100, -100]

for nyan in nyan_list:
    if nyan..collide(pos[0], pos[1]):
        nyan_bonus = nyan_bonus + 50
        nyan_list.remove(nyan)


A few other things to note:

  • conditionals involving booleans don't need an extra equality check: it is redundant and perform unnecessary computations;



  • you use way too much parenthesis in your conditionals;



  • you can combine several comparisons at once in a single statement;



  • you can reduce collide to a single line.



Comparisons

By combining comparisons in collide, you can write it like:

class ItemInSpace(pygame.sprite.Sprite):
    ...
    def collide(self, x, y):
        if (self.posx - 50) <= x <= (self.posx + 50) and (self.posy - 50) <= y <= (self.posy + 50):
            return True
        elif (self.posx - 50) <= x <= (self.posx + 50) and (self.posy) <= y <= (self.posy + 50):
            # if this is true, then the previous conditional is true
            # so this will never be executed
            return True
        elif (self.posx) <= x <= (self.posx + 50) and (self.posy + 50) <= y <= (self.posy - 50):
            # Wait, how can y fullfil this condition? Always false, so never executed
            return True
        elif (self.posx) <= x <= (self.posx + 50) and (self.posy) <= y <= (self.posy + 50):
            # If this is true, then the first condition is also true
            # so this is never executed
            return True
        else:
            return False


Even if we "fixed" the third one to have an acceptable range for y, we can see that the first conditional is sufficient for all the cases. Thus the function can become:

class ItemInSpace(pygame.sprite.Sprite):
    ...
    def collide(self, x, y):
        return (self.posx - 50) <= x <= (self.posx + 50) and (self.posy - 50) <= y <= (self.posy + 50)


Granted that when you move your "rocks" and "nyans" you update their .posx and .posy instead of their .pos.

You can use the same comparisons combining technique to simplify holes creation:

if (pos[0] - 200) < hole.posx < (pos[0] + 200) and (pos[1] - 200) < hole.posy < (pos[1] + 100):
    hole_create()
else:
    hole_sound.play()
    hole_list.append(hole)


However, in this case, it is neater to not rely on recursion to check that the hole is not created too close to the player:

``
def hole_create(x, y): # Adding player coordinates as parameter
hole = ItemInSpace()
hole.image = pygame.image.load("images/black_hole.jpg").convert()
hole.image.set_colorkey(black)
posx, posy = x, y # bootstrap while loop
while (x - 200) < posx < (x + 200) and (y - 200) < posy < (y + 100):
# We were too close, chose another position
posx = randint(5, 845) # removing choice here, as you may have notice by using
randint` directly in the next line, it is not necessary
posy =

Code Snippets

class ItemInSpace(pygame.sprite.Sprite):

    def __init__(self):
        self.image = 0
        self.start_list = []
        self.dx = 0
        self.dy = 0
        self.posx = 0
        self.posy = 0
        self.pos = [self.dx, self.dy]

    def collide(self, x, y):
        if x >= (self.posx - 50) and x <= (self.posx + 50) and y >= (self.posy - 50) and y <= (self.posy + 50):
            return True
        elif (x >= (self.posx - 50)) and (x <= (self.posx + 50)) and (y >= (self.posy)) and (y <= (self.posy + 50)):
            return True
        elif (x >= (self.posx)) and (x <= (self.posx + 50)) and (y >= (self.posy + 50)) and (y <= (self.posy - 50)):
            return True
        elif (x >= (self.posx)) and (x <= (self.posx + 50)) and (y >= (self.posy)) and (y <= (self.posy + 50)):
            return True
        else:
            return False
for hole in hole_list:
    if hole.collide(pos[0], pos[1]):
        life = False
        explosion.play()
        play.rect = [-100, -100]

for rock in rock_list:
    if rock..collide(pos[0], pos[1]):
        life = False
        explosion.play()
        play.rect = [-100, -100]

for nyan in nyan_list:
    if nyan..collide(pos[0], pos[1]):
        nyan_bonus = nyan_bonus + 50
        nyan_list.remove(nyan)
class ItemInSpace(pygame.sprite.Sprite):
    ...
    def collide(self, x, y):
        if (self.posx - 50) <= x <= (self.posx + 50) and (self.posy - 50) <= y <= (self.posy + 50):
            return True
        elif (self.posx - 50) <= x <= (self.posx + 50) and (self.posy) <= y <= (self.posy + 50):
            # if this is true, then the previous conditional is true
            # so this will never be executed
            return True
        elif (self.posx) <= x <= (self.posx + 50) and (self.posy + 50) <= y <= (self.posy - 50):
            # Wait, how can y fullfil this condition? Always false, so never executed
            return True
        elif (self.posx) <= x <= (self.posx + 50) and (self.posy) <= y <= (self.posy + 50):
            # If this is true, then the first condition is also true
            # so this is never executed
            return True
        else:
            return False
class ItemInSpace(pygame.sprite.Sprite):
    ...
    def collide(self, x, y):
        return (self.posx - 50) <= x <= (self.posx + 50) and (self.posy - 50) <= y <= (self.posy + 50)
if (pos[0] - 200) < hole.posx < (pos[0] + 200) and (pos[1] - 200) < hole.posy < (pos[1] + 100):
    hole_create()
else:
    hole_sound.play()
    hole_list.append(hole)

Context

StackExchange Code Review Q#133656, answer score: 12

Revisions (0)

No revisions yet.