HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMajor

Amateur RPG Fun

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
amateurfunrpg

Problem

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"

Solution

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 yo

Code Snippets

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)
response = get_input_in_list("New game or Load game?", ['load', 'new'])
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")
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")
if Class == "Warrior":
    Weapon = "Sword"
elif Class == "Archer":
    Weapon = "Bow"
else:
    Weapon = "Staff"
print("A " + Weapon + " is your weapon")

Context

StackExchange Code Review Q#70436, answer score: 28

Revisions (0)

No revisions yet.