TextRPG written in python

Question

I started to write a terminal based text RPG in python. Its mostly bug free now but I really want to know how I could improve my code and how to implement things better. I know it's a lot of code so I really want to thank you, if you're patient enough to review it.

I uploaded the code to github. You only need classes.py and main.py to run it.

https://github.com/attax420/TextRPG

Because i have to add at least 3 lines of code to post something, here is the classes.py:

from random import choice 
from termcolor import colored
from time import sleep
import os
from platform import system
if system().lower() == 'windows':
 os.system('color')
if system().lower() == 'linux':
 import readline
# ######### MAP ######### #
class GameMap:
 def __init__(self): 
 self.map = []
 self.map_x = 15
 self.map_y = 15
 self.player_pos_x = int(choice(range(self.map_x -1)))
 self.player_pos_y = int(choice(range(self.map_y -1))) 
 self.boss_x = int(choice(range(self.map_x -1)))
 self.boss_y = int(choice(range(self.map_y -1))) 
 for i in range(self.map_y):
 self.map.append([])
 for j in range(self.map_x):
 self.map[i].append(-1)
 def position_update(self):
 run = True 
 while run == True:
 if self.boss_x != self.player_pos_x or self.boss_y != self.player_pos_y: 
 self.map[self.player_pos_y][self.player_pos_x] = colored(' ', 'green', 'on_white')
 run = False
 else:
 self.player_pos_x = int(choice(range(self.map_x -1)))
 self.player_pos_y = int(choice(range(self.map_y -1))) 
 continue
 
 def print_map(self):
 for i in range(self.map_y):
 for j in range(self.map_x):
 print(self.map[i][j], ' | ', end=' ')
 print()
 print()
 def randomize_map(self):
 for i in range(self.map_y):
 for j in range(self.map_x):
 if self.map[j][i] == -1:
 self.map[j][i] = choice([' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ',
 colored('E', 'green'), colored('E', 'yellow'), colored('E', 'red'), colored('+', 'red'), colored('e', 'yellow')])
 self.map[self.boss_y][self.boss_x] = colored('B', 'magenta')
 
 
 def check_field(self):
 if self.map[self.player_pos_y][self.player_pos_x] == colored('X', 'blue'):
 return 'visited'
 if self.map[self.player_pos_y][self.player_pos_x] == colored('+', 'red'):
 return 'heal'
 if self.map[self.player_pos_y][self.player_pos_x] == ' ':
 return 'empty'
 if self.map[self.player_pos_y][self.player_pos_x] == colored('E', 'green'):
 return 'goblin'
 if self.map[self.player_pos_y][self.player_pos_x] == colored('e', 'yellow'):
 return 'dwarf'
 if self.map[self.player_pos_y][self.player_pos_x] == colored('E', 'yellow'):
 return 'troll'
 if self.map[self.player_pos_y][self.player_pos_x] == colored('E', 'red'):
 return 'ork'
 if self.map[self.player_pos_y][self.player_pos_x] == colored('B', 'magenta'):
 return 'dragon'
 def move_north(self):
 if self.player_pos_y != 0:
 self.map[int(self.player_pos_y)][int(self.player_pos_x)] = colored('X', 'blue')
 self.player_pos_y -= 1
 return True
 else:
 return False
 def move_south(self):
 if self.player_pos_y != self.map_y-1:
 self.map[int(self.player_pos_y)][int(self.player_pos_x)] = colored('X', 'blue')
 self.player_pos_y += 1
 return True
 else:
 return False
 def move_west(self):
 if self.player_pos_x != 0:
 self.map[int(self.player_pos_y)][int(self.player_pos_x)] = colored('X', 'blue')
 self.player_pos_x -= 1
 return True
 else:
 return False
 def move_east(self):
 if self.player_pos_x != self.map_y-1:
 self.map[int(self.player_pos_y)][int(self.player_pos_x)] = colored('X', 'blue')
 self.player_pos_x += 1
 return True
 else:
 return False
