5
\$\begingroup\$

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()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 12, 2014 at 20:11
\$\endgroup\$
2
  • \$\begingroup\$ Line 183 doesn't work? If so, we cannot help with that here as it's off-topic. \$\endgroup\$ Commented Jul 12, 2014 at 20:14
  • \$\begingroup\$ You may also remove that line from the code (assuming everything else will still work), otherwise the community may close the question anyway. \$\endgroup\$ Commented Jul 12, 2014 at 20:29

2 Answers 2

6
\$\begingroup\$

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).

answered Jul 12, 2014 at 20:55
\$\endgroup\$
4
\$\begingroup\$

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.

answered Jul 12, 2014 at 22:01
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.