16
\$\begingroup\$

I am looking for this code to look neater, and if possible, less bulky. I am new to def and to dictionaries, so I think throwing a few of those in here could shorten the code.

Here is the code for the RPG:

# RPG_TEST
#Dennis Gordick
#10/21/2014
"""
Task list:
fix bug where you can sell potions you don't have
allow player to leave battle
make battles more difficult
create different types of monsters
create a limit to the cave
add bosses
improve shop inventory
gain skill points every level to improve yourself
"""

import random
import time
import pickle
import shelve


response = input("New game or Load game? (Choose load or new)")
while response != "load" and response != "new":
    print(response + " is invalid input")
    response = input("New game or Load game? (Choose load or new)")

if response == "load":
    try:
        f = shelve.open("save.dat")
        attributes = f["attributes"]
        f.close()
        Name = attributes["Name"]
        Race = attributes["Race"]
        Class = attributes["Class"]
        Weapon = attributes["Weapon"]
        xp = attributes["xp"]
        player_lvl = attributes["player_lvl"]
        gold = attributes["gold"]
        potions = attributes["potions"]
    except:
        print("Save file is corrupt or doesn't exist")
        response = "new"
if response == "new":
    Name = input("What is your name?")
    Race = input("What is your race? (Your choices are Human, Elf, and Dwarf.)")
    Class = input("What is your class? (Your choices are Warrior, Archer, and Mage.")
    if Class == "Warrior":
        Weapon = "Sword"
        print("A " + Weapon + " is your weapon")
    elif Class == "Archer":
        Weapon = "Bow"
        print("A " + Weapon + " is your weapon")
    else:
        Weapon = "Staff"
        print("A " + Weapon + " is your weapon")

if Class == "Warrior":
    Weapon = "Sword"
    print("A " + Weapon + " is your weapon")
elif Class == "Archer":
    Weapon = "Bow"
    print("A " + Weapon + " is your weapon")
else:
    Weapon = "Staff"
    print("A " + Weapon + " is your weapon")

print("The " + str(Weapon) + " weilding " + str(Class) + " of the " + str(
    Race) + " clan, whent out on an adventure. There name was " + str(Name))

xp = 0
player_lvl = 1
extra_health = int(player_lvl) * 10
health = 90 + int(extra_health)
gold = 0
potions = 0

kill = 0
boss_kill = 0

