patternpythonMajor
Amateur RPG Fun
Viewed 0 times
amateurfunrpg
Problem
I am looking for this code to look neater, and if possible, less bulky. I am new to
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"
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 :
Even better, this looks a lot like what you did for "new game" or "load game". Did someone say "reusable components" ?
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 :
First, you could probably just write :
But better than than, you probably meant :
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.
And that's it. It is that simple.
Basic logic
There is not point in writing :
If the first condition is false, I guess the second condition has to be true. Just use a simple
Remove useless conversions
Because your code is so convoluted, you lose yourself and you tend to forget what the objects you are handling are.
Four strings are converted to ... strings.
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
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) * 10Integer 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.