8
\$\begingroup\$

I am working on a text-based adventure game and I'm implementing new features, one of these features being saves and loads. This code works, and I know that there is a more efficient way of coding it, but this is sufficient. I will first post the code (feel free to play with it, but bear in mind that this is only the beginning of the game, and I have not yet started on the fighting and weapons etc.), and I will then post the saving and loading functions.

Keep in mind that it creates the save file in the same directory as the .py, I will explain later how to save them and load them from a separate directory.

#imports
import random
import sys
import time
import pickle

#Player Class
class Player:
    def __init__(self, name):
        self.name = name
        self.maxhealth = 100
        self.health = self.maxhealth
        self.attack = 15
        self.money = 0
    def display(self, name):
        print("Name:",self.name,"\nHealth:",self.health,"/",self.maxhealth,"\nAttack Damage:",self.attack,"\nMoney:",self.money)
#Enemy Class
class Dragon:
    def __init__(self, name):
        self.name = name
        self.maxhealth = 150
        self.health = self.maxhealth
        self.attack = 5

#Start        
def main():
    print("Welcome to The Ultimate Dragon fighter RPG!")
    print("1] Start")
    print("2] Load")
    print("3] Profile")
    print("4] Exit")
    option = input("--> ").upper()
    if option == "1" or option == "S" or option == "START":
        nameInputAsk()
    elif option == "2" or option == "L" or option == "LOAD":
        load()
        nameInputAsk()
    elif option == "3" or option == "P" or option == "PROFILE":
        stats()
    elif option == "4" or option == "E" or option == "EXIT":
        print("Goodbye!")
        time.sleep(2)
        sys.exit()
    else:
        main()

#User Inputs IG Name
def nameInputAsk():
    print("Hello! What do you want your name to be?")
    option = input("--> ")
    global PlayerIG
    PlayerIG = Player(option)
    print("Oh nice! That's a cool name %s!" % PlayerIG.name)
    #time.sleep(2)
    print("Personally I would have gone for something like \'SuperAwesomeBattleFighter\' but I guess that's your loss.")
    nameChangeAsk()

#User changes name
def nameChangeAsk():
    name_input = input("Do you want to change your name?\n1] Yes\n2] No\n--> ")
    if name_input == "y" or name_input == "Y" or name_input == "yes" or name_input == "Yes" or name_input == "1" or name_input == "YES":
            option = input("Enter your new name:\n--> ")
            global PlayerIG
            PlayerIG = Player(option)
            print("%s. Yes, that's better; it has a nice ring to it." % PlayerIG.name)
            statInputAsk()
    #User answered "no"
    elif name_input == "n" or name_input == "N" or name_input == "no" or name_input == "No" or name_input == "2" or name_input == "NO":
        print("Okay. Keep in mind that you won't be able to change your name later!")
        statInputAsk()
    else:
        print("Sorry, that does not compute with me! Please try again!")
        nameChangeAsk()

def statInputAsk():
        check_input = input("Do you want to see your stats %s?\n1] Yes\n2] No\n--> " % PlayerIG.name)
        if check_input == "y" or check_input == "Y" or check_input == "yes" or check_input == "Yes" or check_input == "1":
            stats()
        elif check_input == "n" or check_input == "N" or check_input == "no" or check_input == "No" or check_input == "2":
            print("Oh. Okay then. That's cool. I'll ask again later if you want to see them.")
            userMultiChoice()
        else:
            print("Sorry, that does not compute with me! Please try again!")
            statInputAsk()

def stats():
    print("Name: %s" % PlayerIG.name)
    print("Attack Damage: %d" % PlayerIG.attack)
    print("Health: %i/%i" % (PlayerIG.health, PlayerIG.maxhealth))
    print("Money: %d" % PlayerIG.money)
    userMultiChoice()

def userMultiChoice():
    option = input("What do you want to do?\n1] See your blance. \n2] Go to the shop. \n3] Go on an adventure.\n--> ")
    if option == "1" or option == "balance" or option == "Balance" or option == "BALANCE":
        print("Money: %d" % PlayerIG.money)
    elif option == "2" or option == "shop" or option == "Shop" or option == "SHOP" or option == "store" or option == "Store" or option == "STORE":
        pass
    elif option == "3" or option == "adventure" or option == "Adventure" or option == "ADVENTURE" or option == "go" or option == "Go" or option == "GO":
        adventuringMarshMount()
    else:
        print("Sorry, that does not compute with me! Please try again!")
        userMultiChoice()

