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_matrix('down')
else:
exit()
def position():
ver = np.where(map == 13)[0][0]
hor = np.where(map == 13)[1][0]
return ver, hor
def move_in_matrix(direction):
ver, hor = position()
try:
if direction == 'up':
analize(ver-1 , hor)
elif direction == 'right':
analize(ver , hor+1)
elif direction == 'left':
analize(ver , hor-1)
elif direction == 'down':
analize(ver+1 , hor)
except IndexError:
console.append('Not that way')
def analize(verm, horm):
"""Take decision for each tile
verm and horm = next position
ver and hor = actual position
previous = keeps tile number for later
"""
global previous
ver, hor = position()
def restore(ver, hor, previous):
"""Restore color of the leaved tile"""
map[ver, hor] = previous
if map[verm, horm] == 0:
pass
elif map[verm, horm] == 3:
restore(ver, hor, previous)
previous = map[verm, horm]
map[verm, horm] = 13
elif map[verm, horm] == 8:
print('Open the door (yes/no)?', end='')
answer = input(' ')
if door(answer):
restore(verm, horm, 15)
console.append('The door was opened')
console.append(doors_dict[(verm, horm)]) #show the description
#attached to the door
elif map[verm, horm] == 9:
print('Do you want to fight against the monster (yes/no)?', end='')
answer = input(' ')
result = fight(answer)
if result and randint(0,1) == 1:
restore(verm, horm, 12)
elif result:
restore(verm, horm, 15)
elif map[verm, horm] == 10:
print('the beast seems asleep, you want to wake her?(yes/no)?', end='')
answer = input(' ')
result = fight(answer, dragon=True)
if result:
win()
elif map[verm, horm] == 12:
print("You want to grab the items in the floor (yes/no)?", end='')
if input(' ') == 'yes':
gold()
restore(verm, horm, 15)
elif map[verm, horm] == 15:
restore(ver, hor, previous)
previous = map[verm, horm]
map[verm, horm] = 13
def door(answer):
if answer == 'yes':
return True
elif answer == 'no':
return False
else:
console.append('Invalid command')
def identify_doors():
"""Hardcode: Identify each door and zip() it with
a description stored previously
"""
doors_array = np.where(map == 8)
doors = zip(doors_array[0], doors_array[1])
dict_doors = {}
narrate = open('narrate.txt')
for x in doors:
dict_doors[x] = narrate.readline()
return dict_doors
def fight(answer, dragon=False):
if answer == 'yes':
if dragon == False:
monster = choice([Orc(), Goblin(), Cyclops()])
else:
monster = Dragon()
monster.show_appearance()
print('Fighting against', monster.name)
sleep(2)
while True:
monster.show_appearance()
print(monster.name, 'is attacking you', end = '')
answer = input('fight (1) or defend (2)? ')
if answer == '1':
player.defense = 1 + player.armor
monster.life = monster.life - player.damage
elif answer == '2':
player.defense = 1.5 + player.armor
else:
print('Invalid command')
continue
print(monster.name, 'counterattack!!')
player.life = player.life - (monster.damage / player.defense)
print(monster.life, 'remaining', monster.name, 'life')
print(player.life, 'remaining Player life')
if player.life <= 0:
print('The End')
exit()
if monster.life <= 0:
print('\n' * 5)
print('Enemy defeated')
print('You earn', monster.gold, 'gold coins')
player.gold += monster.gold
break
sleep(3)
return True
else:
return False
def moredamge(val):
player.damage += val
def morearmor(val):
player.armor += val
golds = {'Fire sword': (moredamge, 20),
'Oak shield': (morearmor, 0.1),
'Anhilation sword': (moredamge, 40),
'Iron shield': (morearmor, 0.2),
'Siege shield': (morearmor, 0.5)
}
def gold():
bunch = randint(10, 200)
print('You have found', bunch, 'gold coins')
player.gold += bunch
sleep(1)
print('Is there anything else?')
if randint(0, 10) > 7:
obtained = choice(list(golds))
print('You have get', obtained)
golds[obtained][0](golds[obtained][1]) #access to key: (function, quantity)
del golds[obtained]
else:
print('Oooohh.. nothing else')
sleep(2)
def win():
print('You have win the game')
print('You get', player.gold, 'gold coins!!')
exit()
class Orc():
def __init__(self):
self.name = 'Orc'
self.life = 100
self.damage = 10
self.defense = 1
self.gold = 100
self.appearance = open('orc.txt')
def show_appearance(self):
print(self.appearance.read())
class Player(Orc):
def __init__(self):
self.life = 1000
self.damage = 20
self.gold = 0
self.appearance = open('knight.txt')
self.armor = 0
class Goblin(Orc):
def __init__(self):
self.name = 'Goblin'
self.life = 50
self.damage = 10
self.gold = 50
self.appearance = open('goblin.txt')
class Dragon(Orc):
def __init__(self):
self.name = 'Dragon'
self.life = 400
self.damage = 40
self.gold = 2000
self.appearance = open('dragon.txt')
class Cyclops(Orc):
def __init__(self):
self.name = 'Cyclops'
self.life = 150
self.damage = 20
self.gold = 120
self.appearance = open('cyclops.txt')
def presentation():
print('\n'*2)
player.show_appearance()
print('Welcome hero, are you ready for fight?')
input()
if __name__ == "__main__":
player = Player()
presentation()
console = []
createmap()
array_to_text_map()
doors_dict = identify_doors()
print_text_map()
while True:
move_player_by_key()
array_to_text_map()
print_text_map()
if console:
for x in console:
print(x)
else:
print()
console = []
1 Answer 1
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('$')
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:
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)