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()) 

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?