Simple text RPG in Python

I am trying to teach myself to code using Python. The following is the first real program I have written from scratch. I feel that it is messy and in need of improvement, but I am either unsure of what to improve next or don't know how to improve it. I am looking for suggestions about: turning my functions into class methods, implementing the inventory{} list, improving my ranmob function to work with a much larger (list?) of monsters, and possibly turning my while True loop into a gameLoop function. Any other general advice would be appreciated.

```
from random import randint
class Dice:
def die(num):
die=randint(1,num)
return die
class Character:
def __init__(self,name,hp,thaco,ac,inventory,exp):
self.name=name
self.hp=hp
self.thaco=thaco
self.ac=ac
self.inventory=inventory
self.exp=exp

class Fighter(Character):
def __init__(self):
super().__init__(name=input("What is your characters name?"),thaco=20,ac=10,
hp=10,inventory={},exp=10)
prof = "fighter"
maxhp=10
level=1
hd=10
level2=20

class Cleric(Character):
def __init__(self):
super().__init__(name=input("What is your characters name?"),thaco=20,ac=10,
hp=8,inventory={},exp=8)
prof= "cleric"
maxhp=8
level=1
hd=8
level2=15
class Mage(Character):
def __init__(self):
super().__init__(name=input("What is your characters name?"),thaco=20,ac=10,
hp=4,inventory={},exp=4)
prof= "mage"
mana=1
maxmana=1
maxhp=4
level=1
hd=4
level2=10
class Goblin(Character):
def __init__(self):
super().__init__(name="goblin",
hp=7,thaco=20,
ac=6,inventory={},
exp=7)

class Orc(Character):
def __init__(self):
super().__init__(name="orc",
hp=8,thaco=18,

class Dice:    
    def die(num):
        die=randint(1,num)
        return die

A few points:

• I would call the method roll, and the class Die;

• It's typical to model a die by setting the number of sides in __init__, then calling roll without any arguments;

• Why bother assigning die?

I would have written:

class Die:
    """Represents a single die."""

    def __init__(self, sides=6):
        """Set the number of sides (defaults to six)."""
        self.sides = sides

    def roll(self):
        """Roll the die."""
        return random.randint(1, self.sides)

Note the use of docstrings to provide information about the class and its methods. Now e.g. Dice.die(2) becomes Die(2).roll(), which I think is much clearer about what's happening, and you can make a single die:

four_sided_die = Die(4)

and roll it repeatedly:

four_sided_die.roll()

I would be inclined to make the player classes separate to the monster classes, so you'd have an inheritance structure like:

Character
               /         \
         Player           Monster
        /   |  \           /    \
 Fighter  Mage  Cleric  Goblin   Orc

This lets you factor out more of the duplication. For example:

class Player(Character):

    def __init__(self, hp, exp):
        super().__init__(input("What is your character's name? "),
                         20, 10, hp, {}, exp)

class Fighter(Character):

    # Note that constants should be UPPERCASE_WITH_UNDERSCORES
    HD = 10
    LEVEL = 1  # should this really be a class attribute?
    LEVEL_2 = 20
    MAX_HP = 10
    PROF = "fighter"

    def __init__(self):
        super().__init__(10, 10)

def ranmob():
    mob = Goblin() if Dice.die(2)<2 else Orc()
    return mob

A few points:

• random_mob would be a better name;

• This returns a single enemy, which I'm not sure I'd call a "mob";

• There's no need to use the Dice.

I'd use random.choice for this, and allow a size of mob to be specified:

ENEMIES = (Goblin, Orc)

def random_mob(size):
    return [random.choice(ENEMIES)() for _ in range(size)]

this uses a list comprehension to create a list of randomly-selected enemies from the tuple.

There is a vast amount of duplication in commands, and it contains logic (what the Players can do) that should be stored with the Player. For example, if each Player subclass had a dictionary of valid commands (each implemented as an instance method):

class Fighter(Player):

    ...

    def fight(self):
        super().fight()  # all players can fight

    def cast_spell(self):
        ...

    def generate_mana(self):
        ...

    COMMANDS = {
        'f': ('fight', fight),
        's': ('spells', cast_spell),
        'm': ('generate mana', generate_mana),
    }

Then commands starts:

# Show the valid commands
for command, action in hero.COMMANDS.items():
    print('press {} to {}'.format(command, action[0]))
print('press Enter to skip')

# Get validated user input
while True:
    command = input("~~~~~~~~~Press a key to Continue.~~~~~~~")
    if command and command not in hero.COMMANDS:
        print('Not a valid command')
        continue
    break

# Run the appropriate action
if command:
    hero.COMMANDS[command][1]()  # call the method

See Asking the user for input until they give a valid response for more on input validation. You can extend this to define the appropriate parameters for each action, etc..

StackExchange Code Review Q#91069, answer score: 16