# Moving the player across an ASCII art “world”

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.

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

• 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) < 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):

# 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)

"""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)

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.