2
\$\begingroup\$

This is my first attempt at python game in pygame. It's a maze game where the maze is randomly generated using Prim's Algorithm.

import pygame
import random
import time

pygame.init()
# all fonts used
font1 = pygame.font.SysFont("comicsansms", 49, True)
font2 = pygame.font.SysFont("comicsansms", 150, True)
font3 = pygame.font.SysFont("comicsansms", 28, True)

# creates the string that displays time
def get_time(hours,minutes,seconds):
    if len(str(hours)) > 1:
        a = str(hours)
    else:
        a = "0" + str(hours)

    if len(str(minutes)) > 1:
        b = str(minutes)
    else:
        b = "0" + str(minutes)

    if len(str(seconds)) > 1:
        c = str(seconds)
    else:
        c = "0" + str(seconds)

    return a + ":" + b + ":" + c

# creates the time counter
def draw_time(start_time,pause_time):
    hours = 0
    minutes = 0
    seconds = 0
    current_time = time.time() - pause_time - start_time
    if current_time > 3600:
        while True:
            if current_time - 3600 > 0:
                hours += 1
                current_time -= 3600
            else:
                while True:
                    if current_time - 60 > 0:
                        minutes += 1
                        current_time -= 60
                    else:
                        seconds += int(current_time)
                        break
                break

    else:
        while True:
            if current_time - 60 > 0:
                minutes += 1
                current_time -= 60
            else:
                seconds += int(current_time)
                break

    return [font1.render(get_time(hours, minutes, seconds), True, (0, 0, 0), (255, 255, 255)), get_time(hours, minutes, seconds)]

class cell:
    def __init__(self,up,down,left,right):
        self.visited = False
        self.walls = [up,down,left,right]

class labyrinth:
    # generates the maze
    def __init__(self,id):
        self.id = id
        self.walls = []
        self.maze_walls = []
        self.cells = []

        x = 0
        t = 0

        # creates all cell within the maze
        for f in range(22):
            for s in range(28):
                # if command makes sure no cellls are created where the clock is supposed to be
                if not (f in (0,1,2) and s > 20):
                    self.cells.append(cell((x + 8, t, 25, 8), (x + 8, t + 33, 25, 8), (x, t + 8, 8, 25), (x + 33, t + 8, 8, 25)))
                x += 33
            x = 0
            t += 33

        # generates maze using prim's algorithm
        for v in self.cells[0].walls:
            self.maze_walls.append(v)
            self.walls.append(v)

        self.cells[0].visited = True

        while len(self.walls) > 0:
            wall = random.choice(self.walls)
            # checks which cells are divided by the wall
            divided_cells = []
            for u in self.cells:
                if wall in u.walls:
                    divided_cells.append(u)

            if len(divided_cells) > 1 and (not ((divided_cells[0].visited and divided_cells[1].visited) or ((not divided_cells[0].visited) and (not divided_cells[1].visited)))):
                # checks which cells have been visited
                for k in divided_cells:
                    k.walls.remove(wall)

                    if k.visited == False:
                        k.visited = True

                    for q in k.walls:
                        if not q in self.walls:
                            self.walls.append(q)

                        if not q in self.maze_walls:
                            self.maze_walls.append(q)

                    if wall in self.maze_walls:
                        self.maze_walls.remove(wall)

            self.walls.remove(wall)

        for j in range(0,736,33):
            for i in range(0,951,33):
                self.maze_walls.append((i, j, 8, 8))

    # draws the maze
    def draw(self, goal):
        screen.fill((0, 0, 0))

        for k in self.maze_walls:
            pygame.draw.rect(screen, color, pygame.Rect(k[0],k[1],k[2],k[3]))

        pygame.draw.rect(screen, color, pygame.Rect(695, 0, 300, 105)) # clock background
        pygame.draw.rect(screen, (0, 255, 0), goal) # finish

