3
\$\begingroup\$

Out of boredom one day I decided to make a game in pygame. I think it turned out fine, but am absolutely sure that it can be improved. How so?

import pygame, random, time, pyquark, pygame.freetype
from configparser import ConfigParser

pygame.freetype.init()

config_object = ConfigParser()
config_object.read("config.ini")

sound_volume = config_object["SOUNDVOLUME"]
scores = config_object["SCORES"]

from pygame.locals import (
    RLEACCEL,
    K_UP,
    K_DOWN,
    K_RIGHT,
    K_LEFT,
    K_SPACE,
    K_ESCAPE,
    K_LSHIFT,
    KEYDOWN,
    QUIT,
)

class Heart(pygame.sprite.Sprite):
    def __init__(self):
        super(Heart, self).__init__()
        self.surf = pygame.image.load("images/hearts.png").convert()
        self.surf.set_colorkey((255, 255, 255), RLEACCEL)
        self.rect = self.surf.get_rect(
            center=(
            random.randint(SCREEN_WIDTH + 20, SCREEN_WIDTH + 100),
            random.randint(0, SCREEN_HEIGHT),
            ))


        global asteroid_speed
        self.speed = random.randint(asteroid_speed[0], asteroid_speed[1])

    def update(self):
        self.rect.move_ip(-self.speed, 0)
        if self.rect.right < 0 or self.rect.top < 45:
            self.kill()

        if player.rect.right > SCREEN_WIDTH:
            self.kill()

        if self.rect.colliderect(player.rect):
            player.lives += 1
            self.kill()

class Heart_empty(pygame.sprite.Sprite):
    def __init__(self):
        super(Heart_empty, self).__init__()
        self.surf = pygame.image.load("images/heart_empty.png").convert()
        self.surf.set_colorkey((255, 255, 255), RLEACCEL)
        self.rect = self.surf.get_rect()

class Player(pygame.sprite.Sprite):
    def __init__(self):
        super(Player, self).__init__()
        self.surf = pygame.image.load("images/ship.png").convert()
        self.rect = self.surf.get_rect()
        self.speed = 5
        self.lives = 3

    # Player movement
    def update(self, pressed_keys):
        if pressed_keys[K_UP]:
            self.rect.move_ip(0, -self.speed)
            move_sound.play()
        if pressed_keys[K_DOWN]:
            self.rect.move_ip(0, self.speed)
            move_sound.play()
        if pressed_keys[K_RIGHT]:
            self.rect.move_ip(self.speed, 0)
            move_sound.play()
        if pressed_keys[K_LEFT]:
            self.rect.move_ip(-self.speed, 0)
            move_sound.play()

        # Collision with walls
        if self.rect.left < 0:
            self.rect.left = 0
        if self.rect.right > SCREEN_WIDTH:
            enemies.update()
            pygame.mixer.music.pause()
            move_sound.stop()
            up1_sound.play()
            time.sleep(2)
            global asteroid_speed, level
            self.rect.move_ip(-740, 0)
            asteroid_speed[1] += 1
            level += 1
            if self.lives != 3:
                self.lives += 1
            pygame.mixer.music.play(loops=-1)

        if self.rect.top <= 55:
            self.rect.top = 55
        if self.rect.bottom >= SCREEN_HEIGHT:
            self.rect.bottom = SCREEN_HEIGHT

class Asteroid(pygame.sprite.Sprite):
    def __init__(self):
        super(Asteroid, self).__init__()
        self.textures = ["images/asteroid.png", "images/asteroid2.png"]
        self.surf = pygame.image.load(self.textures[random.randint(0, 1)]).convert()
        self.surf.set_colorkey((255, 255, 255), RLEACCEL)
        self.rect = self.surf.get_rect(
            center=(
            random.randint(SCREEN_WIDTH + 20, SCREEN_WIDTH + 100),
            random.randint(0, SCREEN_HEIGHT),
            )
        )
        
        global asteroid_speed
        self.speed = random.randint(asteroid_speed[0], asteroid_speed[1])
        self.collided_with_player = False

    def update(self):
        self.rect.move_ip(-self.speed, 0)
        if self.rect.right < 0 or self.rect.top < 45:
            self.kill()

        if player.rect.right > SCREEN_WIDTH:
            self.kill()

# Define screen width and height constants
SCREEN_WIDTH = 800
SCREEN_HEIGHT = 600

pygame.mixer.init()

level = 1

asteroid_speed = [5, 5]

