Pygame menu to launch games

Question

I made a menu to display a list of games and their info so that they can be uniformly launched from an arcade cabinet like set up. I was asked to do it in Python so I learned python using this and this. I have a Java background but no prior Python experience.

I have a working product but I realize that my code doesn't quite look like other Python I've seen. Mainly there is no main and there is no use of self. I just used the structure of the pygame tutorial. I'm looking for feedback on code correctness, best practices and design pattern usage.

If you propose major changes, what are the tools to help make them in Python? I'm used to Eclipse's refactoring tools for Java. Right now, I'm switching between Notepad++ and Sublime for Python. Other than find and replace, I don't know how to error free rename variables. How do I extract a method?

This is how it looks like:

enter image description here

import pygame
import json
import collections
import os
pygame.init()
screen = pygame.display.set_mode((800, 600))
clock = pygame.time.Clock()
done = False
_image_library = {}
def create_image(path):
 global _image_library
 image = _image_library.get(path)
 if image == None:
 image = pygame.image.load(path)
 _image_library[path] = image
def get_image(path):
 global _image_library
 image = _image_library.get(path)
 if image == None:
 image = pygame.image.load(path)
 _image_library[path] = image
 return image
def draw_image(image, x, y):
 screen.blit(image, (x, y))
#font = pygame.font.SysFont("comicsansms", 72)
#text = font.render("Hello, World", True, (0, 128, 0))
size = 20
font = pygame.font.Font(None, size)
_text_library = {}
def creat_text(text, color):
 global _text_library
 color = (0, 128, 0)
 _text_library[text] = font.render(text, True, color)
def get_text(text):
 global _text_library
 color = (0, 128, 0)
 s = _text_library.get(text)
 if s == None:
 s = font.render(text, True, color)
 _text_library[text] = s
 return s
def draw_text(text, x, y):
 screen.blit(text, (x, y))
def draw_text_centered(text, x, y):
 screen.blit(text,
 (x - text.get_width() // 2, y - text.get_height() // 2))
def draw_rect(x, y, width, height, color):
 pygame.draw.rect(screen, color, pygame.Rect(x, y, width, height))
_games = collections.OrderedDict()
_DATA_I = 0
_X_I = 1
_Y_I = 2
_Y_TOP = 60
_DELTA_Y = 50
def load():
 global _games
 global _DATA_I
 global _X_I 
 global _Y_I 
 global _Y_TOP
 global _DELTA_Y
 f = open('./games.json')
 s = f.read()
 f.close()
 j = json.loads(s)
 x = 100
 y = _Y_TOP
 y_space = _DELTA_Y
 for game in j['games']:
 _games[game['Name']] = [game, x, y]
 y += y_space
 #Jet Pac is used as a generic key. The key used does not matter as long as it has all the fields to be displayed
 lables = _games['Jet Pac'][_DATA_I].keys()
 for text in lables:
 creat_text(text + ': ', None)
 print 'debug:'
 print _games
 for game in _games:
 print '\t' + game
 for dict_key in _games[game][_DATA_I].keys():
 print '\t\t' + dict_key + ': ' + _games[game][_DATA_I][dict_key]
 creat_text(_games[game][_DATA_I][dict_key], None)
 print _games['Jet Pac'][_DATA_I]['Name']
 print _text_library.keys()
 #end load
RECT_Y_TOP = 40
rect_x = 10
rect_y = RECT_Y_TOP
rect_width = 200
rect_height = 40
color = (50, 50, 50)
load()
selected = _games.items()[0]
print '\n\n', selected
selected_i = 0
selected_data = _games.items()[selected_i][4][0]
print '\n', selected_data
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_i -= 1
 selected_data = _games.items()[selected_i][5][0]
 elif selected_i != 0:
 selected_i -= 1
 selected_data = _games.items()[selected_i][6][0]
 for game in _games:
 print game
 _games[game][_Y_I] -= _DELTA_Y
 #down 
 if pressed[pygame.K_DOWN]:
 if selected_i != len(_games.items())-1:
 rect_y += _DELTA_Y
 selected_i += 1
 selected_data = _games.items()[selected_i][7][0]
 #enter
 if pressed[pygame.K_RETURN]:
 print selected_data['exe']
 os.popen(selected_data['exe'])
 #begin draw
 screen.fill((0, 0, 0))
 #cursor
 draw_rect(rect_x, rect_y, rect_width, rect_height, color)
 #list games
 for game_name in _games:
 #if _games[game_name][_Y_I] >= _Y_TOP and _games[game_name][_Y_I] < RECT_Y_BOT:
 draw_text_centered(_text_library[game_name], _games[game_name][_X_I], _games[game_name][_Y_I])
 #list selected details
 x = 400
 y = 50
 ydif = 40
 draw_text(get_text('Name: ' + selected_data['Name']), x, y)
 y += ydif
 draw_text(get_text('Developer: ' + selected_data['Developer']), x, y)
 y += ydif
 draw_text(get_text('Publisher: ' + selected_data['Publisher']), x, y)
 y += ydif
 draw_text(get_text('Year: ' + selected_data['Year']), x, y)
 y += ydif
 draw_text(get_text('System: ' + selected_data['System']), x, y)
 y += ydif
 draw_text(get_text('Genre: ' + selected_data['Genre']), x, y)
 y += ydif
 draw_text(get_text('Players: ' + selected_data['Players']), x, y)
 y += ydif
 draw_text(get_text('Screenshot: '), x, y)
 y += ydif
 draw_image(get_image(selected_data['Screenshot']), x, y)
 pygame.display.flip()
 #fps
 clock.tick(15)

The game list is loaded from a JSON file here.

Answer 1

Global variables

Global variables are bad. Avoid them as much as possible.

One way to avoid them is by making them method parameters. But I see that it would make create_image and get_image inconvenient if you had to add _image_library in every call. A good solution can be to create an ImageLibrary class, which can contain _image_library, hitting two birds with one stone: 1. no more global variable; 2. still convenient

Similarly, you could create a TextLibrary.

For more on classes, see the Python tutorial (version 3), or version 2 if that's a requirement of pygame.

Code organization

As you noticed yourself, you don't have a main method. It's not a requirement, but a common place to put the glue-code that configures the main objects and launches the main activity of the program.

It's not good to have any code in the global namespace, except some global constants. This way the module cannot be imported and thus reused by other modules. So it's better to move the main glue code inside a main function, and add this boilerplate to call it:

if __name__ == '__main__':
 main()

This condition will only be true when running the script in a shell, it won't be true when importing as a module.

File I/O

Instead of this:

f = open('./games.json')
s = f.read()
f.close()

The recommended pattern is this:

with open('./games.json') as fh:
 content = fh.read()

Notice that you don't need to close the file handle, it gets closed automatically when execution leaves the with block.

Also, s was a poor name, it's better to use meaningful names.

IDE and tools for Python

I recommend PyCharm, by the same company that makes IntelliJ. It's awesome, and would help your learning if you follow its warnings about your code. It would certainly help with refactoring operations.

PEP8 is the official coding style guide for Python, and there is a command line tools named pep8 (install with pip install pep8) which can check your entire project for violations.

Another tool is pyflakes (install with pip install pyflakes), which can detect some bad practices and potential bugs.

• Old Badman Grey: Thank you. I posted a follow up (codereview.stackexchange.com/questions/64831/…) I got a student license of pycharm. I'm not thrilled with it but its better than pydev (eventhough I love eclipse for java). Still, sublime with ctrl shift v is much better at pasting and pydev reformats like I'd expect from eclipse.