# end MAP #
enemies = ['Goblin','Troll','Ork','Dwarf']
# ######### CHARACTERS ######### #
class Character(GameMap):
 def __init__(self):
 GameMap.__init__(self) 
 self.max_hp = None
 self.hp = None
 self.mana = None
 self.attack_damage = None
 self.name = None
 self.spells = None
 self.inventory = None
 self.lvl = None
 self.xp = None
 self.action = None
 self.active_effect = None
 def attack(self, attacker, target): 
 self.action = 'attack'
 if target.hp > 0 and target.action != 'defend':
 target.hp -= int(attacker.attack_damage)
 if target.hp > 0 and target.action == 'defend':
 target.hp -= int(attacker.attack_damage)/2
 def defend(self):
 self.action = 'defend' 
class Player(Character, GameMap):
 def __init__(self): 
 Character.__init__(self) 
 self.name = 'Player'
 self.lvl = 1
 self.attack_damage = 15
 self.max_hp = 100
 self.max_mp = 50
 self.hp = self.max_hp
 self.mp = self.max_mp
 self.xp = 0
 self.spells = ['fireball']
 self.inventory = []
 self.weapon = None
 self.gear = {'Head': 'none',
 'Torso': 'none',
 'Legs': 'none',
 'Feet': 'none'}
 self.dmg_bonus = 0
 
 def update_dmg(self):
 self.attack_damage = self.attack_damage + self.dmg_bonus
 def equip(self, item): 
 if item in weaponlist and item in self.inventory and self.weapon == None:
 self.weapon = item
 self.inventory.remove(item)
 self.dmg_bonus = item.dmg_bonus
 self.update_dmg()
 return True
 return False
 def unequip(self, item):
 if self.weapon != None and item == self.weapon:
 self.dmg_bonus = 0 
 self.inventory.append(item) 
 self.weapon = None
 self.update_dmg()
 return True
 else:
 return False
 def showspells(self):
 return self.spells
 def showxp(self):
 return self.xp, self.lvl
 
 def use_item(self, item):
 if not item.interactive:
 return False, 'not usable'
 if item not in self.inventory:
 return False, 'not available'
 if item in self.inventory:
 item.use(self)
 for i, v in enumerate(self.inventory):
 if v == item:
 self.inventory.pop(i)
 return True, 'success'
 def lvl_up(self):
 lvlup = False 
 if self.lvl == 8:
 self.xp = 0 
 if self.xp >= 100 and self.lvl < 2:
 self.lvl = 2 
 self.max_hp += 20
 self.max_mp += 30
 self.mp = self.max_mp
 self.hp = self.max_hp
 if self.xp >= 100:
 self.xp -= 100
 else:
 self.xp = 0
 self.attack_damage = self.attack_damage * 1.1 
 lvlup = True 
 if self.xp >= 200 and self.lvl < 3:
 self.lvl = 3
 self.max_hp += 40
 self.hp = self.max_hp
 self.max_mp += 60
 self.mp = self.max_mp
 if self.xp >= 200:
 self.xp -= 200
 else:
 self.xp = 0
 self.attack_damage = self.attack_damage * 1.3 
 lvlup = True 
 if self.xp >= 400 and self.lvl < 4:
 self.lvl = 4 
 self.max_hp += 60
 self.max_mp += 80
 self.mp = self.max_mp
 self.hp = self.max_hp
 if self.xp >= 400:
 self.xp -= 400
 else:
 self.xp = 0
 self.attack_damage = self.attack_damage * 1.6 
 lvlup = True 
 if self.xp >= 700 and self.lvl < 5:
 self.lvl = 5 
 self.max_hp += 100
 self.hp = self.max_hp
 self.max_mp += 150
 self.mp = self.max_mp
 if self.xp >= 800:
 self.xp -= 800
 else:
 self.xp = 0
 self.attack_damage = self.attack_damage * 2 
 lvlup = True 
 if self.xp >= 1300 and self.lvl < 6:
 self.lvl = 6 
 self.max_hp += 130
 self.hp = self.max_hp
 self.max_mp += 200
 self.mp = self.max_mp
 if self.xp >= 1600:
 self.xp -= 1600
 else:
 self.xp = 0
 self.attack_damage = self.attack_damage * 2.5 
 lvlup = True 
 if self.xp >= 2000 and self.lvl < 7:
 self.lvl = 7 
 self.max_hp += 180
 self.max_mp += 250
 self.mp = self.max_mp
 self.hp = self.max_hp
 if self.xp >= 2400:
 self.xp -= 2400 
 else:
 self.xp = 0
 self.attack_damage = self.attack_damage * 2.7 
 lvlup = True 
 if self.xp >= 2800 and self.lvl < 8:
 self.lvl = 8 
 self.max_hp += 200
 self.hp = self.max_hp
 self.max_mp += 300
 self.mp = self.max_mp
 self.xp = 0
 self.attack_damage = self.attack_damage * 3 + self.dmg_bonus
 lvlup = True 
 
 if lvlup:
 if self.lvl == 8:
 return('LVL up!!! You are now on LVL 8 which is the maximum LVL!')
 else:
 return ('LVL up!!! You are now LVL ' + str(self.lvl) + '!') 
 else:
 return False
 @staticmethod
 def suicide():
 sleep(2)
 exit('exit by suicide...')
 @staticmethod
 def death():
 sleep(2)
 exit('exit by death...')
 @staticmethod
 def run_away(e):
 mode_choices = ('explore', 'fight')
 mode = choice(mode_choices)
 if mode == 'fight':
 return False
 if mode == 'explore':
 return True