# Set up music & font
pygame.mixer.music.load("sounds/music.wav")
pygame.mixer.music.set_volume(int(sound_volume["music"])/9)
pygame.mixer.music.play(loops=-1)
score_font = pygame.freetype.Font("myfont.ttf", 50)

# Define sfx
win_sound = pygame.mixer.Sound("sounds/game_win.wav")
hit_sound = pygame.mixer.Sound("sounds/hit_sound.wav")
move_sound = pygame.mixer.Sound("sounds/move_sound.wav")
crash_sound = pygame.mixer.Sound("sounds/crash_sound.wav")
up1_sound= pygame.mixer.Sound("sounds/level_up.wav")

hit_sound.set_volume(int(sound_volume["sfx"])/15)
move_sound.set_volume(int(sound_volume["sfx"])/450)
crash_sound.set_volume(int(sound_volume["sfx"])/9)
up1_sound.set_volume(int(sound_volume["music"])/9)
win_sound.set_volume(int(sound_volume["music"])/9)

# Set up the screen
pygame.init()
screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))
pygame.display.set_caption("Planetoids")
icon = pygame.image.load('images/asteroid.png')
pygame.display.set_icon(icon)

# Set up the addenemy custom event
ADDENEMY = pygame.USEREVENT + 1
ADDHEART = pygame.USEREVENT + 2
pygame.time.set_timer(ADDENEMY, 250)
pygame.time.set_timer(ADDHEART, random.randint(500, 10000))

# Set up the player
player = Player()
player.rect.move_ip(0, 260)
player.rect.move_ip(70, 0)

# Set up sprite groups
enemies = pygame.sprite.Group()
helpers = pygame.sprite.Group()
all_sprites = pygame.sprite.Group()
all_sprites.add(player)

# Prepare Hearts
heart1 = Heart()
heart2 = Heart()
heart3 = Heart()
heart_empty = Heart_empty()

# Set up the loop
clock = pygame.time.Clock()
running = True

while running:
    screen.fill((0,0,0)) # Clear display. All sprites should be drawn AFTER this.

    for event in pygame.event.get(): # Check for events
        if event.type == KEYDOWN:
            if event.key == K_ESCAPE:
                running = False

        elif event.type == QUIT:
            running = False

        elif event.type == ADDENEMY:
            new_Asteroid = Asteroid()
            enemies.add(new_Asteroid)
            all_sprites.add(new_Asteroid)

        elif event.type == ADDHEART:
            new_Heart = Heart()
            helpers.add(new_Heart)
            all_sprites.add(new_Heart)


    pressed_keys = pygame.key.get_pressed() # Check witch keys have been pressed
    player.update(pressed_keys) # Update the player with said keys
    helpers.update() # Update helpers
    enemies.update() # Update asteroids


    # Control Hearts
    if player.lives != 0:
        screen.blit(heart1.surf, (20, 20))
        if player.lives != 1:
            screen.blit(heart2.surf, (60, 20))
            if player.lives != 2:
                screen.blit(heart3.surf, (100, 20))

    if player.lives == 1:
        screen.blit(heart_empty.surf, (60, 20))
        screen.blit(heart_empty.surf, (100, 20))
    elif player.lives == 2:
        screen.blit(heart_empty.surf, (100, 20))

    for entity in all_sprites:
        screen.blit(entity.surf, entity.rect)

        if pygame.sprite.spritecollideany(player, enemies): # Check if collided
            if not new_Asteroid.collided_with_player: # Check if the asteroid has already collided with the Player
                # Substract a life, set the asteroids collided player to True so it does not substract more than one, and play some sfx
                player.lives -= 1
                new_Asteroid.collided_with_player = True
                move_sound.stop()

                if player.lives != 0:
                    hit_sound.play()

                # If there are no lives left
                if player.lives == 0:
                    pygame.mixer.music.stop()
                    player.kill()
                    crash_sound.play()
                    time.sleep(2)
                    running = False
                    pyquark.filestart("title.pyw")


    score_font.render_to(screen, (700, 25), str(level), (255, 255, 255))
    # Update Display
    pygame.display.flip()   
    clock.tick(30)# Control Frames

scores["latest"] = str(level)
if int(scores["highscore"]) < level:
    scores["highscore"] = str(level)

with open('config.ini', 'w') as conf:
    config_object.write(conf)

On the development roadmap, I'm thinking of implementing a boss fight maybe each 10 levels.

UPDATE: Link to github: https://github.com/Game1306/Planetoids

