Skeleton for a text-based RPG

Question

I'm new to python and created this small program. It's basically a skeleton for a simple text-based RPG. The program is one menu that allows you to:

• Create a Character
• View Character Stats/Attributes
• Load Character
• Battle
• Save
• Quit

I'm looking for constructive criticism from any angle, be that if I comment too much, if I'm not comment to specific, if my variables are to implicit, or any other flaws in my programming. Perhaps a better way for a menu but this time using tuples or dictionaries. Perhaps you noticed a redundancy in my code.

I'd also appreciate if someone can give me some direction if my goal is to make a top-side RPG. Should I use tkinter? PyGame? Tmxlib? Or should I just focus on the programming aspect for now until I'm a bit more experienced before tackling a library?

# Pickle for object serialization, OS to check if character data file exist
import pickle
import os

class Character(object):
    def __init__(self, name, exp):
        self.name = name
        self.exp = exp
        self.lvl = self.exp/100

    # Simple character statistic display using the new format technique instead of %
    def stats(self):
        print ("Character stats: {}, you are level {} with {} experience points!".format(self.name,self.lvl,self.exp))

    # Simple battle program, has one paramter for the amount of EXP
    # self.lvl is necessary to update the self.lvl variable with the actual level
    def battle(self,exp):
        self.exp = self.exp + exp
        self.lvl = self.exp/100
        print ("Successful battle! {} experienced gained!".format(exp))
        self.stats()


# Not sure what to comment for starting variables? "Global variables?" "Initial variables"?
# sets menu on loop and player sees the main menu
loop_menu = True
menu_choice = "main"

# error prevention
new_character = None
fileCheck = "charData"

# menu_display
# 1 = menu with input
# "new" = input
menu_display = 1

# menu_error
# None = no error yet
# 1 = invalid input received
menu_error = None

while loop_menu == True:
    if menu_display == 1:
        print("New Character, Stats, Load, Battle, Save or Quit?")
        menu_choice = (input("Input your choice: ")).lower()
        menu_display = 0

    if menu_display == "new" and menu_error == None:
        menu_choice = (input("What would you like to do now?: ")).lower()

    if menu_display == "new" and menu_error == 1:
        menu_choice = (input("Please type a real answer, what now?: ")).lower()
        menu_error = None

    # Text Formatting
    print ("\n", end="")

    if menu_choice == "new":
        chardata = ["Foo", 300]
        new_character = Character(*chardata)
        print ("New character created!")
        new_character.stats()

    elif menu_choice == "stats":
        if new_character == None:
            print ("Sorry, you can't check your stats, you haven't made a new/loaded a character yet!")

        else:
            new_character.stats()

    elif menu_choice == "load":
        if os.path.exists(fileCheck):
            print("Load successful!")
            inFile = open("charData", "rb")
            new_character = pickle.load(inFile)
            inFile.close()

        else:
            print("You can't load yet! Data file not created!")
            print("You can use the \"save\" feature after you've created a character to automatically create a save file!")

    elif menu_choice == "battle":
        if new_character == None:
            print("You can't battle, you haven't made a character")

        else:
            new_character.battle(100)

    # Saving character to data file
    elif menu_choice == "save":
        print ("Character saved!")
        outFile = open("charData", "wb")
        pickle.dump(new_character, outFile)
        outFile.close()

    elif menu_choice == "quit":
        print ("Quitting program...")
        break

    else:
        menu_error = 1

    # Text Formatting
    print ("\n", end="")

    # Skips the instructions and goes straight to the input
    menu_display = "new"

Answer 1

Design considerations

There are a few higher level design considerations that will make your life a lot easier if you decide to make a more complicated game or go to a GUI approach.

Input/output

One thing I see is that you don't have a clear separation of the input/output and the game logic. This will make it considerably harder to move to say a GUI approach in the future.

For example:

class Character(object):
    def battle(self,exp):
        self.exp = self.exp + exp
        self.lvl = self.exp/100
        print ("Successful battle! {} experienced gained!".format(exp))
        self.stats()

You have 2 things going on here that really should be separate. You have the battle aspect that updates information about the character and you have some output handling. As it currently stands there is an implicit assumption in parts of your code that the game is a terminal based game because of where the input/output occurs.

These types of situations are much more nicely handled if you split the input/output out from the game logic.

So in this case:

new_character.battle(100)

Might become

old_exp, old_lvl = new_character.exp, new_character.exp
new_character.battle(100)
print ("Successful battle! {} experienced gained!".format(new_character.exp - old_exp))

Now if you move to a GUI approach you just change the print here to something else. This is a lot easier than having to track down the print statements scattered throughout the code.

Lack of functions

There's a noticeable lack of functions here. An especially good example is the menu handler, with the large change of else-if statements:

if menu_choice == "new":
    chardata = ["Foo", 300]
    new_character = Character(*chardata)
    print ("New character created!")
    new_character.stats()

I would start breaking out things like this into functions:

def create_new_character(character_data):
    """Create a new character"""
    new_character = Character(*chardata)
    new_character.stats()
    return new_character

if menu_choice = "new":
    new_character = create_new_character(["Foo", 300])
    print ("New character created!")

Again notice how I am separating out the input/output here from the other functionality. Step 1: create the character then step 2: deal with the output. I would do a similar refactoring with other cases in this chain of else-if's.

Mutable global state

This can turn into a particularly nasty maintenance burden as your codebase grows in size. It makes the flow of the code harder to follow and reason about. This is because having global state that can be change from anywhere makes it much harder to track down which parts of the code can change other parts of the code.

I would strongly recommend creating a class that manages the state of the game which contains the main game loop. This will pay off whenever you have issues you have to debug.

Python usage

In some places you are comparing against None and other "truthiness" values. This is not a recommend practice. For example

while loop_menu == True:

if menu_display == "new" and menu_error == None:

can be written as:

while loop_menu:

if menu_display == "new" and not menu_error:

I notice you quit the program by breaking out of the main game loop. If there is code after the thread loop you might not immediately quit here. If your intent is to quite immediately you can use something like:

elif menu_choice == "quit":
    print ("Quitting program...")
    import sys
    sys.exit()

This guarantees your code exits here.

Answer 2

I noticed that you've naming styles. As per PEP8, Python's official style guide, this is how you should be naming your variables/functions/classes.

1. Variables/function parameters should be in snake_case. Constant variables should be in UPPER_SNAKE_CASE.
2. Functions should be in snake_case as well.
3. Classes should be in PascalCase.

---

Secondly, instead of using regular inline comments, #, to describe your functions/classes, you should be using docstrings. Here's an example of a function docstring:

def my_func( ... ):
    """
    Add a description of your function and it's
    arguments here.
    """
    ...

---

Instead of chaining if/elif/else statements, you can do something like this using dictionaries.

actions = {
    "action_name": function_to_run,
    ...
}

action_input = input("> ")
if action_input in actions:
    actions[action_input]()
else:
    print("Please enter a valid input.")

---

You should add the all the code under loop_menu = True, into a function, preferably named main, and then run main, like this:

if __name__ == "__main__":
    main()

See this Stackoverflow question for more details.

---

Finally, you should split up the logic in your while loop_menu == True: loop into separate functions. This should increase the clarity of your code.