Top-Down Dungeon RPG

Question

I'm making a top-down dungeon minigame where you are a knight and can attack zombies by swinging a sword. So far I haven't implemented collision, player rect(), enemy rect(), a game over screen, or the player sword animation, but all the code I have so far is working well, even though the main loop is crowded. I was wondering how I could either condense the code or streamline it?

import pygame, random, os
from pygame.locals import *

pygame.init()

scr_width = 1020
scr_height = 510
screen = pygame.display.set_mode((scr_width, scr_height))
pygame.display.set_caption('Dungeon Minigame')

clock = pygame.time.Clock()

images = {}
path = 'Desktop/Files/Dungeon Minigame/'
filenames = [f for f in os.listdir(path) if f.endswith('.png')]
for name in filenames:
    imagename = os.path.splitext(name)[0]
    images[imagename] = pygame.image.load(os.path.join(path, name))

font = pygame.font.SysFont('Times_New_Roman', 27)

white = [240, 240, 240]

def main_menu():

    onclick = False

    while True:

        mx, my = pygame.mouse.get_pos()

        screen.blit(images['background'], (0, 0))

        button = screen.blit(images['button'], (480, 300))

        if button.collidepoint((mx, my)):
            if onclick:
                game()

        for event in pygame.event.get():
            if event.type == QUIT:
                pygame.quit()
                sys.exit()

            if event.type == MOUSEBUTTONDOWN:
                onclick = True

        pygame.display.update()
        clock.tick(60)

def game():

    lives = 3
    score = 0

    def player(x, y):
        screen.blit(images['r_knight'], (playerX, playerY))

    class Enemy:
        def __init__(self):
            self.x = random.randint(8, 800)
            self.y = random.randint(8, 440)
            self.moveX = 0
            self.moveY = 0

        def move(self, speed = 1):
            if self.x > playerX:
                self.x -= speed
            elif self.x < playerX:
                self.x += speed

            if self.y < playerY:
                self.y += speed
            elif self.y > playerY:
                self.y -= speed

        def draw(self):
            screen.blit(images['r_zombie'], (self.x, self.y))

    def enemy(x, y):
        screen.blit(images['r_zombie'], (x, y))

    enemy_list = []
    
    for i in range(4):
        new_enemy = Enemy()
        enemy_list.append(new_enemy)

    playerX = 510
    playerY = 220

    while True:

        screen.blit(images['background'], (0, 0))

        score_text = font.render('Score: ' + str(score), True, white)
        lives_text = font.render('Lives: ', True, white)

        screen.blit(score_text, (20, 20))
        screen.blit(lives_text, (840, 20))

        screen.blit(images['r_knight'], (playerX, playerY))

        if lives == 3:
            screen.blit(images['triple_heart'], (920, 0))

        if lives == 2:
            screen.blit(images['double_heart'], (920, 0))

        if lives == 1:
            screen.blit(images['single_heart'], (920, 0))

        if lives <= 0:
            screen.blit(images['triple_empty_heart'], (920, 0))
            if lives < 0:
                lives = 0

        onpress = pygame.key.get_pressed()

        if onpress[pygame.K_a]:
            playerX -= 3
            screen.blit(images['l_knight'], (playerX, playerY))

        if onpress[pygame.K_w]:
            playerY -= 3
            screen.blit(images['l_knight'], (playerX, playerY))

        if onpress[pygame.K_d]:
            playerX += 3
            screen.blit(images['r_knight'], (playerX, playerY))

        if onpress[pygame.K_s]:
            playerY += 3
            screen.blit(images['r_knight'], (playerX, playerY))

        if onpress[pygame.K_w] and onpress[pygame.K_a]:
            screen.blit(images['l_knight'], (playerX, playerY))

        if onpress[pygame.K_s] and onpress[pygame.K_a]:
            screen.blit(images['l_knight'], (playerX, playerY))

        if onpress[pygame.K_w] and onpress[pygame.K_d]:
            screen.blit(images['r_knight'], (playerX, playerY))

        if onpress[pygame.K_s] and onpress[pygame.K_d]:
            screen.blit(images['r_knight'], (playerX, playerY))

        if playerX <= -8:
            playerX = -8
        elif playerX >= 965:
            playerX = 965

        if playerY <= 5:
            playerY = 5
        elif playerY >= 440:
            playerY = 440
        
        for enemy in enemy_list:
            enemy.move()
            enemy.draw()

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

        clock.tick(60)
        pygame.display.update()
main_menu()

Answer 1

Use GLOBALS for constant settings, positions, etc.

In Python we do not have syntax to declare a variable that is no longer allowed to change, but amongst the community it is very well understood that CAPS LOCK variables are to be seen as constants, i.e., variables that will never change.

So, your variables that you create and should not be changed could actually be defined in upper case, e.g. SCR_WIDTH and SCR_HEIGHT.

Furthermore, throughout your code you have "magic numbers" here and there, which are numbers you have determined but that from reading the code it is not clear how they arose. For example, you always make sure the player position in the X axis remains between -8 and 965, but why those values..? Not only that, but those "magic" numbers tend to appear in more than one place, so if you change one, you have to look for all the occurrences of that number to change it again. What is more, what happens if two or more magic numbers happen to be the same?! Code becomes really confusing.

Therefore, it is advisable that you take all such magic numbers and/or constants, and give them names. For example, I like to have my constants for the game in the beginning of the script. Looking at your code, these are just some examples of things I would move to the beginning:

SCR_WIDTH, SCR_HEIGHT = 1024, 512
WHITE = [240, 240, 240]
PLAYER_LIVES_POSITION = (920, 0)
PLAYER_X_BETWEEN = (-8, 965)
PLAYER_Y_BETWEEN = (5, 440)
ENEMY_X_BETWEEN = (8, 800)
ENEMY_Y_BETWEEN = (8, 440)
FPS = 60

