Small Text Game Using Python

Question

I just finished making(well, trying to make one) a small text adventure in Python and I was wondering what I could have improved with it. Please note that this is my first time using Python. I pretty much had to keep Google open the whole time.

"""
Created on Sat Dec 23 23:57:38 2017
"""
import time

hero_race = {1: "Human", 2: "Elf", 3: "Orc"}
race_choice = None
hero_class = {1: "Warrior", 2: "Mage", 3: "Healer"}
class_choice = None
hero_name = None
map_level = None  # Map_level will act as the key for dungeon_map, dungeon_description and level_complete
dungeon_map = {1: "Training_Room", 2: "Slime_Room", 3: "Chimera_Room", 4: "Demon_Room"}
dungeon_description = {1: "You are standing in the knight's training chambers. "
                          "In front of you lies your master-at-arms.",
                       2: "You enter the room. You can barely make out anything from this dark area. However, you see "
                          "a glowing blue light straight ahead of you.\nThe light burns greater... it begins to make "
                          "the shape of a creature... a slime!",
                       3: "As you proceed into the room, you hear eerie noises from all across the room, "
                          "followed by a dark shape that moves too fast for you to catch. You stand in the middle of "
                          "the round room, you hear a loud growl behind you.\nYou turn around to fight a lion, no..."
                          "it's a two-headed creature, both a lion and a giant serpent.",
                       4: "As you step into the room, you find it hard to stand, as if an unknown force is pushing "
                          "your body down into the ground. You start shaking, your teeth grinding.\nIt takes"
                          " you a moment, but you are finally able to regain control of your body. Already tired "
                          "from your previous trials, you push ahead and find an empty throne.\nYou blink and all of a "
                          "sudden there is a dark figure sitting on the chair. He, no... It smiles and stands up, "
                          "then walks toward you.\n.... This is your final challenge."}
level_complete = {1: "After several excruciating blows, your sparring session is over.\n"
                     "Your teacher guides you to the gate where your "
                     "first true challenge begins to become a full-pledged knight.",
                  2: "you fight it off and you (surprisingly) beat your first real enemy.\n"
                     "Behind the gooey carcass of the now-defeated slime lies another room with the doorknob of a lion",
                  3: "You fight it off and you barely beat "
                     "this vicious creature. You reach the edge of the room to find a dark door. "
                     "Red mist flows through the gaps of the door.\nDo you proceed? This may be your final choice",
                  4: "With great struggle, you defeat your final challenge. Now you are ready to become a full-fledged"
                     " knight."
                  }
hero_health = 100
hero_attack = 50


class Monster:
    def __init__(self):
        self.health = 100
        self.attack = 20


def create_hero():
    global hero_name
    global map_level
    global race_choice
    global class_choice
    map_level = 1
    print("What is your name?")
    hero_name = input("My name is: ")

    print("Choose a race")
    print("1- Human\t\t2- Elf\t\t3- Orc")
    race_choice = int(input("My race is: "))

    print("Choose a class.")
    print("1- Warrior\t\t2- Mage\t\t3- Healer")
    class_choice = int(input("My class is: "))
    return


def save_game():
    global race_choice
    global class_choice
    global map_level
    hero_info = str.format("{0:10}{1:10}{2:10}{3:10}{4:10}",
                           hero_name, hero_race[race_choice], hero_class[class_choice], dungeon_map[map_level],
                           hero_health)
    f = open("hero_information.txt", "w")
    f.writelines(hero_info)
    f.close()


def load_game():
    global hero_name
    global race_choice
    global class_choice
    global map_level
    global hero_health
    try:
        f = open("hero_information.txt", "r")
    except FileNotFoundError:
        print("Save file doesn't exist")
        return 1
    line = f.readline()
    hero_info = str.split(line)
    hero_name = hero_info[0]
# The save file contains the VALUES for the dictionary variables, we do this to obtain the keys
    for k, v in hero_race.items():
        if v == hero_info[1]:
            race_choice = k
            break
    for k, v in hero_class.items():
        if v == hero_info[2]:
            class_choice = k
            break
    for k, v in dungeon_map.items():
        if v == hero_info[3]:
            map_level = k
            break
    hero_health = int(hero_info[4])
    f.close()


def status():
    global race_choice
    global class_choice
    print("Welcome back %s the %s %s, you have %s health" % (hero_name, hero_race[race_choice],
                                                             hero_class[class_choice], hero_health))
    return


def encounter():
    global map_level
    if map_level < 5:
        print(dungeon_description[map_level])
        battle()
        print(level_complete[map_level])
    return


def map_navigation():
    global map_level
    while 1:
        print("1- Proceed\t2- Status\t3- Save\t4- Exit")
        map_choice = int(input("What should I do?... "))
        if map_choice == 1:
            map_level += 1
            return 0
        elif map_choice == 2:
            status()
        elif map_choice == 3:
            save_game()
        elif map_choice == 4:
            return 1


def battle():
    global hero_health
    monster = Monster()
    print("Prepare to battle!\n...")
    time.sleep(3)
    while monster.health > 0:
        print("Prepare for monster attack! ....")
        time.sleep(2)
        print("Monster attacks you for {} damage".format(monster.attack))
        hero_health -= monster.attack
        time.sleep(1)
        print("Your turn to fight back!\n...")
        time.sleep(2)
        print("You attack the monster for {} damage!".format(hero_attack))
        monster.health -= hero_attack
        print("Monster has only {} health remaining".format(monster.health))