class Enemy(Character):
 def __init__(self):
 Character.__init__(self)
 self.action = None 
 self.xp_bonus = 0
 def showhp(self):
 return self.name, self.hp
class EnemyGoblin(Enemy):
 def __init__(self, p):
 Enemy.__init__(self) 
 self.xp_bonus = 10
 self.name = 'Goblin'
 self.hp = int(p.max_hp)/2
 self.hp = self.hp.__round__(2) 
 self.attack_damage = int(p.hp)/10
 choices = [healthpotion,manapotion,xppotion]
 self.inventory = [choice(choices)]
class EnemyDwarf(Enemy):
 def __init__(self, p):
 Enemy.__init__(self)
 self.xp_bonus = 35
 self.name = 'Dwarf'
 self.hp = int(p.max_hp)/1.5
 self.hp = self.hp.__round__(2)
 self.attack_damage = int(p.hp)/9
 choices = (healthpotion, manapotion, xppotion, healthpotion, manapotion, xppotion, healthpotion, manapotion, xppotion, sword_bronze)
 self.inventory = [choice(choices)]
class EnemyTroll(Enemy):
 def __init__(self, p):
 Enemy.__init__(self) 
 self.xp_bonus = 25
 self.name = 'Troll'
 self.hp = int(p.max_hp)/1.5
 self.hp = self.hp.__round__(2)
 self.attack_damage = int(p.hp)/8
 choices = (healthpotion, manapotion, xppotion, healthpotion, manapotion, xppotion, healthpotion, manapotion, xppotion, sword_bronze)
 self.inventory = [choice(choices)]
class EnemyOrk(Enemy): 
 def __init__(self, p):
 Enemy.__init__(self) 
 self.xp_bonus = 50
 self.name = 'Ork'
 self.hp = int(p.max_hp)/1
 self.hp = self.hp.__round__(2)
 self.attack_damage = int(p.hp)/5
 choices = (healthpotion, manapotion, xppotion, healthpotion, manapotion, xppotion, sword_bronze, sword_bronze, sword_bronze, sword_steel)
 self.inventory = [choice(choices)]
class EnemyOrkGeneral(Enemy):
 def __init__(self, p):
 Enemy.__init__(self)
 self.xp_bonus = 75
 self.name = 'Ork General'
 self.hp = int(p.max_hp)*1.3
 self.hp = self.hp.__round__(2)
 self.attack_damage = int(p.hp)/3
 choices = (healthpotion, manapotion, xppotion, healthpotion, manapotion, xppotion, sword_bronze, sword_bronze, sword_steel, sword_steel)
 self.inventory = [choice(choices)]