while health > 0:

    extra_health = int(player_lvl) * 10
    health = 90 + int(extra_health)
    LVL_XP = 90 + int(extra_health)

    if int(xp) >= int(LVL_XP):
        player_lvl += 1
        print("Level Up! " + str(player_lvl))

    player_dmg_min = 0 + int(player_lvl)
    player_dmg_max = 7 + int(player_lvl)

    explore = input(
        "Do you want to explore or go to town or look at some stats/info or even save? (only say explore or town or info or save)")
    turns = 1
    if explore == "explore":
        lvl = input("What level monsters?")
        if lvl.isdigit():
            print("You explore")
            turns = 1
            while turns < 100 and int(health) > 0:

                monster_lvl = int(lvl)
                monster_dmg = int(lvl)
                monster_xp = int(lvl) / int(player_lvl) * 2
                monster_loot = int(monster_lvl)

                encounter = random.randint(1, 100)
                drop_lvl = int(lvl)

                #normal fight
                if int(encounter) >= 70:
                    print("You encounterd a LVL: " + str(monster_lvl) + " Monster!")
                    monster_health = int(monster_lvl) * 2
                    while int(monster_health) > 0 and health > 0:
                        print("Your Health: " + str(health))
                        print("Monsters Health: " + str(monster_health))

                        #Actual combat
                        attack = input("Do you attack or use a potion or run? (attack or potion or run)")

                        if attack == "attack":
                            hit = random.randint(1, 100)
                            if int(hit) <= 75:
                                dmg = random.randint(int(player_dmg_min), int(player_dmg_max))
                                monster_health = int(monster_health) - int(dmg)
                                print("\nYou did " + str(dmg) + " damage")
                            else:
                                print("You missed!")
                        elif attack == "potion":
                            if potions > 0:
                                health = 90 + int(extra_health)
                                print("Potions left... " + str(potions))
                            else:
                                print("You have no potions... You just waisted your turn!")

                        else:
                            print("You sit there and take it")
                        monster_hit_chance = random.randint(1, 100)
                        if int(monster_hit_chance) <= 60:
                            health = int(health) - int(monster_dmg)
                            print("The monster did " + str(monster_dmg) + " damage")
                        else:
                            print("The monster missed!")

                        #loot and xp for normal monster
                        if int(monster_health) <= 0:
                            xp = int(xp) + int(monster_xp)
                            print("\nThe monster died\n")
                            print("XP gained: " + str(monster_xp))
                            print("Your XP: " + str(xp))
                            loot_chance = random.randint(1, 100)

                            if int(loot_chance) < 10:
                                print("No loot :(")
                                print("Your gold " + str(gold))
                            elif int(loot_chance) < 70:
                                print("Your gold sir. It this many..." + str(monster_loot))
                                gold = int(gold) + int(monster_loot)
                                print("Your gold " + str(gold))
                            elif int(loot_chance) < 90:
                                print("Rare loot! 1 potoin!")
                                potions += 1
                                print("\nYour total potions " + str(potions))

                            kill += 1

                        elif int(health) <= 0:
                            print("You died")

                elif int(encounter) < 70:

                    loot = random.randint(1, 100)
                    trap = random.randint(1, 100)

                    if int(loot) >= 60:
                        gold = int(gold) + int(lvl)
                        print("You found " + str(lvl) + " gold")
                        print("You now have a total of " + str(gold) + " gold")
                    elif int(loot) <= 10:

                        if int(trap) >= 50:
                            health = int(health) - 10
                            print("You step on a trap")
                            print("You lost ten health")
                            print("Your total health is " + str(health))

                if int(turns) == 100:

                    #Boss fight

                    boss = random.randint(1, 10)

                    if int(boss) > 5:

                        print("Boss Fight!")
                        boss_health = int(health)
                        boss_xp = int(monster_xp) * 3
                        boss_dmg = int(lvl) * 3
                        boss_loot = int(lvl) * 100
                        run = input("Do you fight or run?")

                        while int(boss_health) > 0 and int(health) > 0 and run == "fight":

                            print("Your Health: " + str(health))
                            print("Boss Health: " + str(boss_health))
                            attack = input("Do you attack or use a potion? (attack or potion)")

                            if attack == "attack":

                                hit = random.randint(1, 100)

                                if int(hit) <= 75:

                                    dmg = random.randint(int(player_dmg_min), int(player_dmg_max))
                                    boss_health = int(boss_health) - int(dmg)
                                    print("\nYou did " + str(dmg) + " damage")

                                else:

                                    print("You missed!")

                            elif attack == "potion":

                                if potions > 0:

                                    health = 90 + int(extra_health)
                                    print("Potions left... " + str(potions))

                                else:

                                    print("You have no potions... You just waisted your turn!")

                            else:

                                print("You sit there and take it")

                            boss_hit_chance = random.randint(1, 100)

                            if int(boss_hit_chance) <= 60:

                                health = int(health) - int(boss_dmg)
                                print("The boss did " + str(boss_dmg) + " damage")

                            else:
                                print("The boss missed!")

                            if int(boss_health) <= 0:

                                xp = int(xp) + int(boss_xp)
                                print("\nThe boss died\n")
                                print("XP gained: " + str(boss_xp))
                                print("Your XP: " + str(xp))
                                loot_chance = random.randint(1, 100)

                                if int(loot_chance) < 10:
                                    print("No loot :(")
                                    print("Your gold " + str(gold))

                            elif int(loot_chance) < 90:

                                print("Your gold sir. It this many..." + str(boss_loot))
                                gold = int(gold) + int(boss_loot)
                                print("Your gold " + str(gold))

                            elif int(health) <= 0:

                                print("You died")

                            else:

                                print("Rare loot! 10 potoin!")
                                potions += 10
                                print("\nYour total potions " + str(potions))

                            boss_kill += 1

                print("End of turn " + str(turns) + "\n")
                turns += 1
                time.sleep(1.0)
        else:
            print("That isnt a lvl... your just not going to explore...")

    elif explore == "info":
        print("Total kills", str(kill))
        print("Total boss kills", str(boss_kill))
    #Going to town (giggity)

    elif explore == "town":
        town = input("Where do you want to go in town? (shop, inspector, blacksmith, tavern)")

        if town == "shop":
            print("Your gold " + str(gold))
            print("The shopkeep says 'We only have potions of health! They are 20 gold each!'")
            shop = input("How many do you want?")
            cost = int(shop) * 20

            if int(gold) >= int(cost):
                potions = int(potions) + int(shop)
                gold = int(gold) - int(cost)
                print("Gold left " + str(gold))
                print("Total potoins " + str(potions))

            else:
                print("'Your to poor! Come back with some gold fool!'\nThe shopkeeper kicks you out.")

        elif town == "inspector":
            print("Comeing soon")

        elif town == "blacksmith":
            print("Comeing soon")

        elif town == "tavern":

            print("Hello traveler, what can I do for you? A drink? Or the lates rumore?")
            bar_keep = input("Whats your choice? (drink, rumore, or leave)")

            if bar_keep == "drink":
                print("Drinks cost one gold.")
                drink = input("Do you want a drink?")

                if drink == "yes" and gold > 0:
                    gold = int(gold) - 1
                    print("Your gold: " + str(gold))
                    print("You get drunk out of your mind.")

                else:
                    print("Goodbye")

    elif explore == "save":
        #do you want extra_health or health saved?
        f = shelve.open("save.dat")
        attributes = {"Name": Name, "Race": Race, "Class": Class, "Weapon": Weapon, "xp": xp, "player_lvl": player_lvl,
                      "gold": gold, "potions": potions}
        f["attributes"] = attributes
        f.sync()
        f.close()
        print("Game saved")

