2
\$\begingroup\$

I am currently making a game for myself and i have a lot of code in a function which does almost the exact same thing with some minor differences. I was wondering if there is an way to optimize this code and still get the same output.

import pygame, os, random

def draw_game():
    player = pygame.Rect(450, 350, HOOFD_HEIGHT, HOOFD_WIDTH)

    BULLETS_L = [] 
    BULLETS_R = [] 
    BULLETS_UP = [] 

    HITPOINTS = 4 
    
    B_SPAWN = random.uniform(1000, 6000)# a random number between 1000 and 6000
    E1 = pygame.USEREVENT + 3 #an custom event 
    pygame.time.set_timer(E1, 1 * int(B_SPAWN))# this activates the event after 1 to 6 seconds
    
    B_SPAWN = random.uniform(1000, 6000)
    E2 = pygame.USEREVENT + 4
    pygame.time.set_timer(E2, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E3 = pygame.USEREVENT + 5
    pygame.time.set_timer(E3, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E4 = pygame.USEREVENT + 6
    pygame.time.set_timer(E4, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E5 = pygame.USEREVENT + 7
    pygame.time.set_timer(E5, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E6 = pygame.USEREVENT + 8
    pygame.time.set_timer(E6, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E7 = pygame.USEREVENT + 9
    pygame.time.set_timer(E7, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E8 = pygame.USEREVENT + 10
    pygame.time.set_timer(E8, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E9 = pygame.USEREVENT + 11
    pygame.time.set_timer(E9, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E10 = pygame.USEREVENT + 12
    pygame.time.set_timer(E10, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E11 = pygame.USEREVENT + 13
    pygame.time.set_timer(E11, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E12 = pygame.USEREVENT + 14
    pygame.time.set_timer(E12, 1 * int(B_SPAWN))
    B_SPAWN = random.uniform(1000, 6000)
    E13 = pygame.USEREVENT + 15
    pygame.time.set_timer(E13, 1 * int(B_SPAWN))

    clock = pygame.time.Clock()
    run = True

    while run == True:
        clock.tick(FPS)

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

            if event.type == E1: 
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E1, 1 * int(B_SPAWN))
                bullet = pygame.Rect(0, 280, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_L.append(bullet)

            if event.type == E2:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E2, 1 * int(B_SPAWN))
                bullet = pygame.Rect(0, 70, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_L.append(bullet)
                        
            if event.type == E3:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E3, 1 * int(B_SPAWN))
                bullet = pygame.Rect(0, 620, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_L.append(bullet)

            if event.type == E4:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E4, 1 * int(B_SPAWN))
                bullet = pygame.Rect(0, 430, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_L.append(bullet)

            if event.type == E5:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E5, 1 * int(B_SPAWN))
                bullet = pygame.Rect(WIDTH, 350, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_R.append(bullet)
                    
            if event.type == E6:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E6, 1 * int(B_SPAWN))
                bullet = pygame.Rect(WIDTH, 175, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_R.append(bullet)

            if event.type == E7:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E7, 1 * int(B_SPAWN))
                bullet = pygame.Rect(WIDTH, 525, BULLET_WIDTH, BULLET_HEIGHT)
                BULLETS_R.append(bullet)

            if event.type == E8:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E8, 1 * int(B_SPAWN))
                bullet = pygame.Rect(350, HEIGHT, BULLET_HEIGHT, BULLET_WIDTH)
                BULLETS_UP.append(bullet)

            if event.type == E9:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E9, 1 * int(B_SPAWN))
                bullet = pygame.Rect(550, HEIGHT, BULLET_HEIGHT, BULLET_WIDTH)
                BULLETS_UP.append(bullet)

            if event.type == E10:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E10, 1 * int(B_SPAWN))
                bullet = pygame.Rect(750, HEIGHT, BULLET_HEIGHT, BULLET_WIDTH)
                BULLETS_UP.append(bullet)
                
            if event.type == E11:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E11, 1 * int(B_SPAWN))
                bullet = pygame.Rect(50, HEIGHT, BULLET_HEIGHT, BULLET_WIDTH)
                BULLETS_UP.append(bullet)
                    
            if event.type == E12:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E12, 1 * int(B_SPAWN))
                bullet = pygame.Rect(235, HEIGHT, BULLET_HEIGHT, BULLET_WIDTH)
                BULLETS_UP.append(bullet)
                
            if event.type == E13:
                B_SPAWN = random.uniform(1000, 6000)
                pygame.time.set_timer(E13, 1 * int(B_SPAWN))
                bullet = pygame.Rect(1050, HEIGHT, BULLET_HEIGHT, BULLET_WIDTH)
                BULLETS_UP.append(bullet)
                
            if event.type == PLAYER_HIT:
                HITPOINTS -= 1
                BULLET_HIT_SOUND.play()
        
        keys_pressed = pygame.key.get_pressed()
        movement(keys_pressed, player)

        handle_bullets_R(BULLETS_R, player)
        handle_bullets_L(BULLETS_L, player)
        handle_bullets_up(BULLETS_UP, player)
        draw_window(player, BULLETS_L, HITPOINTS, BULLETS_R, BULLETS_UP)

        DEAD_TXT = ""

        if HITPOINTS <= 0:
            DEAD_TXT = "GAME OVER"
        

        if DEAD_TXT != "":
            end_text(DEAD_TXT)
            run = False
    
    if run == False:
        draw_menu()

as seen above, there is alot of code pieces which could probably be optimized way better.

thanks in advance

\$\endgroup\$
1
  • 1
    \$\begingroup\$ The title should be the purpose of the app, not the area you want reviewed. Thanks. \$\endgroup\$
    – ggorlen
    Commented Feb 27, 2023 at 17:17

1 Answer 1

3
\$\begingroup\$

a lot of code ... which does almost the exact same thing

I am glad that you recognize this and wish to learn how to DRY it up.

There is a separate structural difficulty in this code that we should address. The function draw_game is entirely too long, and should be broken up via Extract Helper. We strive to write functions that can be visually taken in all at once, with no scrolling, as an aid to human comprehension. Usually that means if we've gone on for more than about fifty lines of source, it's time to break it into smaller chunks that focus on doing just one thing.


I'm not sure, but I think that possibly you have a player which is HOOFD_HEIGHT tall that might be a horse? Or some other hoofed animal? In which case, prefer to spell it out: HOOFED_HEIGHT.

Or perhaps keep both dimensions together in a single tuple:

HOOFED_SIZE = (123, 456)  # the original posting doesn't specify

player = pygame.Rect(450, 350, *HOOFED_SIZE)

That starred expression will supply both numbers to the .Rect method.

BTW, kudos on naming these MANIFEST_CONSTANTS according to the usual convention.


    BULLETS_L = [] 
    BULLETS_R = [] 
    BULLETS_UP = [] 

You mentioned you'd like to coalesce some things. I haven't even see how these are used yet, but the naming is suggestive. We might combine the first two, or perhaps all three, in a single datastructure that has a velocity associated with it. For example, "left" velocities might look like -1, and "right" might correspondingly be 1.

We probably want lowercase names like bullets_l here, as these don't seem to be constants. Similarly for hitpoints.


    HITPOINTS = 4 

This is a magic number. We could define INITIAL_HEALTH or INITIAL_HP somewhere. But it would probably be more convenient to push the magic number into the signature, as a value that typically we just let default:

def draw_game(hitpoints=4):

One advantage of that practice is it lets unit tests conveniently alter the value according to their need.


    B_SPAWN = random.uniform(1000, 6000)
    E1 = pygame.USEREVENT + 3 
    pygame.time.set_timer(E1, 1 * int(B_SPAWN))

Push this repeated code down into a helper:

from enum import Enum

def spawn(event_type: EventType):
    b_spawn = random.uniform(1000, 6000)
    event = pygame.USEREVENT + event_type
    pygame.time.set_timer(event, 1 * int(b_spawn))
    return event
...
def draw_game...
    ...
    e1 = spawn(EventType.BULLET)
    e2 = spawn(EventType.MISSLE)
    ...
class EventType(Enum):
    BULLET = 3
    MISSLE = 4
    ...

If "b" denotes "bullet", please spell it out, or at a minimum offer a # comment. The source code is unclear, so I'm just guessing here.

An enum is a classic way to create objects whose value is arbitrary and they merely need to be distinct from one another. That is, enums are one way of coping with magic numbers and giving them meaningful names.


    run = True

This sounds like a verb, to me, and therefore a terrific name for a def run(): function. Variables OTOH often want noun identifiers, and here an adjective would make sense, perhaps running or active. A common loop idiom is to define done = False and then keep going while not done:


        for event in pygame.event.get():

Break out a helper:

def dispatch(event):
    ...

Then the for loop mostly becomes a single helper call and we can bury the complexity one level down.

            if event.type == E1: 

There's a lot of copy-n-paste going on here. Rule of thumb: if you notice you keep leaning on the CMD-V paste key, it's time to start think about parameters instead.

Using a bunch of ifs is not terrible, exactly; feel free to keep that structure during initial refactor. But a dict that maps from event type to its relevant parameters can be an excellent way to implement such dispatch. You can even put function objects into the map.


        DEAD_TXT = ""

        if HITPOINTS <= 0:
            DEAD_TXT = "GAME OVER"
        

        if DEAD_TXT != "":
            end_text(DEAD_TXT)
            run = False
    
    if run == False:
        draw_menu()

Prefer to deal with it all at once in a single if clause:

        if HITPOINTS <= 0:
            end_text("GAME OVER")
            run = False    

    draw_menu()

Notice that the menu draw happens unconditionally when we fall off the bottom of the loop. You had a check which was redundant with the check at top of loop. Making the whole while loop fit in a single screenful would have made that more obvious.


Overall?

Naming could be improved; that will come with practice.

Break up giant functions that don't fit within a screenful, using Extract Helper.

\$\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.