\$\endgroup\$
3
  • \$\begingroup\$ Am I correct in the assumption that you mix game logic & input / output? E.g. your classes have logic variables as well as sprites stored in them. If you want to continue this project, I'd recommend keeping the different layers (in this case game logic and input / output) separated. For examples you can get inspiration from MVC or MVVM, although I do not know how much they can be applied to games or python. en.wikipedia.org/wiki/Separation_of_concerns \$\endgroup\$
    – lukstru
    Commented Jun 3, 2022 at 11:05
  • \$\begingroup\$ And can you put a link to the github page (or something equivalent). When I tried to run the code, some assets were missing. \$\endgroup\$
    – lukstru
    Commented Jun 3, 2022 at 11:29
  • 1
    \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. \$\endgroup\$
    – pacmaninbw
    Commented Jun 5, 2022 at 17:55

1 Answer 1

1
\$\begingroup\$

Your code has some quite long methods and would probably benefit from some extracted functions that are appropriately named (i.e. after their intent and not what they do). This can be done quite easily with the refactoring Extract Method.

Example:

class Player(pygame.sprite.Sprite):
    def __init__(self):
        super(Player, self).__init__()
        self.surf = pygame.image.load("images/ship.png").convert()  # I am not familiar with the framework, but what does surf stand for? Would image / texture, etc.. be a better name?
        self.rect = self.surf.get_rect()  # is self.rect used as the position of the player? If yes, name it so.
        self.speed = 5  # this should probably be a constant or in a config file
        self.lives = 3  # this should probably be a constant or in a config file

    # Player movement
    def update(self, pressed_keys):
        if pressed_keys[K_UP]:  # this whole block could be extracted into a function move() or something similar
            self.rect.move_ip(0, -self.speed)
            move_sound.play()
        if pressed_keys[K_DOWN]:
            self.rect.move_ip(0, self.speed)
            move_sound.play()
        if pressed_keys[K_RIGHT]:
            self.rect.move_ip(self.speed, 0)
            move_sound.play()
        if pressed_keys[K_LEFT]:
            self.rect.move_ip(-self.speed, 0)
            move_sound.play()


        # Collision with walls
        if self.rect.left < 0:
            self.rect.left = 0
        if self.rect.right > SCREEN_WIDTH:  # why do different things happen when we go to the left and right outside the screen? If we extract the ifs to methods and name them appropriately, the name of the functions can communicate the intent of the code
            enemies.update()  # global variable access for game logic is usually bad - even if it's okay now it will probably bite you later
            pygame.mixer.music.pause()
            move_sound.stop()
            up1_sound.play()
            time.sleep(2)
            global asteroid_speed, level
            self.rect.move_ip(-740, 0)
            asteroid_speed[2] += 1
            level += 1
            if self.lives != 3:
                self.lives += 1
            pygame.mixer.music.play(loops=-1)

        if self.rect.top <= 55:  # magic constant, should be extracted
            self.rect.top = 55
        if self.rect.bottom >= SCREEN_HEIGHT:
            self.rect.bottom = SCREEN_HEIGHT

Pseudo Code after renaming and extracting:

class Player(pygame.sprite.Sprite):
    def __init__(self):
        super(Player, self).__init__()
        self.texture = pygame.image.load(ImageConstants.player).convert() 
        self.positionAndSize = self.surf.get_rect()  # does this make sense to split into two variables position, size?
        self.speed = GameConstants.playerStartSpeed
        self.lives = GameConstants.playerStartLives

    # Player movement
    def update(self, pressed_keys):
        self.move(pressed_keys)
        self.detectCollisionsAndCorrect()

    def move(self, pressed_keys):
        ...

    def detectCollisionsAndCorrect(self):
        ...

I would further also separate all game sound, input and output from the game logic classes. I like the MVVM pattern.

\$\endgroup\$
2
  • \$\begingroup\$ Why would the lives be a constant or in a config file? I need to be able to change the lives. Wouldn't putting it in config just make the code slower since I'm reading a file? \$\endgroup\$ Commented Jun 8, 2022 at 20:05
  • \$\begingroup\$ The lives do not have to be a constant or in a config file, however if the starting lives are always the same it's a valid approach. If the starting lives vary, they should probably be passed via the constructor. Note that this is the starting lives, not the current lives of the character. You basically don't want magic numbers or strings floating around in your code, that's why we extract them to constants or configs. \$\endgroup\$
    – lukstru
    Commented Jun 9, 2022 at 1:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.