patternpythonMinor

Moving the player across an ASCII art "world"

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

theplayerartworldasciimovingacross

Problem

Wow, this one is definitely going to need some improvement.

So, just for fun, I decided to make a program where the player moves across a 2-dimensional ASCII-art map. If the player types something such as down, the on screen representation moves down and "loads" a bit more of the map.

from console import clear

class Move(object):
    up_down = 1
    right_left = 1

class Actions(object):
    action = {
        "down": 1,
        "up": -1,
        "left": -1,
        "right": 1
    }

def set_move_values(action):
    if action == "down":
        Move.up_down += Actions.action["down"]
    elif action == "up":
        Move.up_down += Actions.action["up"]
    elif action == "left":
        Move.right_left += Actions.action["left"]
    elif action == "right":
        Move.right_left += Actions.action["right"]

def render_map(x, y):
    for _ in range(y):
        print(''.join(['.' for _ in range(x + 4)]))
    print(''.join(['.' for _ in range(x + 2)])), '@',
    print(''.join(['.' for _ in range(1)]))
    for _ in range(1):
        print(''.join(['.' for _ in range(x + 4)]))

def run_program():
    while True:
        render_map(Move.right_left, Move.up_down)
        set_move_values(raw_input('> '))
        clear()

if __name__ == "__main__":
    run_program()

All I'm really looking for is, what general stuff can I improve? Also, if possible, I'd like something like right 5 to work to. Note: This is Python 2.7, I just prefer Python3's print function better. Also, console is an ios module, as I wrote this script on ios.

Solution

A few comments:

-
You already did a good job with PEP8. The only error I get back is that it expects two blank lines between functions

-
Regarding PEP257, please add some docstrings and comments to make clear what's the code goal to the potential reader

-
If find the Move class confusing. The up_down and left_right class attributes seem coordinates. I'd prefer to have a class named, for example, Player with x and y instance attributes. Note the differenct between class and instance attributes if you want to have multiple players in the future

-
The translation between actions and movements isn't very effective because you need to hardcode all four cases and use up/down/left/right multiple times

-
If iteration is all what is needed, I recommend using xrange instead of range

-
If just one iteration is needed (range(1)), just drop the loop for better readability

-
Use the multiplication operator to get long strings of the same charater instead of loops ('.' * n)

The code I'd have written would be more or less as follows (more comments below, console library not used):

"""ASCII world game."""

class Player(object):

    """Player object with some actions (just move for now)."""

    def __init__(self):
        """Initialize coordinates."""
        self.x = 1
        self.y = 1

    def move(self, dx, dy):
        """Move player to some position.

        :param dx: How much to move in the x axis
        :type dx: int
        :param dy: Move much to move in the y axis
        :type dy: int

        """
        # Ignore invalid movements
        if (self.x + dx)  ')
        return self.CMD_TO_MOVE.get(command_str, self.NO_MOVE)

class Game(object):

    """Game object to keep state and read command for next turn."""

    def __init__(self):
        """Get all the objects needed to run the game."""
        self.player = Player()
        self.game_map = GameMap()
        self.command = Command()

    def run(self):
        """Render map and read next command from stdin."""
        while True:
            self.game_map.render(self.player)
            self.player.move(*self.command.read())

if __name__ == "__main__":
    game = Game()
    game.run()

What I tried to show:

-
Object oriented design: use the language of the problem space to define a class for each type of object in the problem. In general, when you think about the problem, if it's a name it should be a class, if it's a verb it should be a method

-
Use docstrings for every class and method (use sphinx format for arguments and return values if possible)

-
Make a difference about what is command (word) and a movement (delta in the coordinate system)

-
Use constants instead of harcoded values (X_OFFSET)

Note that not everything should be a class. The command reader could be a function, but I preferred to use class in this case for consistency.

I hope this helps.

Code Snippets

"""ASCII world game."""


class Player(object):

    """Player object with some actions (just move for now)."""

    def __init__(self):
        """Initialize coordinates."""
        self.x = 1
        self.y = 1

    def move(self, dx, dy):
        """Move player to some position.

        :param dx: How much to move in the x axis
        :type dx: int
        :param dy: Move much to move in the y axis
        :type dy: int

        """
        # Ignore invalid movements
        if (self.x + dx) < 1 or (self.y + dy) < 1:
            return

        self.x += dx
        self.y += dy


class GameMap(object):

    """GameMap that shows where the player is."""

    X_OFFSET = 4

    def render(self, player):
        """Render map and player position."""
        def render_empty_row():
            """Render row in which there is no player."""
            print('.' * (player.x + self.X_OFFSET))

        for _ in xrange(player.y):
            render_empty_row()

        print('{} @ {}'
              .format('.' * (player.x - 1),
                      '.' * (self.X_OFFSET / 2)))
        render_empty_row()


class Command(object):

    """Command reader from stdin."""

    # Map text command to player movement
    CMD_TO_MOVE = {
        'u': (0, -1),
        'up': (0, -1),
        'd': (0, 1),
        'down': (0, 1),
        'l': (-1, 0),
        'left': (-1, 0),
        'r': (1, 0),
        'right': (1, 0),
    }

    # Default value for unknown command
    NO_MOVE = (0, 0)

    def read(self):
        """Read command (up, down, left, right) from stdin.

        The command is converted to a movement for the player using the same
        coordinate system.

        :returns: Player movement
        :rtype: tuple(int, int)

        """
        command_str = raw_input('> ')
        return self.CMD_TO_MOVE.get(command_str, self.NO_MOVE)


class Game(object):

    """Game object to keep state and read command for next turn."""

    def __init__(self):
        """Get all the objects needed to run the game."""
        self.player = Player()
        self.game_map = GameMap()
        self.command = Command()

    def run(self):
        """Render map and read next command from stdin."""
        while True:
            self.game_map.render(self.player)
            self.player.move(*self.command.read())

if __name__ == "__main__":
    game = Game()
    game.run()

Context

StackExchange Code Review Q#57630, answer score: 7

Revisions (0)

No revisions yet.