id = 0
running = True
while running:
    screen = pygame.display.set_mode((930, 733))
    done = False
    color = (0, 128, 255) # color of the walls
    x = 16
    y = 16
    clock = pygame.time.Clock()
    start = time.time()
    id += 1
    maze = labyrinth(id)
    goal = pygame.Rect(899,701,25,25)
    victory = False
    speed = 4 # movement speed
    pause = False
    pause_time = 0 # time spent in pause menue

    while not done:
        for event in pygame.event.get():

            if event.type == pygame.QUIT:
                done = True
                running = False

            if event.type == pygame.KEYDOWN:
                if event.key == pygame.K_ESCAPE or event.key == pygame.K_p:
                    if pause:
                        pause = False
                        pause_time += time.time() - pause_time_start
                    else:
                        pause = True
                        pause_time_start = time.time()

                if event.key == pygame.K_RETURN:
                    done = True

        if pause:
            screen.fill((0, 0, 0))
            pause_text = font2.render("PAUSE",True,(255,255,255))
            screen.blit(pause_text, (468 - (pause_text.get_width() // 2), 368 - (pause_text.get_height() // 2)))

        # the actual game
        if not victory and not pause:
            move_up = True
            move_down = True
            move_left = True
            move_right = True
            pressed = pygame.key.get_pressed()

            # movment
            if pressed[pygame.K_w] or pressed[pygame.K_UP]:
                # checks if their is a overlap with the wall
                for m in maze.maze_walls:
                    player = pygame.Rect(x, y - speed, 10, 10)
                    if player.colliderect(pygame.Rect(m[0],m[1],m[2],m[3])):
                        move_up = False
                        break
                if move_up:
                    y -= speed

            if pressed[pygame.K_s] or pressed[pygame.K_DOWN]:
                player = pygame.Rect(x, y + speed, 10, 10)
                for m in maze.maze_walls:
                    if player.colliderect(pygame.Rect(m[0],m[1],m[2],m[3])):
                        move_down = False
                        break
                if move_down:
                    y += speed

            if pressed[pygame.K_a] or pressed[pygame.K_LEFT]:
                player = pygame.Rect(x - speed, y, 10, 10)
                for m in maze.maze_walls:
                    if player.colliderect(pygame.Rect(m[0],m[1],m[2],m[3])):
                        move_left = False
                        break
                if move_left:
                    x -= speed

            if pressed[pygame.K_d] or pressed[pygame.K_RIGHT]:
                player = pygame.Rect(x + speed, y, 10, 10)
                for m in maze.maze_walls:
                    if player.colliderect(pygame.Rect(m[0],m[1],m[2],m[3])):
                        move_right = False
                        break
                if move_right:
                    x += speed

            # checks if player has reached the goal
            if goal.colliderect((x, y, 10, 10)):
                victory = True

            # draws the screen
            maze.draw(goal)
            text = draw_time(start, pause_time)
            pygame.draw.rect(screen, (255, 100, 0), pygame.Rect(x,y,10,10))
            screen.blit(text[0], (700, 15))

        # victory screen
        if victory:
            screen.fill((0, 0, 0))
            time_text = font1.render("Time Taken: " + text[1],True,(255,255,255))
            victory_text = font2.render("VICTORY!",True,(255,255,255))
            reset = font3.render("(Press Enter to Start New Game)",True,(255,255,255))

            screen.blit(victory_text,(468 - (victory_text.get_width() // 2), 328 - (victory_text.get_height() // 2)))
            screen.blit(time_text, (468 - (time_text.get_width() // 2), (248 - (time_text.get_height() // 2)) + victory_text.get_height()))
            screen.blit(reset, (468 - (reset.get_width() // 2), (248 - (reset.get_height() // 2)) + victory_text.get_height() + time_text.get_height()))

        clock.tick(60)
        pygame.display.flip()
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$
  • One of your goals should be to avoid repetitive code. This is called the DRY principle. If you have identical code (e.g. functions that call almost the same parameters, blocks of code that are almost identical except for one word), the repetition can almost certainly be reduced by creating more functions or looking into other structures.
  • PEP 8 is the style guide that Python programmers commonly use. It makes your code easier for others to read, and also helps you to define standardized interfaces whose calling conventions will be easy to remember. I reading through it and starting to learn the conventions. There also automated checkers that you can run your code through to point out improvements to you: I use pylint for this. There's one thing I immediately notice in your code that does not follow PEP 8:
    • Classes should be named following the CapWords convention (source):
      • labyrinth should be Labyrinth
  • When creating strings that have a pattern str.format() and formatted string literals come in handy.

There are easier ways to format strings

Take this function:

def get_time(hours,minutes,seconds):
    if len(str(hours)) > 1:
        a = str(hours)
    else:
        a = "0" + str(hours)

    if len(str(minutes)) > 1:
        b = str(minutes)
    else:
        b = "0" + str(minutes)

    if len(str(seconds)) > 1:
        c = str(seconds)
    else:
        c = "0" + str(seconds)

    return a + ":" + b + ":" + c

The three conditionals are not necessary; Python already has three ways to pad strings to two digits. My personal favorite is formatted string literals. Instead of:

    if len(str(hours)) > 1:
        a = str(hours)
    else:
        a = "0" + str(hours)

You can just do:

    a = f"{a:0>2}"

Here's how it works (see here for more details):

f             string prefix thats starts a formatted string literal
 "
  {           starts a variable reference
   a          variable name
    :         start formatting section
     0        padding character
      >       right align
       2      number of characters to pad to
        }     end variable reference
         "

And from there you could do:

def get_time(hours,minutes,seconds):
    return ":".join(f"{part:0>2}" for part in [a, b, c])

But as @Reinderien mentions, it's better to use strftime here. I also recommend reading his answer for more suggestions.

\$\endgroup\$
1
\$\begingroup\$

Time Handling

Most of your time handling code needs to be rewritten. Read more about time in Python here:

https://docs.python.org/3/library/time.html

https://docs.python.org/3/library/datetime.html

Specifically,

  • The entire get_time method needs to be replaced with a call to strftime
  • Stop passing around hours, minutes and seconds separately. Just pass around a datetime.time.
  • Most of your draw_time method needs to go away. Keep the call to render, but the call to get_time should be replaced with a call to strftime.

Formatting

Your code is not PEP8-compliant. In particular, your class names need to be capitalized. Running your code through a linter will help this.

Magic numbers

What do 22, 28, 736, 695, 930, etc. mean? These need to be replaced with constant variables. Where possible, calculate them from other constants.

DRY (don't repeat yourself)

This block:

        if pressed[pygame.K_w] or pressed[pygame.K_UP]:
            # checks if their is a overlap with the wall
            for m in maze.maze_walls:
                player = pygame.Rect(x, y - speed, 10, 10)
                if player.colliderect(pygame.Rect(m[0],m[1],m[2],m[3])):
                    move_up = False
                    break
            if move_up:
                y -= speed

is repeated four times with very little modification. Consider moving it to a function, and accepting arguments for anything that varies (the two key values, and a 2-tuple of ints to add to the coordinates).

\$\endgroup\$
1
\$\begingroup\$

get_time has a fair amount of repetition. I'd move the "padding" check to a new function, and just map it over a list holding the three pieces of time data:

def zero_pad_format(n):
    if n < 10:
        return "0" + str(n)
    else:
        return str(n)

def get_time(hours, minutes, seconds):
    padded = [zero_pad_format(t) for t in [hours, minutes, seconds]]

    return ":".join(padded)

>>> get_time(5, 7, 43)
'05:07:43'

Really, it's unnecessary to turn the number into a string just to check if it needs to be padded. If the stringified number has a string length of greater than 1, that also means it's >= 10. You can just check if the number is less than or equal to 9 to see if you should pad it. That's what I'm doing in the first function. Instead of writing the same code three times, I moved it to its own function so the single function can be used three times.

padded = [zero_pad_format(t) for t in [hours, minutes, seconds]]

This puts the three time data into a list, and maps zero_pad_format over it, and returns a list padded containing the padded data. The line below it

return ":".join(padded)

Takes the padded data, and "joins" the data together with ":". I recommend getting very familiar with join. It's a very useful function.


The looping bit at the top of draw_time to get hours, minutes, and seconds from a raw seconds number is quite messy and inefficient. You're looping where straight math can be used:

def time_from_seconds(elapsed_seconds):
    current_time = elapsed_seconds

    hours = current_time // 3600
    current_time -= 3600 * hours

    minutes = current_time // 60
    current_time -= 60 * minutes

    return hours, minutes, current_time

>>> time_from_seconds(5000)
(1, 23, 20)

>>> time_from_seconds(5001)
(1, 23, 21)

>>> time_from_seconds(3600)
(1, 0, 0)

>>> time_from_seconds(1234567890) 3 How long would this take if using loops?
(342935, 31, 30)

Use division to figure out how many hours you can get out of the current_time, store the number in hours, then multiply that by 3600 to figure how much needs to be subtracted from current_time. Then you do the same for minutes.

There's some repetition here that could be ironically be solved by using some loops, but I feel that wouldn't be quite as clear.

That gets rid of the majority of the code in draw_time though, which is a hint that that code shouldn't have been directly in draw_time in the first place. That chunk of code has nothing to do with drawing the time. It's just figuring out what to draw. You might want a set-up closer to this:

def draw_time(hours, minutes, seconds):
    return (font1.render(get_time(hours, minutes, seconds),
                         True,
                         (0, 0, 0), (255, 255, 255)),
            get_time(hours, minutes, seconds))

# An awful name, and this still isn't an ideal setup. It's mildy better though.
def draw_time_from_start_pause(start_time, pause_time):
    current_time = time.time() - pause_time - start_time

    h, m, s = time_from_seconds(current_time)
    return draw_time(h, m, s)

    # or

    #data = time_from_seconds(current_time)
    #return draw_time(*data) # Spread the data into the arguments
\$\endgroup\$

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.