Menu with buttons using pygame

Question

I wrote this to teach myself how to make a menu with buttons and I want to know if I've done well and if there's any way to make it shorter and/or more efficient.

import pygame, sys
from pygame import *
run = True
global newBG
newBG = False
#-------------------------------------------------------------------------------
def mainLoop():
 global win, newW, newH, newBG
 newW, newH = (960 , 508)
 win = pygame.display.set_mode ((newW, newH) , pygame.RESIZABLE)
 pygame.display.set_caption("SMC")
 backGround = 0
 while run:
 global mouse, currentScreen
 mouse = pygame.mouse.get_pos()
 currentScreen()
 while newBG:
 drawBackground()
 newBG = False
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 pygame.quit()
 sys.exit()
 if event.type == pygame.VIDEORESIZE:
 win = pygame.display.set_mode ( (event.w, event.h) , pygame.RESIZABLE ) 
 newW, newH = event.w, event.h
 drawBackground()
 uiPlate()
 if event.type == pygame.MOUSEBUTTONDOWN:
 for b in buttonList:
 b.click()
 pygame.display.update()
#------------------------------------------------------------------------------
def mainMenu():
 global backGroundImage, buttonList, currentScreen
 backGroundImage = pygame.image.load('Background.jpg')
 ButtonPic = pygame.image.load('Button.jpg')
 ButtonHoverPic = pygame.image.load('ButtonHover.jpg')
 ButtonPressPic = pygame.image.load('ButtonPress.jpg')
 buttonList = [
 button(screenSwap, secondMenu, ButtonPic, ButtonHoverPic, ButtonPressPic, .1, .1, .1, .1),
 button('Hello', None, ButtonPic, ButtonHoverPic, ButtonPressPic, .1, .25, .1, .1),
 button('Hello to you!', None, ButtonPic, ButtonHoverPic, ButtonPressPic, .1, .50, .1, .1)
]
 for b in buttonList:
 b.drawButton()
#------------------------------------------------------------------------------
def secondMenu():
 global backGroundImage, buttonList
 backGroundImage = pygame.image.load('Button.jpg')
 ButtonPic = pygame.image.load('Background.jpg')
 ButtonHoverPic = pygame.image.load('ButtonHover.jpg')
 ButtonPressPic = pygame.image.load('ButtonPress.jpg')
 buttonList = [
 button(screenSwap, mainMenu, ButtonPic, ButtonHoverPic, ButtonPressPic, .75, .1, .1, .1),
 button('Goodbye', None, ButtonPic, ButtonHoverPic, ButtonPressPic, .75, .25, .1, .1),
 button('Goodbye to you friend!', None, ButtonPic, ButtonHoverPic, ButtonPressPic, .75, .50, .1, .1)
]
 for b in buttonList:
 b.drawButton()
#------------------------------------------------------------------------------
uiX = 0
uiY = 0
uiW = 960
uiH = 508
def uiPlate():
 global uiW, uiH, uiX, uiY
 uiW = (int(newW))
 uiH = (int(uiW*0.5296875))
 if uiH > newH:
 uiH = int(newH)
 uiW = (int(uiH/0.5296875))
 uiX = (int((newW - uiW)/2))
 uiY = (int((newH - uiH)/2))
## pygame.draw.rect(win, (100, 255, 100,), (uiX, uiY, uiW, uiH))
## print(uiW, uiH, uiX, uiY)
#------------------------------------------------------------------------------
def drawBackground():
 global backGround
 backGround = pygame.transform.scale(backGroundImage, (newW, newH))
 win.blit((backGround), (0,0))
#------------------------------------------------------------------------------
class button:
 global uiX, uiY, uiW, uiH
 def __init__(self, action, actionProps, ButtonPic, ButtonHoverPic, ButtonPressPic, x, y, w, h):
 self.ButtonPic = ButtonPic
 self.ButtonHoverPic = ButtonHoverPic
 self.ButtonPressPic = ButtonPressPic
 self.bx = int(uiX + (x * uiW))
 self.by = int(uiY + (y * uiH))
 self.bw = int(w * uiW)
 self.bh = int(h * uiH)
 self.action = action
 self.actionProps = actionProps
 def drawButton(self):
 click = pygame.mouse.get_pressed()
 ButtonPic = pygame.transform.scale(self.ButtonPic, (self.bw, self.bh))
 ButtonHoverPic = pygame.transform.scale(self.ButtonHoverPic, (self.bw, self.bh))
 ButtonPressPic = pygame.transform.scale(self.ButtonPressPic, (self.bw, self.bh))
 win.blit(ButtonPic, (self.bx, self.by))
 if ((self.bx < mouse[0] < (self.bx + self.bw)) and (self.by < mouse[1] < (self.by + self.bh))):
 win.blit(ButtonHoverPic, (self.bx, self.by))
 if click[0]:
 win.blit(ButtonPressPic, (self.bx, self.by))
 def click(self):
 if ((self.bx < mouse[0] < (self.bx + self.bw)) and (self.by < mouse[1] < (self.by + self.bh))):
 if self.actionProps != None:
 self.action(self.actionProps)
 else:
 print(self.action)
