Text-based RPG using OOP

Question

I started this project after reading Chapter 3 of Python 3 Object-Oriented Programming by Dusty Phillips. At first I just wanted to create RPG-based objects to practice inheritance, but now I am more interested with seeing how far I can take this RPG project. Just for reference, I'll say that I am a beginner programming student, and while I am decent at solving problems with code, I am completely new to building programs.

######## Python Text Based RPG adventure #########
import time
import random
class Character(object):
 def __init__(self, name='', hp=0, attack=0, **kwargs):
 super().__init__(**kwargs)
 self.name = name
 self.hp = hp
 self.attack = attack
 def Attack(self, other):
 print(self.name + " is attacking " + other.name + "!!!!")
 other.hp -= self.attack
 print('you dealt ' + str(self.attack) + ' damage!!')
class Enemy(Character):
 def __init__(self, **kwargs):
 super().__init__(**kwargs)
class SadSad(Enemy):
 '''This is me considering if I should make
 seperate classes for each enemy instance, or
 maybe make a function that generates random Enemies'''
 def __init__(self, **kwargs):
 super().__init__(**kwargs)
 self.name = "SadSad"
 self.hp = 5
 self.attack = 1
class Player(Character):
 def __init__(self, weapons=None, armor=None, inventory=[], **kwargs):
 super().__init__(**kwargs)
 self.weapons = weapons
 self.armor = armor
 self.inventory = inventory
 def inspect(self):
 print(self.name + ' has ' + \
 str(self.hp) + ' HP and ' + \
 str(self.attack) + ' Attack.')
 @staticmethod
 def inspect_other(other):
 print(other.name + ' has ' + \
 str(other.hp) + ' HP and ' + \
 str(other.attack) + ' Attack.')
 def collect(self, item):
 '''puts any existing item into the Player's inventory'''
 if item.state == None:
 self.inventory += [item]
 else:
 print("You cannot collect that.")
 def equip_gear(self, item):
 if isinstance(item, Weapon) \
 and self.weapons == None and item in self.inventory:
 self.weapons = item
 item.state = True
 self.attack += item.attack
 self.inventory.remove(item)
 elif isinstance(item, Armor) and self.armor == None:
 self.armor = item
 item.state = True
 self.armor += item.armor
 self.inventory.remove(item)
 else:
 print("that cannot be equipped")
 def unequip(self, item):
 if item == self.weapons:
 self.weapons = None
 self.inventory.append(item)
 elif item == self.armor:
 self.armor = None
 self.inventory.append(item)
class Room(object):
 '''What class atributes should a room hold?
 a description of the room, a list of characters,
 list of items, some kind of ID noting its place in
 the game world'''
 def __init__(self, ID = None, description = '',\
 characters = [], items = [], **kwargs):
 super().__init__(**kwargs)
 self.ID = ID
 self.description = description
 self.characters = characters
 self.items = items
 def __str__(self):
 print(self.description)
class Item(object):
 def __init__(self, name='', state=None, **kwargs):
 super().__init__(**kwargs)
 self.name = name
 self.state = state
class Weapon(Item):
 def __init__(self, attack=0, **kwargs):
 super().__init__(**kwargs)
 self.attack = attack
class Armor(Item):
 def __init__(self, armor=0, **kwargs):
 super().__init__(**kwargs)
 self.armor = armor
fop = Weapon(name='Fists of Punching', attack=5) 
sword = Weapon(name="sword", attack = 6)
goblin = Enemy(name="goblin", hp=50, attack=5)
SadSad = SadSad()
r = Player(name="jeremyodell", hp=999, weapons=fop)
Room0 = Room(ID="Room0", description="You are literally in Hell. There is fire all \
around. You are scared and tired. There is a weapon on the ground. \
You see a goblin and a sword.", characters = [goblin],\
 items = [sword])
Room1 = Room(ID="Room1", description="You have escaped from Lucifer, and now find\
 yourself surrounded by two SadSads. Also there are a pair of fists on the ground.",\
 characters = [SadSad, SadSad], items = [sword, fop])
current_room = Room0
current_char = r
game_map = {"Room0": {"north":"Room1", "east":"Room2", "west":"Room3"},
 "Room1": {"south":"Room0", "east":"Room4", "west":"Room5"}}
def split(string):
 return string.split()