class EnemyDragon(Enemy):
 def __init__(self, p):
 Enemy.__init__(self)
 self.mp = 999999
 self.xp_bonus = 800
 self.name = 'Dragon'
 self.hp = 2000
 self.inventory = [sword_diamond, manapotion, manapotion, healthpotion]
 self.attack_damage = 30
 self.spells = ['fireball']
 
# end CHARACTERS #
# ######### ITEMS ######### #
class Item:
 def __init__(self):
 self.name = None
 self.worth = 0
 self.interactive = True
 self.can_use_in_fight = True
 self.consumable = True
 self.weight = 0
class Potion(Item):
 def __init__(self):
 Item.__init__(self)
 self.weight = 0.5
class PotionHP(Potion):
 def __init__(self):
 Potion.__init__(self)
 self.name = 'HP Potion'
 self.hp_bonus = 50
 def use(self, p):
 p.hp = p.hp + self.hp_bonus
 if p.hp >= p.max_hp:
 p.hp = p.max_hp
 return self.hp_bonus
class PotionXP(Potion):
 def __init__(self):
 Potion.__init__(self)
 self.name = 'XP Potion'
 self.xp_bonus = 150
 def use(self, p):
 p.xp = p.xp + self.xp_bonus
 p.lvl_up
 return self.xp_bonus
class PotionMP(Potion):
 def __init__(self):
 Potion.__init__(self)
 self.name = 'MP Potion'
 self.mp_bonus = 50
 def use(self, p):
 p.mp = p.mp + self.mp_bonus
 if p.mp >= p.max_mp:
 p.mp = p.max_mp
 return self.mp_bonus
class Weapon(Item):
 dmg_bonus = None
 pass
class WeaponBronzeSword(Weapon):
 def __init__(self):
 Weapon.__init__(self)
 self.name = 'Bronze Sword'
 self.dmg_bonus = 10
class WeaponSteelSword():
 def __init__(self):
 Weapon.__init__(self)
 self.name = 'Steel Sword'
 self.dmg_bonus = 40
class WeaponDiamondSword():
 def __init__(self):
 Weapon.__init__(self)
 self.name = 'Diamond Sword'
 self.dmg_bonus = 160
class Armor(Item):
 pass
# end ITEMS #
# ######### SPELLS ######### #
class Spell():
 def __init__(self): 
 self.name = None 
 self.effect = None
 self.dmg = None
 self.mana_usage = None
 self.effect_dmg = None
 def cast(self, attacker, target):
 target.hp -= self.dmg
 attacker.mp -= self.mana_usage
 target.active_effect = self.effect
 return self.dmg, self.mana_usage, self.effect
class SpellFireball(Spell):
 def __init__(self): 
 Spell.__init__(self)
 self.name = 'Fireball'
 self.dmg = p.lvl*50
 self.mana_usage = p.lvl*20/p.lvl
 self.effect_dmg = 5
 self.effect = 'fire'
 
class SpellBlizzard(Spell):
 def __init__(self):
 Spell.__init__(self)
 self.name = 'Blizzard'
 self.dmg = p.lvl*50
 self.mana_usage = p.lvl*30/p.lvl
 self.effect_dmg = 0
 self.effect = 'ice'
# end SPELLS #
# ######### Objects ######### #
p = Player()
healthpotion = PotionHP()
manapotion = PotionMP()
xppotion = PotionXP()
spellfireball = SpellFireball()
spellblizzard = SpellBlizzard()
sword_bronze = WeaponBronzeSword()
sword_steel = WeaponSteelSword()
sword_diamond = WeaponDiamondSword()
# ######### ITEMLISTS + SPELLLIST ######### #
weaponlist = [sword_bronze, sword_steel, sword_diamond]
potionlist = [healthpotion, manapotion, xppotion]
spelllist = [spellfireball, spellblizzard]
# end ITEMLISTS + SPELLLIST #
# end OBJECTS #

• Reinderien: Please also include main.py verbatim in this post.

Answer 1