Answer 2

Streamlining blitting of the player

You did a good job in realising you can streamline some of your code, because you have some patterns that are repeated over and over again. That is always a good indication that something shorter can be done.

Taking a look at the series of ifs you have to blit the correct images, here is a possible suggestion to make it more condensed, like you say.

Compute player position changes

# ...
onpress = pygame.key.get_pressed()

dx = 0
if onpress[pygame.K_a]:
    dx -= 3
if onpress[pygame.K_d]:
    dx += 3
playerX += dx

dy = 0
if onpress[pygame.K_w]:
    dy -= 3
if onpress[pygame.K_s]:
    dy += 3
playerY += dy

Blit knight afterwards

Notice that your code has 8 if that are not exclusive, so if at a certain point in time the player is pressing down on AWSD at the same time, your game will make a total of eight blits on top of each other, which is unnecessary.

If you compute the position changes like I showed above, or in any other way you prefer, then you can use dx and dy to decide which player image to blit. For example, you can just look at dx that tells you if the player moved left or right, and blit accordingly:

if dx > 0:
    screen.blit(images['r_knight'], (playerX, playerY))
elif dx < 0:
    screen.blit(images['l_knight'], (playerX, playerY))

Attention: The code I shared is not strictly equivalent to yours and I took the liberty of interpreting your intentions, please let me know if my interpretation is wrong.

Answer 3

Loading images with pathlib

Currently, you are loading your images like so:

import os

images = {}
path = 'Desktop/Files/Dungeon Minigame/'
filenames = [f for f in os.listdir(path) if f.endswith('.png')]
for name in filenames:
    imagename = os.path.splitext(name)[0]
    images[imagename] = pygame.image.load(os.path.join(path, name))

If you turn to pathlib, which I understand to be the more modern alternative to os.path, you see you can rewrite your image loading as such:

import pathlib

images = {}
path = pathlib.Path('Desktop/Files/Dungeon Minigame/')
for img_path in path.glob("*.png"):
    images[img_path.name] = pygame.image.load(img_path)

or perhaps even a bit shorter if you don't need the original path anywhere else:

import pathlib

images = {}
for img_path in pathlib.Path('Desktop/Files/Dungeon Minigame/').glob("*.png"):
    images[img_path.name] = pygame.image.load(img_path)

Notice that shorter isn't always better, I am just trying to avoid creating too many variables that are used just once.

However, if you end up having to load many resources from that same folder, you probably want to declare it as a variable in all CAPS LOCK (which is the universal way to signal a constant, global variable) and then use it in the loop to load images and elsewhere.

Answer 4

Clipping player position

There are a couple of places where you do an operation that is fairly common, which is to ensure a value v is between a lower bound l and an upper bound u, and forcing v to be there if it isn't. This is commonly referred to as clipping, and you do it in a couple of places (more prominently, when dealing with the player position) so you can actually create a short function to do so for you, which makes the rest of the code more to-the-point.

What we want to do is shorten this code:

if playerX <= -8:
    playerX = -8
elif playerX >= 965:
    playerX = 965

if playerY <= 5:
    playerY = 5
elif playerY >= 440:
    playerY = 440

For example, if you install clip_values, you can do

from clip_values import clip
playerX = clip(playerX).between_(-8).and_(965)
playerY = clip(playerY).between_(5).and_(440)

If you don't want to install anything, you can also do something like

def clip(value, lower, upper):
    return min(upper, max(value, lower))

which is then used as follows:

playerX = clip(playerX, -8, 965)
playerY = clip(playerY, 5, 440)

Clipping the lives

Now that you have this clip function, you can actually take a look at this:

        if lives == 3:
            screen.blit(images['triple_heart'], (920, 0))

        if lives == 2:
            screen.blit(images['double_heart'], (920, 0))

        if lives == 1:
            screen.blit(images['single_heart'], (920, 0))

        if lives <= 0:
            screen.blit(images['triple_empty_heart'], (920, 0))
            if lives < 0:
                lives = 0

The way this code is written makes it look like the user always has between 0 and 3 lives, and then you want to blit the appropriate image of the lives the user has. If you start by clipping the number of lives to be in [0, 1, 2, 3], you can then determine programmatically the image to blit:

heart_images = ["triple_empty_heart", "single_heart", "double_heart", "triple_heart"]
lives = clip(lives).between_(0).and_(3)
screen.blit(images[heart_images[lives]], (920, 0))

Answer 5

Create a Player class

The player has a fair amount in common with your zombies:

1. There is logic to move() the Player.
2. Players and monsters both have a blittable image.
3. Players and monsters both have a "facing" direction.
4. Players and monsters both have a speed of movement.

There's enough there to form a base class:

class Mobile:
    def __init__(self, **kwargs):
        self.x = kwargs['x']
        self.y = kwargs['y']
        self.speed = kwargs['speed']
        self.facing = kwargs['facing']
        self.images = ...

    def move(self): ...

    def draw(self):
        screen.blit(self.images[self.facing], (self.x, self.y))

The .move() for Monsters and Players is obvious different -- monsters will just pursue the player, as you have coded. The Player's move() can contain the logic you have to check the pressed keys and to clamp the position, although I think that you are adjusting the position, then drawing the image, then clamping the position, which seems like a bug.

Break your code into functions and methods

Make it explicit what is being done. Pass whatever arguments you need to pass.

I would rewrite your main loop like this:

mobiles = [Player(), *[Monster() for _ in range(4)]]
still_playing = True

while still_playing:

    draw_background()

    for mob in mobiles:
        mob.move()
        mob.draw()

    still_playing = was_quit_pressed()

    pygame.display.update()
    clock.tick(60)