And I also have a save file program named shelvingEx.py:

import shelve

#opens file
f = shelve.open("save.dat")
gold = 2
potions = "3"
#sets all variables as a dictionary
f["attributes"] = {"gold": gold, "potions": potions}
#f.sync() adds all any f["whatever"] to the file
f.sync()
#always close after use!!!
f.close()

#reopen file later to read the contents
f = shelve.open("save.dat")
#save the variables still in dictionary form to a new variable
attributes = f["attributes"]
#always close after use!!!
f.close()
#access each variable individually, and save them to a new variable to match the rest of your code.
gold = attributes["gold"]
print(gold)

Is there anything I can do to shrink the code? Or any improvements to the system?

For the actual game and the code, here is the link to the file.

\$\endgroup\$
4
  • 2
    \$\begingroup\$ Welcome to Code Review! That is one massive script, I'm sure you will get good reviews in order to improve and shorten it! \$\endgroup\$
    – Phrancis
    Nov 20, 2014 at 16:07
  • \$\begingroup\$ When I tried to run your code, I got issues with parenthesis at line 66. After I removed them, I got an indentation error at lin 77. \$\endgroup\$
    – SylvainD
    Nov 20, 2014 at 17:17
  • \$\begingroup\$ I reformatted it with PyCharm based on the github version. This way it actually works, and it also fixes some formatting issues that would have been code review material, well, forgive me \$\endgroup\$
    – janos
    Nov 20, 2014 at 17:48
  • \$\begingroup\$ Thank you to all you guys who edited my question and the code. Your helping me a lot. Note, this is not a class assignment, this is a free lance project for learning purposes. \$\endgroup\$ Nov 21, 2014 at 13:46

1 Answer 1

28
\$\begingroup\$

Your code is very big but this is not the real issue. The real issue is that your code is not made out of little reusable/understandable/testable/maintainable components (functions, classes, modules, etc).

But let's change things little by little.

Style

Python has a guide style called PEP 8. If you have no good reason not to follow it, just follow it. You'll find various tools to check your code (pep8, pep8online.com/, etc) and even to fix it more or less automatically (autopep8).

The list of problems is mostly about whitespaces but it's always good to know/fix before it becomes overwhelming (well, too late I guess).

Input validation

First thing I noticed as I tested your code is that it didn't bother checking my input. When asking whether I was a warrior, an archer or a mage, it would be good to check that I provide valid values.

It is quite easy to define a function to provide such a functionality :

RACES = ['human', 'elf', 'dwarf']
CLASSES = ['warrior', 'archer', 'mage']

def get_input_in_list(prompt, values):
    while True:
        s = input(prompt + '(Your choices are : ' + ', '.join(values) + ')')
        if s in values:
            return s

if response == "new":
    Name = input("What is your name?")
    Race = get_input_in_list("What is your race", RACES)
    Class = get_input_in_list("What is your class ?", CLASSES)

Even better, this looks a lot like what you did for "new game" or "load game". Did someone say "reusable components" ?

response = get_input_in_list("New game or Load game?", ['load', 'new'])

This can also be reused in other places but I'll let you deal with the pleasure of doing so.

Do not repeat yourself

Do not repeat yourself.

Do not repeat yourself.

Many things look wrong in :

if response == "new":
    Name = input("What is your name?")
    Race = get_input_in_list("What is your race", RACES)
    Class = get_input_in_list("What is your class ?", CLASSES)
    if Class == "Warrior":
        Weapon = "Sword"
        print("A " + Weapon + " is your weapon")
    elif Class == "Archer":
        Weapon = "Bow"
        print("A " + Weapon + " is your weapon")
    else:
        Weapon = "Staff"
        print("A " + Weapon + " is your weapon")


if Class == "Warrior":
    Weapon = "Sword"
    print("A " + Weapon + " is your weapon")
elif Class == "Archer":
    Weapon = "Bow"
    print("A " + Weapon + " is your weapon")
else:
    Weapon = "Staff"
    print("A " + Weapon + " is your weapon")

