6
\$\begingroup\$

I'm making a Mario clone in pygame and I have just wrote a function proccesses inputs and acts as a state machine for Mario. Everything is working just as I want it, but I was wondering if you can think of any more efficient way to write this code.

A note that these methods are all a part of a class called mario.

def __init__(self):
    #The inputs currently binded to each of marios actions.
    self.keybinds = {
        'right': K_RIGHT,
        'left' : K_LEFT,
        'jump' : K_z,
        'spd'  : K_LSHIFT
    }

    # The current state mario is in.
    # Valid states: 'stand', 'air', 'death'.
    self.state = 'stand'

    # The current action mario is doing.
    # Valid actions: 'none', 'walk', 'run', 'jump'.
    self.action = 'none'

    # The current direction mario is facing.
    # Valid values: 'r', 'l'.
    self.faceDir = 'r'

    # The direction that the user wants mario to be moving in
    self.des_dir = 'n'

    # The current speed mario is moving at.
    # Valid speeds: 'norm', 'fast'.
    self.spd = 'norm'

    # A boolean based on whether a character instance can jump.
    self.canJump = True

    self.crnt_mdx = 2.7 # The current max x velocity mario can move at.

    self.slow_dx = 2.7 # The max x velocity mario can move at when he is walking.
    self.fast_dx = 4.7 # The max x velocity mario can move at when he is running.


def processInput(self, keys, keyup):
    #keys = a list of all keybord keys currently being pressed.
    #I have imported a module that has a bunch of variables that represent each key.
    #keyup = the current key that was just released, if any.

    if keyup == self.keybinds['jump']:
        self.canJump = True

    if self.state == 'stand' or self.state == 'air':
        if keys[self.keybinds['right']]:
            self.moveHorz('r')
            self.accelerate('r')
            self.set_crnt_acc('r')
            self.faceDir = 'r'
            self.des_dir = 'r'
        elif keys[self.keybinds['left']]:
            self.moveHorz('l')
            self.accelerate('l')
            self.set_crnt_acc('l')
            self.faceDir = 'l'
            self.des_dir = 'l'
        else:
            if self.state == 'stand' and self.horzDir != 'n':
                self.slowDown()
            self.des_dir = 'n'

    if self.state == 'stand':
        if keys[self.keybinds['spd']]:
            self.crnt_mdx = self.fast_dx
        else:
            self.crnt_mdx = self.slow_dx

        if keys[K_z] and self.canJump:
            self.canJump = False
            self.action = 'jump'
    else:
        pass
\$\endgroup\$
2
\$\begingroup\$

Style

The style for naming in Python is as follows.

  • Variables should be in snake_case.
  • Constant variables should be in UPPER_SNAKE_CASE.
  • Functions should also be in snake_case.
  • Classes should be in PascalCase.

You should also replace comments in functions like this:

#keys = a list of all keybord keys currently being pressed.
#I have imported a module that has a bunch of variables that represent each key.
#keyup = the current key that was just released, if any.

To docstrings, like this:

"""Brief description of the function/class.

More detailed description of function/class.

Keyword Arguments:
argument -- Argument description.
...
"""

Design

There's not much about your design that I can nitpick, the only thing I'd really suggest is shortening these two blocks of code:

self.moveHorz('r')
self.accelerate('r')
self.set_crnt_acc('r')
self.faceDir = 'r'
self.des_dir = 'r'

And:

self.moveHorz('l')
self.accelerate('l')
self.set_crnt_acc('l')
self.faceDir = 'l'
self.des_dir = 'l'

To a shortened method, like this:

def change_direction(self, direction):
    if direction == "l" or direction == "r":
        self.moveHorz(direction)
        self.accelerate(direction)
        self.set_crnt_acc(direction)
        self.faceDir = direction
        self.des_dir = direction
    else:
        print("Direction must either be \"l\" or \"r\"")

This is especially useful if you need to repeat this code in many places.

On a side note, you should get rid of the magic values 'l' and 'r' and do something like this:

LEFT = 'l'
RIGHT = 'r'

You could also use integers to represent this as well:

LEFT = 0
RIGHT = 1

Or if you're feeling really adventurous, you can use an Enum, like this:

class Directions(Enum):
    LEFT = 0
    RIGHT = 1
\$\endgroup\$
  • \$\begingroup\$ Yes you are right about the style and I can't believe I didn't think of putting that part into a function :p. \$\endgroup\$ – Robbie Jul 29 '15 at 0:28

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.