room_actions = {"get": current_char.collect, "talk":0,"menu":0, "fight":0}
def room_logic():
 global current_room
 action = input("What will " + r.name + " do? (type 'look' to look,\
 type 'get sword' to get the sword, and type 'move north' to move to a different room)\n")
 if action == "look":
 print(current_room.description + '\n')
 room_logic()
 if split(action)[0] == "get":
 if eval(split(action)[1]) in current_room.items:
 room_actions["get"](eval(split(action)[1]))
 print("You picked up the things!")
 room_logic()
 else:
 print("you can't get " + split
 (action)[1])
 room_logic()
 if split(action)[0] == "move":
 if split(action)[1] in game_map[current_room.ID]:
 current_room = eval(game_map[current_room.ID][split(action)[1]])
 print("You have moved " + split(action)[1] + "!")
 room_logic()
 else:
 print("What does that mean?")
 room_logic()
room_logic()

So far, as far as actual playability goes, all the player can input are things like: "move north", "get sword", and "look".

My biggest concerns about this program are: The current_room and current_char variables. My thinking was that this would be useful for having the program actually know where it is, rather than just using a lot of different text options. I am worried that I will end up writing myself into a wall using this method.

I am also unsure about the technique that I thought of for the game_map. Is a dictionary with dictionaries in it a good way to over-complicate code? Same with my room_actions variable.

Finally the thing that I am most confused about is the actual use and implementation of game logic. My idea is to make room_logic, a function that is running all the time (except maybe during combat, which I have yet to implement) that will handle the player's inputs, no matter what room he is in. I read elsewhere on Stack Overflow that having long functions that handle many different actions is rarely a good idea, but this is just a prototype.

Answer 1

You have bugs, bugs everywhere! inventory=[] looks innocent, nothing possibly can go wrong here! But it's actually very different to how I think you think it works.
To show this bug, I'll make a function, that takes some data, mutates it a little and returns it. If we pass nothing to it, it should return just the mutation. So to keep it simple we append 'test' to it. So test() -> ['test'] and test(['this', 'is', 'a']) -> ['this', 'is', 'a', 'test']. But I'll show you the bug.

>>> def test(data=[]):
 data.append('test')
 return data
>>> test()
['test']
>>> test(['this', 'is', 'a'])
['this', 'is', 'a', 'test']
>>> test() == test()
True
>>> test() is test()
True
>>> test()
['test', 'test', 'test', 'test', 'test', 'test']

Yes, you can test it yourself. That is not what you want. And you should be able to see every time we call test() it adds one 'test' to the list. This is as all function calls share the same instance of the list. Which is why test() is test() is True.

To fix this you should change all default parameters to None. And overwrite it with a new instance of a list. And so I'd take a leap out of JavaScripts book and use or to the overwrite the list. Allowing for:

>>> def test(data=None):
 data = data or []
 data.append('test')
 return data
>>> test()
['test']
>>> test()
['test']
>>> test(['this', 'is', 'a'])
['this', 'is', 'a', 'test']

Or if you don't like that a more verbose, but probably a better function:

>>> def verb_test(data=None):
 if data is None:
 data = []
 data.append('test')
 return data
>>> l = []
>>> test(l)
['test']
>>> l
[]
>>> verb_test(l)
['test']
>>> l
['test']

But ultimately it's your choice. I'm going to use or as it makes smaller code.

---

Your classes are good. But I'd change them. A lot.

• The class SadSad is bad as it's just going to end up as an instance of Enemy. But if you really need this class I'd say use type, SadSad = type('SadSad', (object,), {}). This is as you then don't need to write all the boilerplate to make a class, and you can create all the classes on one or two lines using a comprehension. You may prefer the class way however.

  The reason I'd still make the minion a type, is as you can then just make a ton of them. But I'd not make them a class as you aren't going to and don't add anything new.

  I'd also not do goblin = Enemy(name="goblin", hp=50, attack=5). As then you can't make other goblins, and there is no hierarchy.

• I'd move inspect into Character, and remove inspect_other. This is as inspect_other is going to be used to inspect a Character or a subclass of it, which already has all the attributes in inspect. I'd also remove the inspect_other as it's a duplicate of inspect. And should have no use after moving inspect to Character.

• I'd re-organise your code so that you group classes. You broke the inventory management group by putting Room between Player and Item. And this threw me off when I was reading your code.

