Simple console roguelike game

I have written this small game for the "learn python the hard way" book. I ask for some opinions about it and how to improve it.

I know about pygame and others game engines but all the game runs in the terminal. The text file orc.txt, cyclops.txt, etc are ASCII drawings

```
from PIL import Image
import numpy as np
import msvcrt
from time import sleep
from random import choice, randint

previous = 15 #start position tile

def createmap():
"""
The map is sketched in bmp, I make a array
to use it like a tileset
"""
global bmp
bmp = Image.open('sketch.bmp')
bmp_data = bmp.getdata()
bmp_array = np.array(bmp_data)
global map
map = bmp_array.reshape(bmp.size)

def array_to_text_map():
""" Tileset
# = Wall
. = sand
+ = door
e = enemy
D = dragon
$ = gold
@ = the player

numbers as due to the 16 color palette used
in the sketch
"""
text_map = open('map.txt', 'w')
for x in map:
for y in x:
if y == 0:
text_map.write('#')
elif y == 3:
text_map.write('.')
elif y == 8:
text_map.write('+')
elif y == 9:
text_map.write('e')
elif y == 10:
text_map.write('D')
elif y == 12:
text_map.write('$')
elif y == 13:
text_map.write('@')
elif y == 15:
text_map.write(' ')
text_map.write('\n')
text_map.close()

def print_text_map():
text_map = open('map.txt')
for x in range(bmp.size[1]):
print(text_map.readline(), end='')

def move_player_by_key():
wkey = msvcrt.getch
if wkey() == b"\xe0":
key2 = wkey()
if key2 == b"H":
move_in_matrix('up')
elif key2 == b"M":
move_in_matrix('right')
elif key2 == b"K":
move_in_matrix('left')
elif key2 == b"P":
move_in_mat

I'll assume you're fairly new to Python; tell me if you feel this stuff is too basic, and I'll start criticizing you for things that you don't deserve to be criticized for.

State

Ok, so the first thing that made me go "eww" in the code was global. In general, whenever you use global, you're probably doing something wrong. If I was to write your createmap function, it might look something more like this:

def createmap():
    """
    The map is sketched in bmp, I make a array
    to use it like a tileset
    """
    bmp = Image.open('sketch.bmp')
    bmp_data = bmp.getdata()
    bmp_array = np.array(bmp_data)
    map = bmp_array.reshape(bmp.size)
    return bmp, map

Instead of using a global bmp and map variable, I'm returning the non-global bmp and map variables. This means that I have a lot more flexibility in calling the function:

# I can use it to create the old global variables:
global bmp, map
bmp, map = createmap()

# Or some local ones
local_bmp, local_map = createmap()

# But most importantly, I can use it to create multiple sets of them
bmp1, map1 = createmap()
bmp2, map2 = createmap()

The last case is the most important; using your version of createmap(), when I call createmap() for the second time, it overwrites the previous bmp and map variables. This might not seem like a big problem; why would you ever need more than one map? but it makes it much harder to test things when they change global variables.

We can do similar things to your other functions to avoid using global variables. The array_to_text_map function currently uses the map global variable, but we can replace that with a map argument. This way we will be able to convert any map to a text map, not just the one stored in the global map variable.

One downside to this is that it does require more typing; I have to explicitly pass around all of my state. However, one of the core tenets of Python is that explicit is better than implicit, so get used to it ;) I should mention that people often wrap up their program's commonly used variables into classes, though that's probably more complex than you are looking for right now.

Switch Statements

Another thing that makes me feel uncomfortable about the code is when you have long chains of elif statements:

if y == 0:
    text_map.write('#')
elif y == 3:
    text_map.write('.')
elif y == 8:
    text_map.write('+')
elif y == 9:
    text_map.write('e')
elif y == 10:
    text_map.write('D')
elif y == 12:
    text_map.write('

This is ugly, and violates the DRY principle. What you are doing here is looking up a value, based on the value of the variable y. The Pythonic (and sane) way of doing this is to define a dictionary (mapping) from the values of y to the string that should be written to text_map:

TEXT_MAP_CHARS = {
    0: "#",
    3: ".",
    8: "+",
    9: "e",
    10: "D",
    12: "$",
    15: " "
}


We can then look up the correct character, and then write that to the file:

char = TEXT_MAP_CHARS[y]
text_map.write(char)


Which is much neater, and doesn't mix up program data (the mapping from numbers to characters) and program logic (the writing of the character to a file based on the value of y).

With that said, I would praise you for your style. For every problem your code may have semantically, it is formatted very well (hooray for PEP8))
elif y == 13:
     text_map.write('@')
elif y == 15:
     text_map.write(' ')

This is ugly, and violates the DRY principle. What you are doing here is looking up a value, based on the value of the variable y. The Pythonic (and sane) way of doing this is to define a dictionary (mapping) from the values of y to the string that should be written to text_map:

%%CODEBLOCK_3%%

We can then look up the correct character, and then write that to the file:

%%CODEBLOCK_4%%

Which is much neater, and doesn't mix up program data (the mapping from numbers to characters) and program logic (the writing of the character to a file based on the value of y).

With that said, I would praise you for your style. For every problem your code may have semantically, it is formatted very well (hooray for PEP8)

StackExchange Code Review Q#42203, answer score: 7