#------------------------------------------------------------------------------
def screenSwap(screen):
 global currentScreen, newBG
 currentScreen = screen
 newBG = True
#------------------------------------------------------------------------------
currentScreen = mainMenu
mainLoop()

Sorry about the length, if it's too much to bother looking through, I'd like to be shown some code for a well-made menu system that I can compare to.

• Reinderien: mainMenu and secondMenu are incorrectly indented. Please fix this before review.
• Chris Bitney: Not sure what you mean.
• Reinderien: Try copying your code from here and running it. It'll show you where you have syntax failures due to indentation.

Answer 1

PEP-0008

Follow PEP-0008 guidelines. Use an automated checker (pylint, pyflakes, ...) to check your code for violations.

Issues include:

• Variable, function, and method names should be snake_case. For example newBG should be new_background, mainLoop should be main_loop, and drawButton should be draw_button.
• No space between a function/method name and the opening parentheses. set_mode ( should be set_mode(.
• No spaces before commas, and no spaces between adjacent parenthesis, or after opening parenthesis or before closing parenthesis. set_mode ( (event.w, event.h) , pygame.RESIZABLE ) should be set_mode((event.w, event.h), pygame.RESIZABLE).
• Class names should begin with uppercase letters. So Button instead of button.

to name a few. Run a checker to see all of them.

Import *

import pygame, sys
from pygame import *

Are you importing pygame, or are you importing everything from inside pygame? Do one or the other, preferably the former, but definitely not both!

Global variables are Global

global newBG
newBG = False

newBG is being defined in the global scope. global newBG is not needed, and is useless.

Type Consistency

What is the type of backGround? Is it an image? If so, then explain:

 backGround = 0

It looks like you are trying to assign an integer to something which normally holds an image. But wait! It is a local variable, and is not used anywhere.

While is not If

Is this a loop or an if statement?

 while newBG:
 drawBackground()
 newBG = False

It can never execute more than once, because it sets the loop condition to False. The following is clearer:

 if newBG:
 drawBackground()
 newBG = False

Integer Arithmetic

If you don't want floating point values, don't divided by two. Instead integer-divide by two!

Instead of this:

uiX = (int((newW - uiW)/2))
uiY = (int((newH - uiH)/2))

write this:

uiX = (newW - uiW) // 2
uiY = (newH - uiH) // 2

Persistance

Your main loop reads roughly:

while run:
 currentScreen()
 # Handle events
 pygame.display.update()

currentScreen() either calls mainMenu() or secondMenu():

def mainMenu():
 # load 4 images
 # create 3 button objects in a list
 # draw button objects
def secondMenu():
 # load 4 images
 # create 3 button objects in a list
 # draw button objects

So ... every iteration of your main event loop (say, 30 to 60 times a second), you are:

• LOADING MULTIPLE IMAGES
• CREATING UI ELEMENTS
• drawing some stuff
• DISCARDING THE IMAGES AND UI ELEMENTS

Forgive the above screaming. You don't want to waste time while painting your UI doing repetitive, redundant file I/O. Load the images once, at startup or the first time they are used. Create your buttons once, or at the very most, create them only when you swap to the new screen. Populate the button list when switching to this new screen. As this button list must maintain its value across multiple iterations of the event loop, you'll want this as a global variable, or ...

Stop Using Global Variables

Create a main class for your application, and store your "global" information as main class self members.

Create a screen class, and create two instances of it: one for your main menu screen, the other for your second menu. The screen class would hold:

• the button list for that screen.
• the background image for that screen.
• a scaled copy of the background image (so you don't have to rescale it each iteration of the event loop!)

If the application detects VIDEORESIZE, and when changing screens, it would tell the screen object, so the screen object can re-layout its objects and rescale the background image.

Single-Coat Paint

What does this do?

 win.blit(ButtonPic, (self.bx, self.by))
 if ...:
 win.blit(ButtonHoverPic, (self.bx, self.by))
 if ...:
 win.blit(ButtonPressPic, (self.bx, self.by))

Unless ButtonPic, ButtonHoverPic and ButtonPressPic are not all the same size, or if ButtonHoverPic or ButtonPressPic have transparent areas, the above code can paint the button only to immediately repaint the button, and perhaps immediately repaint it a third time. Given that Premium One-Coat Guaranteed PaintTM is likely being used, this is doubling or tripling the cost of painting for no gain.

Instead, I'd recommend something more like:

 image = self._image
 if ...:
 if ...:
 image = self._press_image
 else:
 image = self._hover_image
 win.blit(image, (self.bx, self.by)