• I'd finally add an Equipment class that handles adding and removing items from your equipment and inventory. All the additional code that you added to the Player class modifies these properties. And so I'd write a new class to handle this, so that if you need to add it to a special NPC you can add it with ease.

  To make it somewhat easy to add I'd add it to a class via inheritance. This has the down-side of you having to manually initialize it. But it allows you to add two things and for the code to be fine.

  If I didn't like this idea then I would make it a stand alone class and manually instantiate it and call it from the child class. This allows a separation of the two objects, which doesn't make too much sense, if the whole point of the class it to allow you to easily add a feature to an existing class.

  Either way, to change to and from each other is easy.

I made some changes to the classes as I didn't like how you implemented some of the code. If we look at equip_gear you should be able to see that there is code duplication, and the only change is that you changed from self.weapon to self.armor. Instead I'd make a function that tells us which slot the object belongs in, then a function that actually equips the item, and the original function who's purpose is to interface these two functions. I also removed the print functions from this function, as I don't like the idea of printing and mutating data. As it violates SRP.

I'd also like to mention that unequip wouldn't allow us to re-equip the gear, as it never changed the state of the item. And I used the same approach as I did for equipping the item.

This resulted in me getting the following Equipment class:

class Equipment:
 def __init__(self, equipment=None, inventory=None):
 self.equipment = equipment or {}
 self.inventory = inventory or []
 def collect(self, item):
 if item.state == None:
 self.inventory.append(item)
 return True
 return False
 @staticmethod
 def _get_slot(item):
 types = [(Weapon, 'weapon'), (Armor, 'armor')]
 for class_, slot in types:
 if isinstance(item, class_):
 return slot
 return None
 def equip(self, item):
 slot = self._get_slot(item)
 if slot is None:
 return False
 return self._equip(item, slot)
 def _equip(self, item, slot):
 if self.equipment[slot] is None and item in self.inventory:
 self.equipment[slot] = item
 item.state = True
 self.inventory.remove(item)
 return True
 return False
 def unequip(self, item):
 slot = self._get_slot(item)
 if slot is None:
 return False
 return self._unequip(item, slot)
 def _unequip(self, item, slot):
 if item == self.equipment[slot]:
 item = self.equipment[slot]
 self.inventory.append(item)
 self.equipment[slot] = None
 item.state = None
 return True
 return False

To use the inheritance approach, you will have to manually call the __init__ of the Equipment class. I also added the prints here so that I don't fully break your program. And so I changed the Player class to:

class Player(Character, Equipment):
 def __init__(self, equipment=None, inventory=None, **kwargs):
 super().__init__(**kwargs)
 Equipment.__init__(self, equipment, inventory)
 @property
 def attack(self):
 return self._attack + self.equipment['weapon'].attack
 def collect(self, item):
 if not super().collect(item):
 print('You cannot collect that.')
 def equip(self, item):
 if not super().equip(item):
 print('That cannot be equipped.')

From this you should notice that I added a property to the class, one called attack. This is to simplify the parent class, and all other sub-classes. The property wrapper allows us to use it as if the function is just a variable. It also means that we can't set it, with say player.attack = 6. And so is based off the other two attributes. This is good as then we can just overwrite it in the other classes to allow for greater changes in how they attack.

I also changed the prints, this is as reading self.name + " is attacking " + other.name + "!!!!" is harder than if we were to use format. This is as with format we can use a mini-language that lets us pass in a variable to any '{}'. I pass the object self and other, then in the string I index the variables you need such as name. Which resulted in print('{0.name} is attacking {1.name}.\nThey dealt {0.attack} damage!'.format(self, other)).

I'd also like to mention that in Python3 you don't have to inherit from object, as all classes are now new-style classes by default. And you shouldn't use super in it either as "object() takes no parameters". And so I implemented:

class Character:
 def __init__(self, name='', hp=0, attack=0, **kwargs):
 self.name = name
 self.hp = hp
 self._attack = attack
 @property
 def attack(self):
 return self._attack
 def Attack(self, other):
 print('{0.name} is attacking {1.name}.\nThey dealt {0.attack} damage!'.format(self, other))
 other.hp -= self.attack
 def inspect(self):
 print('{0.name} has {0.hp} HP and {0.attack} Attack.'.format(self))

---