First, congratulations on writing a game that runs, and hangs together without crashing! (It's harder than you might think.)

Here are some ideas on how to make it better:

Packages

If you're going to use external packages, provide some standard mechanism to install them. The most obvious would be a requirements.txt file generated by pip. If not that, you could try to be hipster and use one of the (many, many) competitors (Google is not your friend, here. Because there are so damn many...)

Classes

I don't think you've quite mastered classes, yet. You've done a lot of things mostly right, but you've done a bunch of things wrong, as well. So this is an area where you need to focus your effort.

Inheritance

The problem with how OOP and classes get taught is that most courses and most instructors focus on inheritance as a characteristic of OO, and teach the "Java model" of big tall hierarchies.

The reality of modern OO thought is that inheritance is not a characteristic, but rather it is a tool. That is, you should default to not using inheritance until and unless you make an explicit, conscious decision that inheritance provides some specific benefit to you in a particular instance.

There are a variety of benefits that you might get from inheritance. Sometimes, the benefits you get will depend on the implementation language. (For example, Python generally doesn't care about memory layout. So while "memory layout" might make sense as a benefit in Go or C++ or D, it's kind of "meh" in Python.)

For your code, the obvious benefits are virtual behavior and common member names.

That means providing a .attack() method that does different things for different classes. And it means providing a common attribute interface, like .current_hp and .damage attributes.

You have a class GameMap. That's fine. The orthography is good. Some of the fields make sense (more below). GameMap seems like a good, solid class.

Then you have class Character(GameMap). Why do you think that a character derives from a game map? In what way does an IS-A relationship work here?

Then you have class Player(Character, GameMap). Why would you implement multiple-inheritance, especially since Character already inherits from GameMap?

As far as I can see, this entire subtree of your class hierarchy is wrong. I don't see much reason for any class to inherit from GameMap.

You have a class Enemy(Character) that makes just a little bit of sense. You add one method, .showhp(), and a .xp_bonus attribute. Your assignment to self.action appears redundant.

You have classes for each enemy type (Goblin, Dwarf, etc.). This appears to be a sensible basic approach.

Encapsulation

Encapsulation is the key benefit to OO programming. Even if you are not using inheritance, or virtual dispatch, or anything else, being able to bundle up a bunch of related data and functions that will operate on that data is a big win.

Whenever you write code that "checks" the type of an object, you are violating encapsulation. Don't do that. Code like

if e.name == 'Dragon':

is a total lose. You're breaking encapsulation and making your code harder to understand and harder to maintain.

What you want to do is describe the behavior in abstract terms, and either write a separate driver function to handle it, or write a separate virtual method to handle it.

You might write code like this to implement a special driver:

if enemy.is_boss:
 boss_fight()
else:
 basic_fight()

Or you might keep a single "fight" driver, and write code like:

if mode == 'fight':
 fight_intro()
 while mode == 'fight':
 show_fight_stats()
 enemy.maybe_taunts()
 enemy.turn()
 player.turn()
 enemy.update()
 player.update()

The point, though, is that you should strive to push everything down into the appropriate "capsule."

Tell, don't Ask

This is one of those OO aphorisms that gets thrown around a lot. And you need to keep it in your head pretty much all the time when you're thinking about classes.

Here's an example:

# ######### ENEMY DEATH ######### # 
if e.hp <= 0:
 print('You killed the '+str(e.name)+' and got '+str(e.xp_bonus)+'XP!')

This is your fight driver asking the enemy how many hit points it has, then making a decision about whether the enemy is dead, then asking the enemy for its name and EXP.

What should you do differently? Tell, don't ask!

if e.is_dead():
 e.award_exp(player)

Of course, it's also possible that the check-for-dead-and-award-exp could be moved into the player.attack(enemy) method. That's up to you.

The point is that it is not the business of the fight driver function to manage either the player or the enemy. Tell those classes what to do, and let them do it for you!

Control Flow

There's a lot of code like this:

if condition1:
 do_stuff1()
if condition2:
 do_stuff2()
...

In many cases, those if statements are mutually exclusive: when condition1 is true, condition2 cannot possibly be true.

You should either use if ... elif ... else statements, or use a dictionary of string->function mappings to dispatch to the appropriate function.

Function level of detail

Try to keep your functions at a single level of detail. If you are writing a function that dispatches to other functions, then have all the cases dispatch to another function. Don't mix and match dispatch to functions with code in-line.

Also, try to differentiate between "control flow" functions and "procedure" functions. Instead of a "giant wall of code" with control flow and long blocks of instructions, put your control flow into a single function and have it dispatch to procedures for each control flow case.

Unify your code and data

Part of OO is encapsulating code and data together. This can have unexpected implications.

You have a function called randomize_map (a method of GameMap) that sets the individual map cells to empty, or strings like colored('E', 'green').

That's a magic number. (Magic string, actually, but still...)

There's another method, check_field, that maps those magic strings to class names:

if self.map[...][...] == color('E', 'green'):
 return 'goblin'

It makes a lot more sense to me to move the class into the discussion. Instead of writing:

self.map[j][i] = choice([
 ' ', ' ', ' ', ' ', ' ', ' ', ' ', 
 ' ', ' ', ' ', colored('E', 'green'), 
 colored('E', 'yellow'), colored('E', 'red'),
 colored('+', 'red'), colored('e', 'yellow')])

Why not write:

FIELD_LIST = ([None] * 10
 + [EnemyGoblin, EnemyTroll, EnemyOrk, 
 PotionHP, EnemyDwarf])

And then:

occupant = choice(FIELD_LIST)
self.map[j][i] = ' ' if occupant is None else occupant.display_string

This has the effect of moving the class-specific data (the colored display string) into the class and out of the GameMap.

First-class objects

In Python, classes, functions, and instances of classes are all first class objects. That means you can pass them around and use them in your code.

Instead of returning a string like:

return 'goblin'

You could do:

return EnemyGoblin.name

But you could also just do:

return EnemyGoblin

(Returning the class directly, instead of a string with a description of the class in it.)

Game Play

I've played Zork, so I hate your game. I don't want to type "go north". I want to type "n". I certainly don't want to type "cast spell" and then "fireball". I want to type "cast fireball" or even just "fireball" or "fb".

You really need to work on adding some abbreviations support to the game. You might just build a map of common abbreviations and expand them to the full commands -- that's a fairly easy way to "bolt on" this kind of functionality.

But you could also write a full-on command parser. It depends on why you are writing the game. If it's for a class, you might be able to add the parser in a later part of the course, or as an extra credit assignment.

• Alexander Müller: First of all: Thank you very much for your time and patience! I started this project to actually have a use case for the stuff I learn in different python video courses. I just started with programming 2-3 months ago and really appreciate your in depth analysis of my code and the errors I made. It will certainly take a few days until my slow brain processed everything you wrote, but i will do my best to improve my code based on your recommendations. I really thought I know how classes work, but you showed me that I certainly need to learn more about them.

Answer 2

First of all, How do expect someone to read your code? Neither type hints nor doc-strings.

Review

• What would a Player or a Enemy inherit from a GameMap?
• I would prefer to implement a .attack_player(player: Player) method as it is more flexible.
• In Player.lvl_up(), you should try to form a formula.
• We can use dicts to simplify stuff For example -

enemy_types = {
 'Goblin': {
 'name': 'Goblin',
 'xp_bonus': 10,
 'hp': ...
 },
 ...
}
class Enemy:
 def __init__(self, type: str) -> None:
 self.__dict__.update(enemy_types[type])
 def attack_player(self, player: Player) -> None:
 '''Attack the given player, returns None.'''
 player.hp -= 10

def check_field():
 field_types = {
 colored('X', 'blue'): 'visited',
 ...
 }
 return field_types[self.map[self.player_pos_y][self.player_pos_x]]

• We can create a list of pre-build maps and chose a random one from it, it will allow to have more appropriate maps.

• Assume I input 'go north', would you want to check it with all the commands? It is only one of them. In such cases, we use an elif.

• Do not do unnecessary conversions, like here - int(choice(range(self.map_y -1)))

• What is this p everywhere? Give it a more readable name such as player.

---

In general, you have completely overlooked readability and performance. All the best in improving your code!