GUI system in PyGame

Question

I'm making a GUI system in Python for game development with PyGame. Right now I have a button that changes colors when you hover and click on it. My question is, is this code well designed or can it be improved?

game.py

import sys, pygame
from pygame.locals import *
from gui import *
def input(events):
 for e in events:
 if e.type == QUIT:
 pygame.quit()
 sys.exit()
pygame.init()
pygame.display.set_caption("GUI Demo")
window = pygame.display.set_mode((640, 480))
screen = pygame.display.get_surface()
clock = pygame.time.Clock()
font = pygame.font.Font(None, 32)
button = Button("btn_1", "Hello World", 50, 50, 200, 120)
button.decorate((255, 0, 0), (0, 255, 0), (0, 0, 255))
while True:
 screen.fill((120, 40, 200))
 input(pygame.event.get())
 button.update(pygame.mouse.get_pos(), pygame.mouse.get_pressed())
 button.draw(screen)
 pygame.display.flip()

gui.py

# Mini-GUI demo
import pygame
BUTTON_NORMAL = 0
BUTTON_HOVER = 1
BUTTON_ACTIVE = 2
class Surface: 
 def __init__(self, x, y, w, h, color):
 self.x = x
 self.y = y
 self.w = w
 self.h = h
 self.color = color
 def collides(self, surface):
 return self.y < surface.top + surface.height and self.y + self.h > surface.top and self.x < surface.left + surface.width and self.x + self.w > surface.left
 def getCoords(self):
 return (self.x, self.y)
 def draw(self, surface):
 pass
class GUIElement(Surface):
 def __init__(self, id, x, y, w, h, color):
 super(GUIElement, self).__init__(x, y, w, h, color)
 self.id = id
class Textbox(GUIElement):
 def __init__(self, id, x, y, w, h, color):
 super(Textbox, self).__init__(id, x, y, w, h, color)
 self.data = ""
 # TODO: update and draw
class Button(GUIElement):
 def __init__(self, id, text, x, y, w, h):
 super(Button, self).__init__(id, x, y, w, h, None)
 self.text = text
 self.font = pygame.font.Font(None, 32)
 self.state = BUTTON_NORMAL
 self.normalColor = None
 self.hoverColor = None
 self.activeColor = None
 def decorate(self, normalColor, hoverColor, activeColor):
 self.normalColor = normalColor
 self.hoverColor = hoverColor
 self.activeColor = activeColor
 self.color = self.normalColor
 def update(self, mouseCoords, mousePressed):
 (up, down, middle) = mousePressed
 if len(mouseCoords) < 4: mouseCoords = pygame.Rect(mouseCoords[0], mouseCoords[1], 1, 1)
 if not self.collides(mouseCoords): 
 self.state = BUTTON_NORMAL
 self.color = self.normalColor
 return
 if up:
 self.state = BUTTON_ACTIVE
 self.color = self.activeColor
 else: 
 self.state = BUTTON_HOVER
 self.color = self.hoverColor
 def draw(self, surface):
 pygame.draw.rect(surface, self.color, pygame.Rect(self.x, self.y, self.w, self.h))
 surface.blit(self.font.render(self.text, 1, (255, 255, 255)), self.getCoords())

• Gareth Rees: The comment "TODO: update and draw" suggests that this code might not be ready for review yet. It's best for you to fix all the problems you know about before submitting it for review.
• Ryan: That's just for the textbox element. The button is complete so far.

Answer 1

1. There are no docstrings. What is the purpose of each of your classes and methods, and how am I supposed to call them?

2. If you click outside the button, and then slide the pointer over the button, it becomes active. Normal GUI buttons require the initial click to be on the button in order to be able to activate it.

3. The class name Surface runs the risk of confusion with pygame.Surface. Or in some applications, of shadowing it.

4. The GUIElement class is a Surface with an id. But none of the code uses the id, so this class is useless. (No doubt you have some purpose in mind, but what is it?)

5. Calling sys.exit() makes it awkward to test your program from the interactive interpreter.

6. If you care about portability to Python 2, you should make Surface inherit from object so that it is a new-style class. On the other hand, if you don't care about portability to Python 2, just call super() instead of super(GUIElement, self) or whatever.

7. Surface takes arguments x, y, w, h. Why not use a pygame.Rect? Then you could just call Rect.colliderect instead of writing your own version.

8. Surface.draw does nothing. Is this because Surface is an abstract base class? If so, you should use metaclass = ABCMeta and decorate the draw method with @abstractmethod to make this clear.

9. In order to create a coloured button, you have to create the Button object and then call its decorate method to set its colours. Why not allow the caller to set the colours when they create the button? (Via optional keyword arguments.) Similarly for the font.

10. Since you can deduce a button's color from its state, you don't need a separate color attribute (or do you?). By having two attributes you run the risk of them getting out of sync. Why not have a map from state to colour?

11. up, down, middle are misleading variable names: the values returned by pygame.mouse.get_pressed() are actually the state of the three mouse buttons. Use names like button1, button2, button3.

12. pygame.mouse.get_pos() returns the mouse coordinates (x, y) so it's not clear why you write if len(mouseCoords) < 4. Won't this always be true? And why do you need to make a rectangle here anyway?