eval is evil! Don't use it! Instead try to use strings, Python allows for a lot of ways to do this. I personally would add say a get command to the room to have an easy way to get both the item and the index of the item in one command. The input should be the item name and the list, where the output should be the index and item (it's instance). This allows for us to easily remove the item from the list if we need to. As there is a possibility that we won't get an item I added a print to tell us that. (Your program would just error) Otherwise I'd room_actions["get"](item) and remove it from the room, room.items.pop(index). I also added a 'fight' and an 'inspect' method that worked in the same way.

I would also not base the program on repetitive calls to the same function rather than using a while loop. This is as the program will reach the maximum recursion depth. And the only way to fix this is to not use recursion, ignore the answers on SO that say to change the recursion depth, as it won't work out.

I also changed a few of the variable names and ended up changeing Room and room_logic to:

class Room:
 def __init__(self, ID=None, description='',
 characters=[], items=[], **kwargs):
 super().__init__(**kwargs)
 self.ID = ID
 self.description = description
 self.characters = characters
 self.items = items
 def get(self, list, name):
 list = getattr(self, list)
 for index, item in enumerate(list):
 if item.name == name:
 return index, item
 return -1, None
 def __str__(self):
 print(self.description)
def room_logic():
 room = current_room
 user = current_char
 print("(type 'look' to look, type 'get sword' to get the sword, and type 'move north' to move to a different room)")
 while True:
 action = input("What will {0.name} do? ".format(user)).strip()
 actions = action.split()
 if action == "look":
 print(room.description + '\n')
 elif actions[0] == "get":
 index, item = room.get('items', actions[1])
 if item is None:
 print('No item {}.'.format(actions[1]))
 continue
 room_actions["get"](item)
 room.items.pop(index)
 print("You picked up {}.".format(actions[1]))
 elif actions[0] == "fight":
 index, char = room.get('characters', actions[1])
 if char is None:
 print('No character {}.'.format(actions[1]))
 continue
 user.Attack(char)
 if char.hp <= 0:
 room.characters.pop(index)
 print('{0.name} died.'.format(char))
 else:
 print('{0.name} has {0.hp} left.'.format(char))
 elif actions[0] == "inspect":
 index, char = room.get('characters', actions[1])
 if char is None:
 print('No character {}.'.format(actions[1]))
 continue
 char.inspect()
 elif actions[0] == "move":
 if actions[1] in game_map[room.ID]:
 room = eval(game_map[room.ID][actions[1]])
 print("You have moved " + actions[1] + "!")
 else:
 print("What does that mean?")

---

To further improve this you can look into graphs in Python. They are quite easy in Python as Python is abstracted quite far from the metal. And so to make a triangular graph you could do:

class Node:
 def __init__(self, data, n=None):
 self.data = data
 self.n = n
a = Node(1)
b = Node(2, a)
c = Node(3, b)
a.n = c
print(a.data) # 1
print(a.n.data) # 3
print(a.n.n.data) # 2
print(a.n.n.n.data) # 1

Using something like this you could change your Room class to work without eval.

---

I'd also highly recommend that you read and follow PEP8, your code has a lot of inconsistency. And it was hard to work with.

Here's all the changes I made:

import time
import random
class Equipment:
 def __init__(self, equipment=None, inventory=None):
 self.equipment = equipment or {}
 self.inventory = inventory or []
 '''
 def __getattribute__(self, name):
 equipment = self.equipment.get(name, None)
 if equipment is not None:
 return equipment
 return self.__dict__[name]
 '''
 def collect(self, item):
 if item.state == None:
 self.inventory.append(item)
 return True
 return False
 @staticmethod
 def _get_slot(item):
 types = [(Weapon, 'weapon'), (Armor, 'armor')]
 for class_, slot in types:
 if isinstance(item, class_):
 return slot
 return None
 def equip(self, item):
 slot = self._get_slot(item)
 if slot is None:
 return False
 return self._equip(item, slot)
 def _equip(self, item, slot):
 if self.equipment[slot] is None and item in self.inventory:
 self.equipment[slot] = item
 item.state = True
 self.inventory.remove(item)
 return True
 return False
 def unequip(self, item):
 slot = self._get_slot(item)
 if slot is None:
 return False
 return self._unequip(item, slot)
 def _unequip(self, item, slot):
 if item == self.equipment[slot]:
 item = self.equipment[slot]
 self.inventory.append(item)
 self.equipment[slot] = None
 item.state = None
 return True
 return False
