5
\$\begingroup\$

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

\$\endgroup\$
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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: \$\endgroup\$ – Old Badman Grey Oct 6 '14 at 14:27
  • \$\begingroup\$ See bullet #2 here: legacy.python.org/dev/peps/pep-0008/… \$\endgroup\$ – jonrsharpe Oct 6 '14 at 14:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.