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

Amateur RPG fun - follow-up

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

Problem

I have posted before, and the code was very bulky and ugly. Here is the new and improved version.

```
#RPG_TEST
#Dennis Gordick
#Brandon McCurry
#10/21/2014

#file system fully works, if you don't understand how to use it ask Brandon
"""
Task list:
allow player to leave game
create different types of monsters
improve shop inventory
gain skill points every level to improve yourself
add multiple save files
monster creator file-(Goal is to make a program that creates new monsters via user input, and is added to Enemies.py
"""
import random
import time
import pickle, shelve
from Enemies import Player, Monster, Shop_Keeper, Monster_Boss

class main():
def __init__(self):
response=self.valid_input("New game or Load game?",["load","new"])
if response == "load":
try:
f = shelve.open("save.dat")
attribute = f["attributes"]
person = attribute[0]
f.close()
print("Success!")
except:
print("Save file is corrupt or doesn't exist")
response="new"
if response=="new":
Name=input("What is your name?")
Race=self.valid_input("What is your race? (Human, Elf, Dwarf) ", ["human","elf","dwarf"])
#Race=input("What is your race? (Your choices are Human, Elf, and Dwarf.)")
Class=self.valid_input("What is your class? (Warrior, Archer, Mage) ", ["warrior","archer","mage"])
#Class=input("What is your class? (Your choices are Warrior, Archer, and Mage.")
person = Player(Name, Race, Class)

while person.health > 0:
person.update()
explore=self.valid_input("Do you want to explore, go to town, look at some info, or save? ", ["explore","town","save","info"])
#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

Solution

Basic style/layout stuff:

  • Python has a style guide - e.g. Shop_Keeper should be ShopKeeper, Player.Class should be Player.class_, whitespace (blank lines, etc.) should be reviewed.



  • It would be good to see some docstrings for your classes and methods. I like the Google style, but others are available.



You have an opportunity to do some real OOP and reduce duplication, here; all of your characters have e.g. lvl, xp, gold, so why not abstract out to:

class Character(object):

    def __init__(self, lvl, xp, gold, ...):
        self.xp = xp
        self.lvl = lvl
        self.gold = gold


Then your shopkeeper is:

class ShopKeeper(Character):

    def __init__(self, player, potions):
        potions = int(potions) * -1
        super(ShopKeeper, self).__init__(potions*10, ...)
        ...


Also, some of the calculated attributes would be easier as properties:

class Player(Character):

    @property
    def extra_health(self): 
        return self.lvl * 10


Now the value changes automatically if the Player's lvl increases.

It is not clear why main is a class. It seems to be used mainly as a collection of helper methods (e.g. valid_input, valid_int) which would be better packaged as simple functions - note that very few of the methods include references to self attributes or other methods. Also, using __init__ to run things is unconventional - if you did have some kind of main class, I would expect it to be called like:

if __name__ == "__main__":
    Main().run()


i.e. first instantiate the class, then call some method to run it. Note the guard to prevent this running if you from wherever import Main later on.

Your locations could be classes, too. For example:

class Tavern(Location):
    ...

if town == "tavern":
    player.enter(Tavern())


This allows you to make the town neater:

TOWN = {'tavern': Tavern, ...}

player.enter(TOWN[town]())


Some of the functionality could be moved to the Character classes, e.g.

class Player(Character):

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

    def __init__(self, name, race, class, ...):
        ...
        super(Player, self).__init__(lvl, xp, gold, ...)
        ...

    def fight(self, enemy):
        ...

    @classmethod
    def from_input(cls):
        name = input("What is your name?")
        race = valid_input("What is your race?", cls.RACES)
        class_ = valid_input("What is your class?", cls.CLASSES)
        return cls(name, race, class_, ...)


Note that this is neater if valid_input doesn't included the valid options in the prompt - this leads to duplication of the input data (e.g. valid_input("What is your race? (Human, Elf, Dwarf) ", ["human","elf","dwarf"]) includes the same data twice.)

Code Snippets

class Character(object):

    def __init__(self, lvl, xp, gold, ...):
        self.xp = xp
        self.lvl = lvl
        self.gold = gold
class ShopKeeper(Character):

    def __init__(self, player, potions):
        potions = int(potions) * -1
        super(ShopKeeper, self).__init__(potions*10, ...)
        ...
class Player(Character):

    @property
    def extra_health(self): 
        return self.lvl * 10
if __name__ == "__main__":
    Main().run()
class Tavern(Location):
    ...

if town == "tavern":
    player.enter(Tavern())

Context

StackExchange Code Review Q#71511, answer score: 11

Revisions (0)

No revisions yet.