class Item(object):
 def __init__(self, name='', state=None, **kwargs):
 super().__init__(**kwargs)
 self.name = name
 self.state = state
class Weapon(Item):
 def __init__(self, attack=0, **kwargs):
 super().__init__(**kwargs)
 self.attack = attack
class Armor(Item):
 def __init__(self, armor=0, **kwargs):
 super().__init__(**kwargs)
 self.armor = armor
class Character:
 def __init__(self, name='', hp=0, attack=0, **kwargs):
 self.name = name
 self.hp = hp
 self._attack = attack
 @property
 def attack(self):
 return self._attack
 def Attack(self, other):
 print('{0.name} is attacking {1.name}.\nThey dealt {0.attack} damage!'.format(self, other))
 other.hp -= self.attack
 def inspect(self):
 print('{0.name} has {0.hp} HP and {0.attack} Attack.'.format(self))
class Enemy(Character):
 def __init__(self, **kwargs):
 super().__init__(**kwargs)
class Player(Character, Equipment):
 def __init__(self, equipment=None, inventory=None, **kwargs):
 super().__init__(**kwargs)
 Equipment.__init__(self, equipment, inventory)
 @property
 def attack(self):
 return self._attack + self.equipment['weapon'].attack
 def collect(self, item):
 if not super().collect(item):
 print('You cannot collect that.')
 def equip(self, item):
 if not super().equip(item):
 print('That cannot be equipped.')
class Room:
 def __init__(self, ID=None, description='',
 characters=[], items=[], **kwargs):
 super().__init__(**kwargs)
 self.ID = ID
 self.description = description
 self.characters = characters
 self.items = items
 def get(self, list, name):
 list = getattr(self, list)
 for index, item in enumerate(list):
 if item.name == name:
 return index, item
 return -1, None
 def __str__(self):
 print(self.description)
fop = Weapon(name='Fists of Punching', attack=5)
sword = Weapon(name="sword", attack = 6)
goblin = Enemy(name="goblin", hp=50, attack=5)
SadSad = Enemy(name="SadSad", hp=5, attack=1)
r = Player(name="jeremyodell", hp=999, equipment={'weapon':fop})
Room0 = Room(ID="Room0", description="You are literally in Hell. There is fire all \
around. You are scared and tired. There is a weapon on the ground. \
You see a goblin and a sword.", characters = [goblin],\
 items = [sword])
Room1 = Room(ID="Room1", description="You have escaped from Lucifer, and now find\
 yourself surrounded by two SadSads. Also there are a pair of fists on the ground.",\
 characters = [SadSad, SadSad], items = [sword, fop])
current_room = Room0
current_char = r
game_map = {"Room0": {"north":"Room1", "east":"Room2", "west":"Room3"},
 "Room1": {"south":"Room0", "east":"Room4", "west":"Room5"}}
room_actions = {"get": current_char.collect, "talk":0,"menu":0, "fight":0}
def room_logic():
 room = current_room
 user = current_char
 print("(type 'look' to look, type 'get sword' to get the sword, and type 'move north' to move to a different room)")
 while True:
 action = input("What will {0.name} do? ".format(user)).strip()
 actions = action.split()
 if action == "look":
 print(room.description + '\n')
 elif actions[0] == "get":
 index, item = room.get('items', actions[1])
 if item is None:
 print('No item {}.'.format(actions[1]))
 continue
 room_actions["get"](item)
 room.items.pop(index)
 print("You picked up {}.".format(actions[1]))
 elif actions[0] == "fight":
 index, char = room.get('characters', actions[1])
 if char is None:
 print('No character {}.'.format(actions[1]))
 continue
 user.Attack(char)
 if char.hp <= 0:
 room.characters.pop(index)
 print('{0.name} died.'.format(char))
 else:
 print('{0.name} has {0.hp} left.'.format(char))
 elif actions[0] == "inspect":
 index, char = room.get('characters', actions[1])
 if char is None:
 print('No character {}.'.format(actions[1]))
 continue
 char.inspect()
 elif actions[0] == "move":
 if actions[1] in game_map[room.ID]:
 room = eval(game_map[room.ID][actions[1]])
 print("You have moved " + actions[1] + "!")
 else:
 print("What does that mean?")
room_logic()