Simple text RPG in Python

Question

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,
                         ac=6,inventory={},
                         exp=8)

def profession():
    print("What is your class?",'\n',
          " press f for Fighter",'\n',
          " press c for Cleric",'\n',
          " press m for Mage")
    pclass=input(">>>")
    if pclass =="f":
        Prof = Fighter()
    elif pclass=="c":
        Prof = Cleric()
    elif pclass == "m":
        Prof = Mage()
    else:
        Prof=Fighter()
        #profession()
    return Prof
def ranmob():
    mob = Goblin() if Dice.die(2)<2 else Orc()
    return mob


def playerAttack():
    roll=Dice.die(20)   
    if roll>=hero.thaco-mob.ac:
        print("You hit")
        if hero.prof=="fighter":
            rollD=Dice.die(10)

        if hero.prof=="cleric":
            rollD=Dice.die(6)

        if hero.prof=="mage":
            rollD=Dice.die(4)
        print("for",rollD,"damage")
        mob.hp-=rollD
        print("the",mob.name,"has",mob.hp,"hp left")
    else:
        print("You miss")

def monsterAttack():
    roll=Dice.die(20)   
    if roll>=mob.thaco-hero.ac:
        print("Monster hit")
        if mob.name=="goblin":                    
            rollD=Dice.die(4)
        elif mob.name=="orc":
            rollD=Dice.die(6)
        print("for",rollD,"damage")
        hero.hp-=rollD
        print(hero.name,"has",hero.hp,"hp left")
    else:
        print("Monster misses")

def levelUp():


    while hero.exp>=hero.level2:
        levelGain=False
        hero.level+=1
        levelGain=True
        hero.level2=hero.level2*2
        if levelGain==True:
            hero.maxhp+=Dice.die(hero.hd)
            hero.hp=hero.maxhp
            if hero.prof=="mage":
                hero.maxmana+=1
                hero.mana=hero.maxmana

            print("You Gained a level","\n",'hp:',hero.hp,"\n",'level:',hero.level)
            levelGain=False
    while hero.level>=3:
        hero.level-=3
        hero.thaco-=1
        print("thaco:",hero.thaco)


def commands():
    if hero.prof=="fighter":
        print (" press f to fight",'\n',
               "press enter to pass")
        command=input("~~~~~~~~~Press a key to Continue.~~~~~~~")
        if command=="f":            
            playerAttack()
        if command=="":
            pass

    if hero.prof=="cleric":
        print (" press f to fight",'\n',
               "press h to heal",'\n',
               "press enter to pass")
        command=input("~~~~~~~~~Press a key to Continue.~~~~~~~")
        if command=="f":            
            playerAttack()
        elif command =="h":
            if hero.hp<hero.maxhp:
                hero.hp+=Dice.die(8)                
                if hero.hp>hero.maxhp:
                    hero.hp=hero.hp-(hero.hp-hero.maxhp)                    
                print("You now have:",hero.hp,"hp")
            else:
                print("Your hit points are full")
                commands()
        elif command=="":
            pass
    if hero.prof=="mage":
        print (" press f to fight",'\n',
               "press s for spells",'\n',
               "press m to generate mana",'\n',
               "press enter to pass")
        command=input("~~~~~~~~~Press a key to Continue.~~~~~~~")
        if command=="f":            
            playerAttack()
        elif command =="s":
            print("You have",hero.mana,"mana")
            if hero.mana>=1 and hero.mana<3:
                print("press s for sleep",'\n',
                      "press m for magic missile")
                command=input(">>>")
                if command =="s":
                    print("You put the monster to sleep it is easy to kill now")
                    mob.hp-=mob.hp
                    hero.mana-=1
                if command=="m":
                    if hero.mana<hero.maxmana:
                        hero.mana+=Dice.die(4)                
                        if hero.mana>hero.maxmana:
                            hero.mana-=(hero.mana-hero.maxmana)
                    dam =Dice.die(4)*hero.mana
                    mob.hp-=dam
                    print("You use all your mana! and do",dam,"damage!")
                    hero.mana-=hero.mana
            elif hero.mana>=3:
                print("press s for sleep",'\n',
                      "press m for magic missile",'\n',
                      "press f for fireball")
                command=input(">>>")
                if command =="s":
                    print("You put the monster to sleep it is easy to kill now")
                    mob.hp-=mob.hp
                    hero.mana-=1
                if command=="m":
                    dam=Dice.die(4)*hero.mana
                    mob.hp-=dam
                    print("You use all your mana! and do",dam,"damage!")
                    hero.mana-=hero.mana
                if command=="f":
                    print("You are temporarily blinded by a feiry flash of light.")
                    dam=0
                    dam+=Dice.die(6)
                    dam+=Dice.die(6)
                    dam+=Dice.die(6)
                    mob.hp-=dam
                    print("You did",dam,"points of damage")

                    hero.mana-=3
            else:
                print("Your mana is empty")
                commands()
        elif command =="m":
            if hero.mana<hero.maxmana:
                hero.mana+=1
                print("You have",hero.mana,"mana")
            elif hero.mana>=hero.maxmana:
                print("Your mana is full.")
                print("You have",hero.mana,"mana")
                commands()

        elif command=="":
            pass

mob=ranmob()
hero=profession()
print("name hp thaco ac inventory xp",'\n',
      hero.name,hero.hp,hero.thaco,hero.ac,hero.inventory,hero.exp)