#Marshlands or Mountains - Random
def adventuringMarshMount():
    askSave()
    print("You walk outside your home and head down a path.")
    print("""You suddenly come across a fork in the path, and cannot make a decision
whether to go left or right to the marshlands or the mountains,
so you close your eyes and spin around and let fate decide...""")
    time.sleep(10)
    spinMarshMount = ""
    while spinMarshMount == "":
        print("You start to feel dizzy, so you slow down to a stop and open your eyes to see...")
        time.sleep(5)
        spinMarshMount = random.choice(["marsh", "mount"])
        if spinMarshMount == "marsh":
            print("the marshlands!")
            randomDragonAppear()
        else:
            print("the mountains!")
            time.sleep(1)
            print("You start to walk down the path to the mountains until...")
            time.sleep(3)
            randomDragonAppear()
        break

#Dragon, no Dragon - Random
def randomDragonAppear():
    print("you hear a crunch behind you")
    time.sleep(2)
    drag_appear = ""
    while drag_appear == "":
        print("You turn around and...")
        time.sleep(2)
        drag_appear = random.choice(["true", "false"])
        if drag_appear == "true":
            print("THERE IS A DRAGON!")
            dragonRunFight()
        else:
            print("the noise was only a Pigeon. Pesky things.")
        break


def askSave():
    ask = input("Do you want to save?\n--> ").upper()
    if ask == "Y" or ask == "YES":
        Save = Player(PlayerIG.name)
        #date = time.strftime("%d %b %Y", time.localtime())
        pickle.dump(Save, open("Save File", "wb"))
    elif ask == "N" or ask == "NO":
        print("Okay, maybe next time!")
        return
    else:
        print("Sorry, that does not compute with me! Please try again!")
        askSave()


def load():
    #date = time.strftime("%d %b %Y", time.localtime())
    me = pickle.load(open("Save File","rb"))
    me.display(Player.display)
    return dragonRunFight()


def dragonRunFight():
    print("You can choose to either turn and run like a coward or fight the majestic beast like a man.")
    fight = input("Run or Fight?:\n--> ").upper()
    if fight == "R" or fight == "RUN":
        print("You ran.")


main()

Player's class:

class Player:
    def __init__(self, name):
        self.name = name
        self.maxhealth = 100
        self.health = self.maxhealth
        self.attack = 15
        self.money = 0

The Saving Function:

def askSave():
    ask = input("Do you want to save?\n--> ").upper()
    if ask == "Y" or ask == "YES":
        Save = Player(PlayerIG.name) #Saves the desired class AND a chosen attribute
        pickle.dump(Save, open("Save File", "wb")) #Creates the file and puts the data into the file
#The file doesn't have an extension because it is a binary file and makes it easier to parse. 
    elif ask == "N" or ask == "NO":
        print("Okay, maybe next time!")
        return
    else:
        print("Sorry, that does not compute with me! Please try again!")
        askSave()

The Loading Function:

def load():
    me = pickle.load(open("Save File","rb")) #Loads the file
    me.display(Player.display) #Prints the stats out to verify the load has successfully happened
    return dragonRunFight()
\$\endgroup\$
11
\$\begingroup\$

Comments

You have #Player Class and #Enemy Class as comments. Comments should be used to clarify something that may otherwise be unclear. The line class Player: means that you are creating the Player class, so that comment does nothing.

Spacing

You need a little whitespace. Between the methods in the Player class, for example, I would add a blank line. Between the Player class and the Dragon class, I would add two blank lines. PEP 8, the official style guide for Python, agrees: https://www.python.org/dev/peps/pep-0008/#blank-lines.

Chained or

In a few places, you use if option == ... or option == ... or .... That's longer than it needs to be. You should take advantage of in (See also https://stackoverflow.com/q/15112125/5827958). For example:

if option in {"1", "S", "START"}:
    nameInputAsk()
elif option in {"2", "L", "LOAD"}:
    load()
    nameInputAsk()
...

Global variables

Global variables are evil. Note that global constants are okay, but if you're using the global keyword, it's probably a bad idea. A much better idea is to pass PlayerIG as an argument to whatever function needs it. In addition to avoiding the scourge of global variables, there is also a more clear advantage. You can now use these functions with multiple players.

Naming

