Pygame menu to launch games - follow up

Question

Follow up of Pygame menu to launch games

I rewrote most of the code per the one answer. I'm looking for further advice to improve to the code. I'm afraid some of the abstraction may be too Java-like. Some of the comments may be a bit stupid but no one in the group really knows Python or PyGame (including me).

menu.py:

#!/usr/bin/python
import sys
import pygame
import json
import collections
import os
# Holds references to image objects
class Library:
 def __init__(self, font):
 self.image_library = {}
 self.text_library = {}
 self.font = font
 # returns the image found at the path given and creates a reference to it if needed
 def get_image(self, path):
 image = self.image_library.get(path)
 if image is None:
 try:
 image = pygame.image.load(path)
 except pygame.error:
 print 'image ' + path + ' not found'
 image = self.get_text(path, (255, 0, 0))
 self.image_library[path] = image
 return image
 # returns a text object
 def get_text(self, strs, color=(0, 128, 0)):
 text = self.text_library.get((strs, color))
 if text is None:
 text = self.font.render(strs, True, color)
 self.text_library[(strs, color)] = text
 return text
class Drawer:
 def __init__(self, screen):
 self.screen = screen
 def draw_text(self, text, x, y):
 self.screen.blit(text, (x, y))
 def draw_text_centered(self, text, x, y):
 self.screen.blit(text, (
 x - text.get_width() // 2, y - text.get_height() // 2))
 def draw_image(self, image, x, y):
 self.screen.blit(image, (x, y))
 def draw_rect(self, x, y, width, height, color=(50, 50, 50)):
 pygame.draw.rect(self.screen, color, pygame.Rect(x, y, width, height))
def load(path, games, images):
 with open(path) as fh:
 raw_str = fh.read()
 json_data = json.loads(raw_str)
 for game in json_data['games']:
 games.append(game)
 images.get_image(game['Screenshot'])
 print game
 print games
def main():
 # Flag to allow no args
 use_default_config = False
 default_config_path = None
 # cmd params
 args = sys.argv[1:]
 if not args:
 if use_default_config:
 path = default_config_path
 print 'loading from ', default_config_path
 else:
 print 'Please provide a game list. See readme for details'
 sys.exit(1)
 else:
 path = args[0]
 print 'loading from ', args[0]
 # Load
 # Pygame stuff
 pygame.init()
 clock = pygame.time.Clock()
 screen = pygame.display.set_mode((800, 600))
 # 'Game' loop condition
 done = False
 # Font name, size. Default is used
 font = pygame.font.Font(None, 20)
 d = Drawer(screen)
 lib = Library(font)
 games = []
 load(path, games, lib)
 rect_y_top = 40
 rect_y_bot = 560
 rect_x = 10
 rect_y = rect_y_top
 rect_width = 200
 rect_height = 40
 delta_y = 50
 selected_game_i = 0
 top_of_screen_i = 0
 # 'Game' loop
 while not done:
 # Input
 for event in pygame.event.get():
 if event.type == pygame.QUIT:
 done = True
 if event.type == pygame.KEYDOWN and event.key == pygame.K_ESCAPE:
 done = True
 pressed = pygame.key.get_pressed()
 # Up
 if pressed[pygame.K_UP]:
 if not rect_y <= rect_y_top:
 rect_y -= delta_y
 selected_game_i -= 1
 elif selected_game_i != 0:
 selected_game_i -= 1
 top_of_screen_i -= 1
 # Down
 if pressed[pygame.K_DOWN]:
 if not rect_y >= rect_y_bot - delta_y:
 rect_y += delta_y
 selected_game_i += 1
 elif selected_game_i != len(games) - 1:
 selected_game_i += 1
 top_of_screen_i += 1
 # Main Enter/return
 if pressed[pygame.K_RETURN]:
 exe = games[selected_game_i]['exe']
 print exe
 os.popen(exe)
 # Draw
 # Clear screen
 screen.fill((0, 0, 0))
 # cursor
 d.draw_rect(rect_x, rect_y, rect_width, rect_height)
 # Draw list of games
 x = 100
 y = 60
 for i in xrange(top_of_screen_i, len(games)):
 game = games[i]
 d.draw_text_centered(lib.get_text(game['Name']), x, y)
 y += delta_y
 x = 400
 y = 50
 ydif = 40
 game = games[selected_game_i]
 d.draw_text(lib.get_text('Name: ' + game['Name']), x, y)
 y += ydif
 d.draw_text(lib.get_text('Developer: ' + game['Developer']), x, y)
 y += ydif
 d.draw_text(lib.get_text('Publisher: ' + game['Publisher']), x, y)
 y += ydif
 d.draw_text(lib.get_text('Year: ' + game['Year']), x, y)
 y += ydif
 d.draw_text(lib.get_text('System: ' + game['System']), x, y)
 y += ydif
 d.draw_text(lib.get_text('Genre: ' + game['Genre']), x, y)
 y += ydif
 d.draw_text(lib.get_text('Players: ' + game['Players']), x, y)
 y += ydif
 d.draw_text(lib.get_text('Screenshot: '), x, y)
 y += ydif
 d.draw_image(lib.get_image(game['Screenshot']), x, y)
 y += ydif
 # Updates screen
 pygame.display.flip()
 # fps
 clock.tick(15)
if __name__ == '__main__':
 main()

Example .json config file

Answer 1

Per the style guide, you should rearrange the imports as follows:

import collections
import json
import os
import sys
import pygame

---

Your main seems overcomplicated. For example:

def main():
 # Flag to allow no args
 use_default_config = False
 default_config_path = None
 # cmd params
 args = sys.argv[1:]
 if not args:
 if use_default_config:
 path = default_config_path
 print 'loading from ', default_config_path
 else:
 print 'Please provide a game list. See readme for details'
 sys.exit(1)
 else:
 path = args[0]
 print 'loading from ', args[0]

Could be simplified to:

def main(default_config_path=None): 
 if len(sys.argv) > 1:
 path = sys.argv[1]
 elif default_config_path is not None:
 path = default_config_path
 else:
 print 'Please provide a game list. See readme for details'
 sys.exit(1)
 print 'loading from ', path

(You could also look into argparse, but that's probably overkill here.)

I would also suggest moving the GUI code out into a separate function, so all main does is process the arguments and start the GUI.

---

Rather than the # comments for functions and classes, consider including docstrings, for example:

class Library:
 def __init__(self, font):
 """Holds references to image objects."""
 ...
 def get_image(self, path):
 """Returns the image found at the path given.
 Notes:
 Creates a reference to the image if needed.
 """
 ...
 def get_text(self, strs, color=(0, 128, 0)):
 """Returns a text object."""
 ...

(I like the Google style, but other conventions are available.)

This could also add more explanation on what each class is supposed to do - I'm not sure, for example, why a Library of images has a font (should this really be two classes, ImageLibrary and TextLibrary?)

---

In Python 2.x, your classes must explicitly inherit from object to be "new-style":

class Library(object):
 ...

---

In Pythonic terms,

while True:
 if somecondition:
 break

is better than

flag = True
while flag:
 if somecondition:
 flag = False

as it makes the loop easier to read - you don't need to figure out what else in the loop will run after the flag is set; as soon as the break is reached, the loop ends. In your case, the whole rest of the loop runs once after the user has notionally quit, which may not be what you want.

• Old Badman Grey: Thank you. I actually got the odd args code from the Google's exercises. The loop condition came from the pygame example. It makes sense to have one library class because texts are just rendered images and it allows for a text to be made when an image is not found, so I think that will stay. Why use if default is not None: instead of just if default:
• jonrsharpe: See bullet #2 here: legacy.python.org/dev/peps/pep-0008/…