First, you could probably just write :

    if Class == "Warrior":
        Weapon = "Sword"
    elif Class == "Archer":
        Weapon = "Bow"
    else:
        Weapon = "Staff"
    print("A " + Weapon + " is your weapon")


if Class == "Warrior":
    Weapon = "Sword"
elif Class == "Archer":
    Weapon = "Bow"
else:
    Weapon = "Staff"
print("A " + Weapon + " is your weapon")

But better than than, you probably meant :

if Class == "Warrior":
    Weapon = "Sword"
elif Class == "Archer":
    Weapon = "Bow"
else:
    Weapon = "Staff"
print("A " + Weapon + " is your weapon")

as there is not point in doing things twice.

Data over code

Sometimes, you have to write a lot of code because "Hey, I have a lot of logic to write, I have to write code, that's the point of programming" but the point is more to keep things simple and to use the right tool (which is not always code) for the right thing.

An example in your code is how you get the default weapon from the class. Can't we just define a simple association between classes and weapons. Sure we can, we can do this with a simple dictionnary. Even better, we had defined a constant list with the different classes in it but we could reuse this as the dictionnary.

CLASSES = {'warrior': 'sword', 'archer': 'bow', 'mage': 'staff'}
...
Weapon = CLASSES[Class]

And that's it. It is that simple.

Basic logic

There is not point in writing :

           # normal fight
            if int(encounter) >= 70:
                ...
            elif int(encounter) < 70:

If the first condition is false, I guess the second condition has to be true. Just use a simple else.

Remove useless conversions

Because your code is so convoluted, you lose yourself and you tend to forget what the objects you are handling are.

print(
    "The " +
    str(Weapon) +
    " weilding " +
    str(Class) +
    " of the " +
    str(Race) +
    " clan, whent out on an adventure. There name was " +
    str(Name))

Four strings are converted to ... strings.

extra_health = int(player_lvl) * 10

Integer converted to integer (this one is all over the place).

Pretty much every single conversion you are performing is pointless.

User experience

When I read :

        print(
            "Hello traveler, what can I do for you? A drink? Or the lates rumore?")
        bar_keep = input("Whats your choice? (drink, rumore, or leave)")

        if bar_keep == "drink":
            print("Drinks cost one gold.")
            drink = input("Do you want a drink?")

            if drink == "yes" and gold > 0:
                gold = int(gold) - 1
                print("Your gold: " + str(gold))
                print("You get drunk out of your mind.")

            else:
                print("Goodbye")

I thought that this would be very confusing for any player : you are asked for something that might or might not be relevant.

First, I can choose "rumore" or "leave" but I have the feeling they do the exact same thing.

Then, I am asked if I want to drink before checking if I have enough to drink.

        bar_keep = get_input_in_list("What's your choice?", ['drink', 'leave']

        if bar_keep == "drink":
            print("Drinks cost one gold. You have " + str(gold))
            if gold > 0:
                if get_input_in_list("Do you want a drink?", ['yes', 'no']) == 'yes':
                    gold = int(gold) - 1
                    print("Your gold: " + str(gold))
                    print("You get drunk out of your mind.")
            print("Goodbye")

Many other things can be improved but the main thing would be to split your code into smaller parts.

\$\endgroup\$
7
  • \$\begingroup\$ (This is all in a happy tone. I'm very grateful) I was not the only person to code this mass of text... The Save game feature was someone else entirely. I coded everything else tho... So I had no reusable features, and I'm also still learning. I'm in a Tech school, learning Python. I wrote that before I had the Knowledge of Dictionaries and definitions. But thank you for your feed back. I will be updating the code with what you posted, and i will post that code as well. \$\endgroup\$ Nov 21, 2014 at 14:18
  • \$\begingroup\$ Just a note about the last bit: The barkeep wouldn't know how much gold you have. It makes perfect sense for him to ask even if you don't have money. What doesn't make sense is this conversation: "What's your choice?" drink "Drinks cost one gold. Want one?" yes "Goodbye" \$\endgroup\$
    – cHao
    Nov 21, 2014 at 14:18
  • \$\begingroup\$ I tried your code... It looks good, but it goes into an endless loop when it asks for my race... Any one know why? \$\endgroup\$ Nov 21, 2014 at 14:29
  • 1
    \$\begingroup\$ @AmateurProgramer: Keep in mind that people are reviewing the code, not the programmer. :) \$\endgroup\$
    – cHao
    Nov 21, 2014 at 14:29
  • \$\begingroup\$ @cHao: Yes... I know. But that was my explanation to why the code was different. The save file is well coded because it was coded by a more experienced python coder. I asked him for help because I had no clue on how to create a save game feature. \$\endgroup\$ Nov 21, 2014 at 14:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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