According to PEP 8 (https://www.python.org/dev/peps/pep-0008/#function-names):

Function names should be lowercase, with words separated by underscores as necessary to improve readability.

mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

This is also true of "instance" variables (https://www.python.org/dev/peps/pep-0008/#method-names-and-instance-variables). That means PlayerIG, for example, would also follow this rule and become player_ig.

String formatting

In many places, you use the "old-style" formatting; i.e. %s, %d, etc. There has been a new style of formatting introduced that has actually been around since Python 2.6; i.e. .format(). Since Python 3.0, that is the preferred method (see https://docs.python.org/2/library/stdtypes.html#str.format), though the old style isn't actually deprecated (as pointed out by jpmc26). That means changing something like:

input("Do you want to see your stats %s?" % player_ig.name)

to:

input("Do you want to see your stats {}?".format(player_ig.name))

Even the %d ones don't need any special handling. If you really want, you can use {:d} to make sure it treats it as an integer, but when you are dealing with real integers (not a subclass), it looks the same either way.

Incorrect while loop

In adventuringMarshMount(), you have a while loop that is guaranteed to execute only once, which makes the loop useless. This is because you wrote break at the end of the loop. Whether spinmarshMount is/isn't "marsh"; whether or not it's blank, that loop will break. Come to think of it, it's okay if it executes only once. This isn't user input; it's a random choice, so it's guaranteed to come out with something we're okay with. You can just skip the loop.

The same thing is true of randomDragonAppear(). In that function, however, I have another problem:

true and false as strings

True and False are constants in Python. You might as well use them like that instead of coming up with your own constants. They even work better in the if check:

drag_appear = random.choice([True, False])
if drag_appear:
    print("THERE IS A DRAGON!")
    dragonRunFight()
else:
    print("the noise was only a Pigeon. Pesky things.")

That looks so clean now.

Saving

You have askSave() do the saving for you in addition to asking. What if you decide sometime that you'll save automatically every 5 minutes, say? You have no way of saving except either duplicating code or asking the user anyway. I would put the saving code in its own function, and then call that function from askSave(). Better, just have askSave() return True or False, and let the calling function decide what to do.

Recursion

The default maximum recursion limit is 1000. That means that someone who is trying to crash your program can say something other than one of the correct options 1000 times and you're program will have a nasty error. Maybe you don't care about people who are trying to mess it up, but you might as well keep that from happening. I don't like recursion in most cases. Sometimes it's unfeasable to use a loop, but generally I prefer a loop. For example, askSave() can look like this:

while True:
    ask = input("Do you want to save?\n--> ").upper()
    if ask in {"Y", "YES"}:
        save_game()
    elif ask in {"N", "NO"}:
        print("Okay, maybe next time!")
    else:
        print("Sorry, that does not compute with me! Please try again!")
        continue
    break
\$\endgroup\$
  • 2
    \$\begingroup\$ In addition to the note on string formatting: Python 3.6 introduced this handy inline notation: f"x = {x}, y = {y}" == "x = {}, y = {}".format(x, y) \$\endgroup\$ – gntskn Apr 4 '17 at 20:48
4
\$\begingroup\$

while loop is more intuitive than recursion

For example you can re-write:

def userMultiChoice():
    option = input("What do you want to do?\n1] See your blance. \n2] Go to the shop. \n3] Go on an adventure.\n--> ")
    if option == "1" or option == "balance" or option == "Balance" or option == "BALANCE":
        print("Money: %d" % PlayerIG.money)
    elif option == "2" or option == "shop" or option == "Shop" or option == "SHOP" or option == "store" or option == "Store" or option == "STORE":
        pass
    elif option == "3" or option == "adventure" or option == "Adventure" or option == "ADVENTURE" or option == "go" or option == "Go" or option == "GO":
        adventuringMarshMount()
    else:
        print("Sorry, that does not compute with me! Please try again!")
        userMultiChoice()

as

def userMultiChoice():
    option = input("What do you want to do?\n1] See your blance. \n2] Go to the shop. \n3] Go on an adventure.\n--> ")
    while True:
        if option == "1" or option == "balance" or option == "Balance" or option == "BALANCE":
            print("Money: %d" % PlayerIG.money)
            break
        elif option == "2" or option == "shop" or option == "Shop" or option == "SHOP" or option == "store" or option == "Store" or option == "STORE":
            # shop()
            break
        elif option == "3" or option == "adventure" or option == "Adventure" or option == "ADVENTURE" or option == "go" or option == "Go" or option == "GO":
            adventuringMarshMount()
            break
        else:
            print("Sorry, that does not compute with me! Please try again!")
            pass # Repeat loop

Iterative constructs are just more intuitive for the majority of people.

\$\endgroup\$
3
\$\begingroup\$

I would change a few things for the readability's sake, in addition to other things (e.g. globals).

While loop vs. recursion
Like other answers pointed out, use while loop over recursion when you can (especially in games, where the idea of a "main game loop" is very grounded). Recursion may be a more "elegant" solution in general, but while loop is definitely the more intuitive in most cases. Also in your case, you're now returning None after None after None... which is not only pointless but also potentially dangerous.

def askSave():
    while True:
        ask = input("Do you want to save?\n--> ").upper()
        if ask == "Y" or ask == "YES":
            Save = Player(PlayerIG.name) #Saves the desired class AND a chosen attribute
            pickle.dump(Save, open("Save File", "wb")) #Creates the file and puts the data into the file
            break # (or return works too)
            #The file doesn't have an extension because it is a binary file and makes it easier to parse. 
        elif ask == "N" or ask == "NO":
            print("Okay, maybe next time!")
            break
        else:
            # Basically, if the input is invalid the infinite loop never stops
            print("Sorry, that does not compute with me! Please try again!")

Comments
Your comments serve little to no purpose! Consider your saving function:

def askSave():
    while True:
        ask = input("Do you want to save?\n--> ").upper()
        if ask == "Y" or ask == "YES":
            Save = Player(PlayerIG.name) #Saves the desired class AND a chosen attribute
            pickle.dump(Save, open("Save File", "wb")) #Creates the file and puts the data into the file
#The file doesn't have an extension because it is a binary file and makes it easier to parse. 

The first may be OK - sometimes you do want to specify what this function with a clear name exactly does. But the second comment is useless - pickle.dump(Save, open()) alone tells you that the line is opening a file, then Saving it through pickling. Comments are good, but don't clutter your code with pointless ones! By the way, the third comment is really good. It should have been in the place of the second comment, because it explains why you're using pickle instead of redundantly describing what pickle does.

Text input options - not comprehensive enough YET not really readable
If you are going to give the players the option of free user input, you must account for a large variety of formats. And writing them all out is, while doable, messy. Here is a solution I'd use.

In a separate file, named responses.py, I would write the following lines:

option_yes = ['y', 'yes', 'yeah', 'yep']
option_no = ['n', 'no', 'nope']

And in the main script, always lower/upper the user input & check if the input is in the options. You actually did the forced-case trick in the askSave function, but a lot of your other functions are missing it.

def askSave():
    while True:
        ask = input("Do you want to save?\n--> ").lower()
        if ask in resp.option_yes:
            Save = Player(PlayerIG.name)
            # The file doesn't have an extension because it is a binary file 
            # and makes it easier to parse. 
            pickle.dump(Save, open("Save File", "wb"))
            break
        elif ask in resp.option_no:
            print("Okay, maybe next time!")
            break 
        else:
            print("Sorry, that does not compute with me! Please try again!")

Globals You used the global keyword exactly twice. While they are fine in a small program like yours, they become increasingly more difficult to maintain and error-prone as your program grows. Unless it is absolutely necessary, always avoid globals like the plague. Now, getting rid of them at this point of production is quite difficult; but since your structure is not very sound (as I have mentioned earlier, don't use recursion), I'll show you how I would change up the first half of the code.

import responses as resp

class Player:
    def __init__(self):
        self.name = None
        self.maxhealth = 100
        self.health = self.maxhealth
        self.attack = 15
        self.money = 0

    def display(self):
        print("Name: {0}\nHealth:{1}/{2}\nAttack Damage:{3}\nMoney:{4}\n".format(self.name, self.health, self.maxhealth, self.damage, self.money))

    def nameInputAsk(self):
        # Don't ask for a new name if the player already has a name
        if self.name: return "stats"

        print("Hello! What do you want your name to be?")
        name = input("--> ")
        print("Oh nice! That's a cool name %s!" % name)
        #time.sleep(2)
        print("Personally I would have gone for something like \'SuperAwesomeBattleFighter\' but I guess that's your loss.")
        self.name = name
        self.nameChangeAsk()

    def nameChangeAsk(self):
        option = input("Do you want to change your name?\n1] Yes\n2] No\n--> ")
        if option in resp.option_yes:
            name = input("Enter your new name:\n--> ")
            print("%s. Yes, that's better; it has a nice ring to it." % name)
            self.name = name
            return "stats"
        elif option in option_no:
            print("Okay. Keep in mind that you won't be able to change your name later!")
            return "stats"
        else:
            print("Sorry, that does not compute with me! Please try again!")
            return "start"

# TODO: Make more enemies!
class Dragon:
    def __init__(self, name):
        self.name = name
        self.maxhealth = 150
        self.health = self.maxhealth
        self.attack = 5

def welcome():
    print("Welcome to The Ultimate Dragon fighter RPG!")
    print("1] Start")
    print("2] Load")
    print("3] Profile")
    print("4] Exit")

    option = input("--> ").upper()
    return option

def main():
    # Initialize the class only once!
    you = Player()

    option = welcome()
    while True:
        if option in resp.option_start:
            option = you.nameInputAsk()      
        elif option in resp.option_load:
            option = load()
        elif option in resp.option_profile:
            option = stats()
        elif option in resp.option_exit:
            print("Goodbye!")
            time.sleep(2)
            break
        else:
            print("Try again!")

This way, there is only one big loop, and the flow of the game is controlled by one local variable option. Having to return option with every function and maintaining it is rather annoying though, and that's why we use frameworks and engines like Pygame, kivy, cocos2d-python, cocos2d-x, Unity, Unreal, etc.

Also, if I were you, I'd write all the messages in one big text file and parse them at game start-up. This way you can get rid of writing long strings directly in your game & make your code look cleaner. This would be done in a very similar manner to how I imported responses.py.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.