while True:

    if mob.hp<=0:
        print('The',mob.name,'is dead!')        
        hero.exp+=mob.exp        
        print('hero xp',hero.exp)
        mob=ranmob()
    if hero.hp<=0:
        mob.exp+=hero.exp
        print("mob xp:",mob.exp)
        print(hero.name,'died!')
        #name=input("What is your characters name?")                
        hero=profession()
        print("name hp thaco ac inventory xp",'\n',
              hero.name,hero.hp,hero.thaco,hero.ac,hero.inventory,hero.exp)

    levelUp()

    print("You see",mob.name+",",mob.name,"has",mob.hp,"hp.")
    if hero.hp>0:
        commands()
    if mob.hp>0:                
        monsterAttack()

Answer 1

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

A few points:

1. I would call the method roll, and the class Die;
2. It's typical to model a die by setting the number of sides in __init__, then calling roll without any arguments;
3. 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:

1. random_mob would be a better name;
2. This returns a single enemy, which I'm not sure I'd call a "mob";
3. 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..

Answer 2

There's a lot of code here, (258 lines), so I'm not going to give a full review, but rather, a few suggestions on what you can improve in general.

• My first suggestion is to be consistent, and clear with your whitespace usage. For example, on some lines, you have something like this: self.thaco=thaco, and on other lines you have something like this: prof= "cleric". Both of these should be changed to something like variable = value. This makes it clearer to read. You should also have whitespace between operators, for example: x >= y, and two blank lines in between function and class declarations.
• Another suggestion would be to add comments and docstrings explaining what your code does. For example:

---

def my_function(arguments):
    """
    This is a docstring. Put an explanation of
    your function or class here, and put your code below.
    """
    pass

---

• When naming classes, variables, and functions, it's important to give them good names. For example, I have no idea what the variable self.thaco is used for from just looking at it. Names should be descriptive of what they are used for, but not too long either. After all, nobody wants to have to type an name like this_is_the_x_position_of_the_player.
• You have quite a few functions which are just large, hard-to-read blocks of code. See if you can separate certain actions out into smaller, more easy to read functions. This is for the benefit of you, and someone who might want to read your code.
• I'd also create a generalized function for choosing player decisions, rather than large if/elif/else blocks. Here's what I'd implement.

---

def player_decision(player_input, possible_choices):
    """
    Return a value based on the player's input.
    """
    if player_input in possible_choices:
        return possible_choices[player_input]
    else:
        print "Invalid choice!"

---

• player_input would simply be the string the player inputs, and possible_choices is a dictionary of possible choices and their return values.
• Finally, I'd recommend reading Python's official style guide, PEP8.

Anyway's, I hope this helps you with your project!

Answer 3

The Official style guide

You have about 250 violations to PEP8, use autopep8 to make your code more readable

Dictionary

A long series of if and elif is considered bad style, a dictionary also allows easier adding of more classes and options.

def profession():
    letter_to_profession = {
        'f': Fighter,
        'c': Cleric,
        'm': Mage
        }
    print("What is your class?\n")
    for letter in letter_to_profession.keys():
        print("- Press {} for {}".format(
            letter, letter_to_profession[letter].__name__))
    pclass = input(">>>")
    return letter_to_profession[pclass]()

Also note that the printing is generated from the dictionary for maximum convenience.

Stand-alone functions

In Python functions can float freely in the global namespace an it is raccomended that they do so when it makes sense, so:

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

Must become:

def die(sides):
     return randint(1,num)

also it is better to return the value directly (without and intermediate variable).

Extend the classes

def playerAttack():
    roll=Dice.die(20)   
    if roll>=hero.thaco-mob.ac:
        print("You hit")
        if hero.prof=="fighter":
            rollD=Dice.die(10)

        if hero.prof=="cleric":
            rollD=Dice.die(6)

        if hero.prof=="mage":
            rollD=Dice.die(4)
        print("for",rollD,"damage")
        mob.hp-=rollD
        print("the",mob.name,"has",mob.hp,"hp left")
    else:
        print("You miss")

This if chain is not nice and will become even less nice when more characters are added, you should implement the max_damage or max_power inside the Player Class so as to write:

def playerAttack():
    roll = Dice.die(20)
    if roll >= hero.thaco - mob.ac:
        print("You hit")
        roolD = die(hero.max_power)
        print("for", rollD, "damage")
        mob.hp -= rollD
        print("the", mob.name, "has", mob.hp, "hp left")
    else:
        print("You miss")

Put effort into pretty printing.

The following:

print("name hp thaco ac inventory xp", '\n',
      hero.name, hero.hp, hero.thaco, hero.ac, hero.inventory, hero.exp)

when printed looks like:

name hp thaco ac inventory xp 
 foobar 10 20 10 {} 10

Instead:

print("Name: {}, HP: {}, Thaco: {}, Ac:{}, Inventory:{}, XP:{}\n".format(
     hero.name, hero.hp, hero.thaco, hero.ac, hero.inventory, hero.exp))

Looks like:

Name: foobar, HP: 4, Thaco: 20, Ac:10, Inventory:{}, XP:4

Answer 4

The Dice class is superfluous as written. If you just want to be able to invoke a die roll, you just need the function

def roll(s):
    return random.randint(1, s)

However, the Dice class makes sense if you were storing it to be used for something. For example, if you make the modification suggested in this other answer, then you could modify your classes such as

class Fighter(Character):
    # ...
    damageDice = Die(10)

and then playerAttack would simplify to

def playerAttack():
    roll=Die(20).roll()
    if roll>=hero.thaco-mob.ac:
        print("You hit")
        rollD=hero.damageDice.roll()
        print("for",rollD,"damage")
        mob.hp-=rollD
        print("the",mob.name,"has",mob.hp,"hp left")
    else:
        print("You miss")

This change also opens up the path to make damageDice something more interesting in the future, and to make it an attribute of instances of a character rather than being tied to its class -- e.g. your fighter might upgrade to a +1 greatsword, and you'd want to change damageDice to an object that would roll 2d6+1.