1
\$\begingroup\$

I'm making a Start Screen class for my game, it has 3 buttons and they're spread out horizontally on the screen. The screen also has a scrolling background, moves from top to bottom. It's a very basic Start Screen but the code is 111 lines long. I'm sure there's a much more efficient way of doing things, any tips on how to shorten the file or do things better is very much appreciated, thank you!

Code:

import pygame
import game
import math

pygame.init()

def main():
    fps = 60

    SCREEN_WIDTH = 1000
    SCREEN_HEIGHT = 700
    FONT = pygame.font.SysFont("comicsans", 30)

    screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))
    pygame.display.set_caption("Space Avoiders")

    background = pygame.image.load("background1.png").convert()
    tiles = math.ceil(SCREEN_WIDTH / background.get_height()) + 1

    clock = pygame.time.Clock()

    scroll = 0
    def redraw_window(scroll):
        screen.blit(screen, (0, 0))

        for i in range(-1, tiles):
            screen.blit(background, (0, i * background.get_height() + scroll))

    def start_menu_buttons():
        mouse = pygame.mouse.get_pos()

        #Buttons
        start = pygame.image.load("Sprites/Buttons/Start.jpg")
        start_hover = pygame.image.load("Sprites/Buttons/Start_Hover.jpg")
        start_rect = start.get_rect()
        start_x = 0
        start_y = screen.get_height() / 2
        start_rect.x = start_x
        start_rect.y = start_y

        settings = pygame.image.load("Sprites/Buttons/Settings.jpg")
        settings_hover = pygame.image.load("Sprites/Buttons/Settings_Hover.jpg")
        settings_clicked = pygame.image.load("Sprites/Buttons/Settings_Clicked.jpg")
        settings_rect = settings.get_rect()
        settings_x = screen.get_width()/2 - (settings_rect.width / 2)
        settings_y = screen.get_height() / 2
        settings_rect.x = settings_x
        settings_rect.y = settings_y

        customize = pygame.image.load("Sprites/Buttons/Customize.jpg")
        customize_hover = pygame.image.load("Sprites/Buttons/Customize_Hover.jpg")
        customize_clicked = pygame.image.load("Sprites/Buttons/Customize_Clicked.jpg")
        customize_rect = customize.get_rect()
        customize_x = screen.get_width() - customize_rect.width
        customize_y = screen.get_height() / 2
        customize_rect.x = customize_x
        customize_rect.y = customize_y


        start_blit = start
        settings_blit = settings
        customize_blit = customize

        collide_start = start_rect.colliderect(mouse, (1,1))
        collide_settings = settings_rect.colliderect(mouse, (1,1))
        collide_customize = customize_rect.colliderect(mouse, (1,1))

        button_pressed = pygame.mouse.get_pressed()[0]

        if collide_start:
            start_blit = start_hover
        if collide_start and pygame.mouse.get_pressed()[0]:
            pygame.display.quit()
            game.game()

        if collide_settings:
            settings_blit = settings_hover
        if collide_settings and button_pressed:
            settings_blit = settings_clicked

        if collide_customize:
            customize_blit = customize_hover
        if collide_customize and button_pressed:
            customize_blit = customize_clicked

        screen.blits(((start_blit, (start_x, start_y)),
                      (settings_blit, (settings_x, settings_y)),
                      (customize_blit, (customize_x, customize_y))))

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

        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                run = False
                break

        redraw_window(scroll)
        start_menu_buttons()
        pygame.display.update()
        scroll += 1.5

        if scroll >= background.get_height():
            scroll = 0

    pygame.quit()

if __name__ == "__main__":
    main()
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I believe that the most important issue with your code is that since you call start_menu_buttons() inside your loop, you end up loading all the button images again and again in every iteration of the main loop.

It's much more efficient to load the buttons only once at the beginning of the function and just reference them later.

Besides that, I think encapsulating the behavior of your buttons into classes would avoid a lot of code repetition, but supposing you are consciously avoiding using classes for some reason, I have a few other suggestions:

1 - One thing I usually do when loading assets for games is to find a way to load everything automatically based on a list of resources. You could do something like this, in the beginning of your main function:

button_names = ["Start", "Start_Hover", "Settings", "Settings_Hover", "Settings_Clicked", 
                "Customize", "Customize_Hover", "Customize_Clicked"]

button_images = dict()

for name in button_names:
    button_images[name] = pygame.image.load("Sprites/Buttons/{}.jpg".format(name))

And then, you would refer to your assets by their key in the dict. For example:

start_rect = button_images["Start"].get_rect() 

2 - Your if conditions can be organized in a more efficient and clear way. For example, you check collide_settings, and if its true, you change the value of settings_blit. Immediately below this condition, you check the truth of collide_settings again, and if button_pressed is also true, you change the value of settings_blit again.

This could be replaced by:

if collide_settings:
    if button_pressed:
        settings_blit = settings_clicked
    else:
        settings_blit = settings_hover

Although you end up with an extra line of code, you may reduce that using Python's equivalent to a ternary operator:

if collide_settings:
    settings_blit = settings_clicked if button_pressed else settings_hover

3 - For all your buttons, you define button_y as equal to screen.get_height() / 2, and then you set button_rect.y with this value. Instead, you could declare a "constant" like BUTTONS_Y = SCREEN_HEIGHT/2 and set button_rect.y directly. I believe Pygame even allows you to use this rect as the dest parameter of its blits function, ignoring its width and height and using just its coordinates.

\$\endgroup\$
1
  • \$\begingroup\$ I am incredibly late to respond to you, life has gotten in the way of so much! But I still wanted to thank you for reviewing my code and giving me insight! I'm not consciously avoiding using classes, I guess I just didn't realize where I should be using them, I'm still a bit new at this! Thank you so much for your response and I will keep these things in mind! \$\endgroup\$ Commented Oct 2, 2023 at 1:14

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.