Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Please suggest improvements/refactoring to this game to make it more idiomatic Python.

import random
import time

class Hero:
    def __init__(self):
        self.level = 1
        self.max_hp = 3
        self.hp = self.max_hp
        self.attack = 5 + self.level
        self.defense = 5 + self.level
        self.name = ''
        self.xp = 0

    def name_self(self):
        self.name = raw_input("What do you call yourself, anyway? ")
        if self.name == "":
            self.name_self()

    def heal_self(self):
        amount = self.xp * .2
        self.hp += amount
        print "You attempt to heal yourself..."
        time.sleep(1)
        print "You healed yourself for %d HP, but used half your XP. Feels good, man." % amount
        self.xp *= .5
        self.hp_limit()

    def hp_limit(self):
        if self.hp > self.max_hp:
            self.hp = self.max_hp

    def death(self):
        print "Sorry, %s, you is dead now." % (self.name)
        time.sleep(1)
        print "Well, aren't you lucky, there is an afterlife after all."

    def xp_up(self, xp):
        self.xp += xp
        print "You gain: %s XP" % xp

    def look_self(self):
        print ("You are %s, not from around here.") % self.name
        print ("You are level %s with %s attack and %s defense.") % (self.level, self.attack, self.defense)
        print ("You need %s XP to level up.") % (self.level**2 * 10)

class Monster:
    def __init__(self, name):
        self.name = name
        self.hp = random.randint(2,10)
        self.attack = random.randint(2,5)
        self.defense = random.randint(2,5)
        self.xp = random.randint(2,8)

class Room:
    def __init__(self, key):
        self.room_data = {
            "home":{"description":"You're at home. This is where you live, unfortunately.",
                    "exits":["forest"]},
            "forest":{"description":"You're in a dark forest. It's fairly gloomy.",
                    "exits":["home", "lake"]},
            "lake":{"description":"You see a lake circled by rocks. It's too cold to swim.",
                    "exits":["forest","mountain"]},
            "mountain":{"description":"You can see for miles around. Don't fall off.",
                    "exits":["lake"]},
            }
        self.description = self.room_data[key]["description"]
        self.exits = self.room_data[key]["exits"]
        self.name = str(key)
        self.monster_list = {}

class Game:
    def __init__(self):
        self.command_list = ["look","name","fight","heal",'report','move','?']
        self.hero = Hero()
        self.current_room = Room("home")


    def list_commands(self):
        print 'Commands are', ', '.join(self.command_list[:-1]), 'and', self.command_list[-1] + '.'

    def handle_input(self):
        com = raw_input(self.prompt()).lower().split()
        if len(com) < 1:
            print ("Huh?")
        elif com[0] == "fight":
            if len(com) > 1:
                if com[1] in self.current_room.monster_list:
                    self.combat(self.hero, self.current_room.monster_list[com[1]])
            else:
                print "Fight what?"
        elif com[0] == "report":
            self.hero.look_self()
        elif com[0] == "look":
            if len(com) > 1:
                if com[1] in self.current_room.monster_list:
                    self.look_monster(self.current_room.monster_list[com[1]])
                else:
                    print "You don't see that monster."
            else:
                self.look()
        elif com[0] == "move":
            if len(com) > 1:
                self.move(com[1])
            else:
                print "Move where?", "You can exit to: %s" % ', '.join(self.current_room.exits)
        elif com[0] == "name":
            self.hero.name_self()
        elif com[0] == "info":
            self.info()
        elif com[0] == "heal":
            self.hero.heal_self()
        elif com[0] == "?":
            self.list_commands()
        else:
            print ("lol wut")

    def combat(self, attacker, defender):
        while defender.hp > 0 and attacker.hp > 0:
            attack = int(random.random() * attacker.attack)
            defense = int(random.random() *defender.defense)
            print "Attack: %s vs Defense: %s" % (str(attack), str(defense))
            if attack > defense:
                print "You hit the %s for %s HP." % (defender.name.capitalize(), str(attack))
                defender.hp -= attack
            elif attack == defense:
                print "The attack missed. You feel kind of disappointed."
            else:
                print ("The %s hit you for %s HP and it hurt real bad.") % (defender.name.capitalize(), str(attack))
                self.hero.hp -= attack
                if self.hero.hp < 2:
                    print "You attempt to escape..."
                    time.sleep(1)
                    break
            time.sleep(.5)
        if defender.hp < 1:
            print "You killed the %s. How sad for the %s's family." % (defender.name.capitalize(), defender.name.capitalize())
            self.hero.xp_up(defender.xp)
            del self.current_room.monster_list[defender.name]
        if self.hero.hp < 1:
            self.hero.death()


    def level_up(self):
        if self.hero.xp > self.hero.level**2 * 10:
            self.hero.level += 1
            print "You've reached level " + str(self.hero.level)
            self.hero.max_hp += self.hero.level
            self.hero.hp = self.hero.max_hp

    def populate(self):
        for i in range(self.hero.level):
            new_monster = random.choice(["ogre", "orc", "goblin"])
            self.current_room.monster_list[new_monster] = Monster(new_monster)

    def look(self):
        print self.current_room.description
        print "You can exit to: %s" % ', '.join(self.current_room.exits)
        monster_list= []
        for name in self.current_room.monster_list:
            monster_list.append(self.current_room.monster_list[name].name.capitalize())
        if monster_list:
            print "You see: %s" % ', '.join(monster_list)

    def look_monster(self, monster):
        print ("The %s has %s HP, %s attack, %s defense, and is worth %s XP."
            % (monster.name.capitalize(), monster.hp, monster.attack, monster.defense, monster.xp))

    def move(self, exit):
        if exit in self.current_room.exits:
             self.current_room = Room(exit)
             self.populate()
             self.look()
        elif exit == self.current_room.name:
            print ("You're already here.")
        else:
            print ("You can't get there from here.")

    def update(self):
        self.level_up()
        if self.hero.hp <= 0:
            self.hero.death()
            time.sleep(2)
            game = Game()

    def prompt(self):
        return '\n' + self.hero.name + " HP:" + str(int(self.hero.hp)) + " XP:" + str(self.hero.xp) + " >"

    def output(self):
        pass