def main():
    global map_level
    while 1:
        print("1- New Game\n2- Load Game\n3- Exit")
        start_choice = int(input())

        if start_choice == 1:
            create_hero()
            while map_level < 5:
                encounter()
                if map_navigation() == 1:
                    break
            print("To be continued.")
            break

        elif start_choice == 2:
            if load_game() == 1:  # 1 is the error return value if there is no save file
                continue
            while map_level < 5:
                if map_navigation() == 1:
                    break
                encounter()
            print("To be continued.")
            break

        elif start_choice == 3:
            break


if __name__ == "__main__":
    main()

There isn't a lot of functionality yet (or even a coherent story line). The battles are automated and I haven't done anything in case the hero's health drops to 0 or below. I had a couple of things I was curious if I could improve, first of all, was it a good idea to keep the dungeon_description and level_complete as dictionaries?

Second of all, I noticed I had to use the keyword global [variable_name] several times, could I have done something to avoid that?

P.S.: I just realized that I didn't need to use global for the monster health or attack in the battle function, since they were initialized in a Monster class. Should I have created a Class for hero and defined the variables there?

Also, any other notes on what else I could improve would be greatly appreciated! My biggest concern is how to optimize this.

Answer 1

Congrats, for your first Python program you have done a really good job. I've seen programmers with a lot more experience write less clean code. What I like most about your code is that to a great extent it is self-documenting; your naming is clear and concise and your structure is easy to follow. Nonetheless, there are some areas that would make it better.

1. I would separate all text into a separate file. For now, you can just define variables. Later a YAML file or similar would be a great choice. This does two things. It allows you to more easily insert different languages (other than English). AsIt also cleans up your code, making it easier to read the business logic without getting lost in the text.
2. Use a list instead of a dictionary of 1:item1, 2:item2.
3. A monster class was a good choice. I would have a hero class as well. That way you can pass the hero object around, both as a return from the create method as well as parameters to your encounter method.
4. Instead of using global, consider passing as a parameter. I rarely use global because it almost always violates the principle of least surprise.
5. Some of your method names are not self-documenting. The method "status", for example, needs a name change. Does it get the status? write the status? or what? I shouldn't have to step into your code to know what it does.
6. By making hero a class, and probably map as well, saving the game can just be a matter of serializing your objects using pickle.
7. Prefer to return tuples or objects vs. modifying globals inside a method. For example, the battle method should accept a monster and hero, and return the modified hero, not modify a global hero variable. Your functions should be able to exist as a stand-alone item -- depending only on input parameters (unless part of a class, then they can depend on class variables also).

There is more, but these items will get you a long way. I'd say make these changes and resubmit.

Answer 2

Good points from @SteveJ, but I'd like to add something.

1. Limit all lines to a maximum of 79 characters.

2. If you have Python3.6 you can use f-strings. For example, your status printing will look like this:

   print(f"Welcome back {hero_name} "
         f"the {hero_race[race_choice]} {hero_class[class_choice]}, "
         f"you have {hero_health} health")
   

3. Instead of while 1 write while True.

4. Don't use single letter variable names. Rename your f as file, k as key, and v as value.

5. There is no need to add return in the end of your functions if they don't return anything.

6. In main conditions start_choice == 1 and start_choice == 2 are almost the same. Don't repeat yourself. Also, put the condition for start_choice == 3 at the top, so you exit earlier when you don't need to do any processing.

7. Instead of iterating by numeric indices like what you do with your dungeon maps, iterate over elements of some list/tuple/dict containing them. So, you won't need that magic number 5. Imagine yourself adding new maps. You won't reach them because of this number. And it is encountered three times in the code, so you would have to go through the code and change it to 6, 7, etc. Not cool I think.

8. In encounter you don't need to check if map_level < 5 because it is already satisfied in main. Probably, you don't need this function at all.

9. You write open("hero_information.txt", "r") but r is a default mode. So you can omit it. Also, instead of writing:

   try:
       f = open("hero_information.txt", "r")
   except FileNotFoundError:
       print("Save file doesn't exist")
   

   You can use os.path.exists or os.path.isfile.
   And it would be better if you take this checking out of the function and return real data instead of 1 or nothing.

10. Use with statement to open files.

11. I am not sure about this one. But I've never seen anyone splitting a line like this: str.split(line). I write line.split() instead and this is what I see all the time.

12. Looking for a key by a value of a dict in load_game is a sign that probably you are doing something wrong. See the 2nd point of @SteveJ answer.

13. When you are giving a choice to a user, you could write something like this:

    for index, choice in enumerate(choices, start=1):
        print(f'{index}- {choice}')  
    

Waiting for the next version without any globals!

Edit: Answering your question about last point.
Personally, I would make the choices as default arguments of the main function. And, therefore, I would use tuples instead of a lists as they are immutable. (Explanation on why mutable default argument should be avoided: link) For example, only for a start_choices:

def main(start_choices=('New Game', 'Load game', 'Exit')):
    print_choices(start_choices)
    # The rest of the code...

Where print_choices is:

def print_choices(choices):
    for index, choice in enumerate(choices, start=1):
        print(f'{index}- {choice}')

When there are too many elements in the tuple and things start to look not pretty, I take them out like this:

START_CHOICES = ('New Game', 'Load game', 'Exit')


def main(start_choices=START_CHOICES):
    print_choices(start_choices)
    # The rest of the code...

Or even create a separate module and put them there if it is justified. For example, it could look like this:

import main_menu 


def main(start_choices=main_menu.START_CHOICES):
    print_choices(start_choices)
    # The rest of the code...