game = Game()

game.populate()
print ("Welcome, adventurer.")
game.list_commands()
game.hero.name_self()
game.hero.look_self()
game.look()

while True:
    game.handle_input()
    game.update()
    game.output()
share|improve this question
    
Line 183 doesn't work? If so, we cannot help with that here as it's off-topic. –  Jamal Jul 12 at 20:14
    
Thanks removed off topic question –  MattO Jul 12 at 20:22
    
You may also remove that line from the code (assuming everything else will still work), otherwise the community may close the question anyway. –  Jamal Jul 12 at 20:29

2 Answers 2

up vote 6 down vote accepted

A few suggestions:


Simplify and reduce duplication using inheritance, for example:

class Character(object): # note new-style classes

    def __init__(self, name, hp, attack, defence, xp):
        self.name = name
        self.hp = hp
        self.attack = attack
        self.defence = defence
        self.xp = xp


class Monster(Character):

    def __init__(self, name):
        super(Monster, self).__init__(name, random.randint(2,10),
                                      random.randint(2,5), 
                                      random.randint(2,5),
                                      random.randint(2,8))


class Hero(Character):

    MAX_HP = 3

    def __init__(self, level=1):
        super(Hero, self).__init__('', self.MAX_HP, level+5, 
                                   level+5, 0)
        self.level = level

Move general class data out of the instance:

class Room(object):

    ROOM_DATA = {"home": {"description": "You're at home. This is where you live, unfortunately.",
                          "exits": ["forest"]},
                 "forest": {"description": "You're in a dark forest. It's fairly gloomy.",
                            "exits": ["home", "lake"]},
                 "lake": {"description": "You see a lake circled by rocks. It's too cold to swim.",
                          "exits": ["forest", "mountain"]},
                 "mountain": {"description":"You can see for miles around. Don't fall off.",
                              "exits": ["lake"]}}

    def __init__(self, key):
        self.description = self.ROOM_DATA[key]["description"]
        self.exits = self.ROOM_DATA[key]["exits"]
        self.name = str(key)
        self.monster_list = {}

Similarly e.g. Game.COMMAND_LIST isn't instance-specific.


Simplify the display using "magic methods" and str.format:

class Monster(Character):

    ...

    def __str__(self):
        template = "The {0} has {1.hp} HP, {1.attack} attack, ..." # etc.
        return template.format(self.name.capitalize(), self)


class Game(object):

    ...

    def look_monster(self, monster):
        print str(monster)

Use looping rather than recursion for input validation (see e.g. this SO community wiki):

def name_self(self):
    while True:
        self.name = raw_input("What do you call yourself, anyway? ")
        if self.name != "":
            break

Consider structuring your functions differently, such that they call each other, to cut down multiple lines of calls at the top level. For example: Game.__init__ could call populate and look; Hero.__init__ could call name_self and look_self(); and you could add Game.play_round to call handle_input(), game.update() and game.output().


Put the top-level code in an if __name__ == '__main__': block (see e.g. this SO question).

share|improve this answer

The other answer from @jonrsharpe addresses the most important points, but a few other nits are:

Eliminate the "info" choice in handle_input

If the user types in "info" at the command line, the reward is an error message and the end of the program.

Fix the calculating and reporting in heal_self

The message starting with "You healed your self for %d HP..." is a bit misleading since that reports the previously calculated amount which is calculated as self.xp * .2 rather than the actual number which is self.xp *= .5 and is capped by self.hp_limit()

Consider replacing "magic numbers" with named parameters

At some point, you might wish to change, say, how "tough" monsters can be, so rather than having self.attack = random.randint(2,5) you may want to put the 2 and 5 into variables that may be tuned or even altered during the game. For example, maybe mountain Ogres are tougher than forest Ogres, or perhaps monsters get tougher as the player does.

Start fractional numbers with "0."

It's really easy to overlook the difference between "2" and ".2" so it's common practice in many engineering disciplines to write "0.2" instead of ".2" just to avoid that error in reading.

Consider the user

The user is told when he or she advances to the next level, but has to use the "report" command to find out how many points to the next level. Why not also add the points-to-next level to the reporting in level_up? Also, if the user types "move" without a destination, the program prints the list of possible destinations. That's good design. Maybe the program could also list monsters if the user types "fight" without naming a monster.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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