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.

As the title says, I am just learning to code in python and have been reading the book "Learn python the hard way". In chapter 45 he basically says: "Ok, go figure out how to make a text based game" ... So before I get too far in this I just want to be sure I'm not doing anything stupid. The code as it is is fairly functional just not too many rooms etc :p

I am running this under windows.

from sys import exit
from random import randint
import curses,random
screen = curses.initscr()
screen.border(0)
width   = screen.getmaxyx()[1]
height  = screen.getmaxyx()[0]
size    = width*height
char    = [" ", ".", ":", "^", "*", "x", "s", "S", "#", "$"]
b       = []

curses.start_color()
curses.init_pair(1,0,0)
curses.init_pair(2,1,0)
curses.init_pair(3,3,0)
curses.init_pair(4,4,0)
global x
x = 1

def cls():
    screen.clear()
    screen.border(0)
    screen.addstr(20,1,">")
    screen.refresh()

def info_special(message,message2):
    screen.move(19,x)
    screen.clrtoeol()
    screen.addstr(19,x,message,curses.A_BOLD)
    screen.addstr(message2)
    screen.move(20,3)
    screen.clrtoeol()
    screen.border(0)
    screen.refresh()

def info(message):
    screen.move(19,x)
    screen.clrtoeol()
    screen.addstr(19,x,message)
    screen.move(20,3)
    screen.clrtoeol()
    screen.border(0)
    screen.refresh()

def help():
    screen.move(19,x)
    screen.clrtoeol()
    string = "Commands:"
    for j in commands:
        string = "%s\n\t %s" % (string,j)
    screen.addstr(10,x,string)
    screen.move(20,3)
    screen.clrtoeol()
    screen.border(0)
    screen.refresh()

def intro():
    curses.curs_set(0)
    screen.clear
    welcome = """_  _  ____  __     ___  __   _  _  ____ 
/ )( \(  __)(  )   / __)/  \ ( \/ )(  __)
\ /\ / ) _) / (_/\( (__(  O )/ \/ \ ) _) 
(_/\_)(____)\____/ \___)\__/ \_)(_/(____)
"""
    dragon = """                                    ______________
                              ,===:'.,            `-._
                                   `:.`---.__         `-._
                                     `:.     `--.         `.
                                       \.        `.         `.
                               (,,(,    \.         `.   ____,-`.,
                            (,'     `/   \.   ,--.___`.'
                        ,  ,'  ,--.  `,   \.;'         `
                         `{D, {    \  :    \;
                           V,,'    /  /    //
                           j;;    /  ,' ,-//.    ,---.      ,
                           \;'   /  ,' /  _  \  /  _  \   ,'/
                                 \   `'  / \  `'  / \  `.' /
                                  `.___,'   `.__,'   `.__,'
"""
    for i in range(size+width+1): b.append(0)

    while 1:
        for i in range(int(width/9)): b[int((random.random()*width)+width*(height-1))]=65
        for i in range(size):
                b[i]=int((b[i]+b[i+1]+b[i+width]+b[i+width+1])/4)
                color=(4 if b[i]>15 else (3 if b[i]>9 else (2 if b[i]>4 else 1)))
                if(i<size-1):   screen.addstr(  int(i/width),
                                                i%width,
                                                char[(9 if b[i]>9 else b[i])],
                                                curses.color_pair(color) | curses.A_BOLD )

        screen.addstr(0,1,welcome,curses.color_pair(3))
        screen.addstr(dragon,curses.color_pair(4))
        screen.refresh()
        screen.timeout(30)
        if (screen.getch()!=-1): break
    screen.timeout(0)
    curses.curs_set(1)

class Monster(object):

    def __init__(self,name,strength,location):
        self.sname = name.split(' ')[0]
        self.name = name
        self.strength = strength
        self.location = location

class Weapon(object):

    def __init__(self,name,strength, location):
        self.sname = name.split(' ')[0]
        self.name = name
        self.strength = strength
        self.location = location

class Character(object):

    def __init__(self,name,location):
        self.name = name
        self.location = location
        self.inv = []

    def look(self):
        cls()
        place = self.location
        string = "Current Room:"
        screen.addstr(1,x,string,curses.A_REVERSE)
        string = " %s" % place.name
        screen.addstr(string)
        description = place.description.split('\n')
        i = 2 # starting line
        for line in description:
            screen.addstr(i,x,line)
            i += 1
        try:
            items = place.items
            for item in items:
                string = "The %s is %s" % (item.name,item.location)
                screen.addstr(i+1,x,string)
        except:
            pass
    def inventory(self):
        i = 10
        while i <= 18:
            screen.move(i,x)
            screen.clrtoeol()
            i += 1

        screen.move(19,x)
        screen.clrtoeol()
        screen.addstr(9,x,"Your inventory contains:")
        i = 10 #starting line for list
        curses.init_pair(1, curses.COLOR_RED, curses.COLOR_BLACK)
        if not len(self.inv):
            string = '\tNOTHING!'
            screen.addstr(i,x,string,curses.color_pair(1))
        else:
            for item in self.inv:
                string2 = item.name.split(' ')
                string1 = "\t%s " % string2.pop(0)
                string2 = ' '.join(string2)
                screen.addstr(i,x,string1,curses.color_pair(1))
                screen.addstr(string2)
                i += 1
        screen.move(20,3)
        screen.clrtoeol()
        screen.border(0)
        screen.refresh()

class Room(object):

    def __init__(self,name,description):
        self.name = name
        self.description = description
        self.paths = []
        self.items = []

Start = Room('Start',"""
You are in a dark room.
There is a door to the West and East.
""")
BearRoom = Room('Bear Room',"""
There is a bear!
""")
TreasureRoom = Room('Treasure Room',"""
Treasure has Been Stolen.
The room is empty!
""")
Sword = Weapon('Sword of Truth',5, 'Propped up against the wall')
Lantern = Weapon('Lantern',100, 'In Inventory')
#[n,w,s,e]
Start.paths = [0,BearRoom,0,TreasureRoom]
Start.items = []
BearRoom.paths = [0,0,0,Start]
BearRoom.items = [Sword]
TreasureRoom.paths = [0,Start,0,0]
TreasureRoom.items = []

character = Character('Bob Loblaw',Start)
character.inv = [Lantern] #starting items

commands = ['go north,west,south, or east','take item','attack creature','drop item','(l)ook = Examine room','(i)nventory = get inventory','help = this list']
intro()
character.look()
while True:
    command = screen.getstr(20,3,80)
    command = command.split(' ')
    if command[0] == "go":
        try:
            if command[1] == "north":
                if character.location.paths[0] == 0:
                    string = "You cannot go that way."
                    info(string)

                else:
                    character.location = character.location.paths[0]
                    character.look()

            elif command[1] == "west":
                if character.location.paths[1] == 0:
                    string = "You cannot go that way."
                    info(string)

                else:
                    character.location = character.location.paths[1]
                    character.look()

            elif command[1] == "south":
                if character.location.paths[2] == 0:
                    string = "You cannot go that way."
                    info(string)

                else:
                    character.location = character.location.paths[2]
                    character.look()

            elif command[1] == "east":
                if character.location.paths[3] == 0:
                    string = "You cannot go that way."
                    info(string)

                else:
                    character.location = character.location.paths[3]
                    character.look()

            else:
                string = "I Don't Understand the direction: %s" % command[1]
                info(string)

        except:
            string = "I need a direction such as north,east,south or west."
            info(string)

    elif command[0] == "take":
            try:
                if command[1]:
                    if character.location.items:
                        i = 0
                        for item in character.location.items:
                            if command[1] in (item.sname,item.sname.lower()):
                                character.inv.append(item)
                                del character.location.items[i]
                                string = "%s " % item.name
                                string2 = "added to inventory"
                                info_special(string,string2)
                            else:
                                string = "Can't Find item: %s" % command[1]
                                info(string)
                            i += 1

                    else:
                        string = "There are no items to take in this room."
                        info(string)
            except:
                string = "Take What?"
                info(string)
    elif command[0] == "drop":
            try:
                if command[1]:
                    i = 0
                    for item in character.inv:
                        if command[1] in (item.sname,item.sname.lower()):
                            del character.inv[i]
                            character.location.items.append(item)
                            item.location = "On the floor."
                            string = "%s " % item.name
                            string2 = "dropped"
                            info_special(string,string2)
                        else:
                            string = "Can't Find item: %s" % command[1]
                            info(string)
                        i += 1
            except:
                string = "Drop What?"
                info(string)

    elif command[0] in ('inv','inventory','i'):
        character.inventory()

    elif command[0] in ("look","l"):
        character.look()

    elif command[0] in ('help','commands'):
        help()

    elif command[0] == "attack":
        try:
            pass
        except:
            pass

    elif command[0] in ("end","quit","exit","q"):
        screen.clear()
        screen.refresh()
        curses.endwin()
        print "Sorry to see you go so soon."
        exit(0)
    else:
        string = "I Dont Understand the command: %s" % command[0]
        info(string)
share|improve this question

migrated from stackoverflow.com Dec 30 '12 at 12:39

This question came from our site for professional and enthusiast programmers.

    
Comments are always nice. color=(4 if b[i]>15 else (3 if b[i]>9 else (2 if b[i]>4 else 1))) isn't nice, nor are try and if statements that are nested to a depth of five. Try condensing your repetitive code with functions and add some comments. –  Blender Dec 30 '12 at 9:38
    
Trying to get your code reviewed as you start out is hardly learning Python the hard way. It's essential that you made some mistakes. –  martineau Dec 30 '12 at 11:05
    
@Blender Yea im always bad at commenting code.. trying to improve appreciate you pointing it out to remind myself yet again as far as the code in intro goes its a ANSI animation so its going to look messy no matter how its written lol –  Dreded Dec 30 '12 at 11:44
    
if you are going to use ascii art in your code, you should probably consider using a library of text documents you can read from instead of placing the directly into your code. actually, you should do that anyway, and have parts of the dialogue or game information on text documents that are read by the game when needed. –  Inbar Rose Dec 30 '12 at 13:59

4 Answers 4

up vote 2 down vote accepted

my main pointers are that the whole game should be a class, so you can have members (self.VARIABLE) and then not have to pass them on to too many functions.

and you should restructure your "commands" it should be a dictionary, where the key is the command (as the user inputs it) and then a list or another dict (or class) which would have the displayed command, and then a pointer to its method of what it does.

this way when you modify commands dynamically based on location, you dont need a function for each location. also it lets you make a more robust game.

there are a few other minor concerns, but they are mostly opinions, and not "mistakes" that you are making or anything. but try to read about making things more generic, and about class inheritance.

you requested an example:

def attack(args):
    print "recieved attack command with args: %s" % args

def move(args):
    print "recieved move command with args: %s" % args

def inspect(args):
    print "recieved inspect command with args: %s" % args

commands = {
    'attack': {'description': "makes an attack", 'display': 'Attack', 'function': attack},
    'move': {'description': "moves somewhere", 'display': 'Move', 'function': move},
    'inspect': {'description': "inspect something", 'display': 'Inspect', 'function': inspect}}



def show_commands():
    for command, specs in commands.items():
        print "%s: %s, %s" % (command, specs['display'], specs['description'])

def get_command():
    user_input = raw_input('choose a command: ')
    command, args = user_input.split(' ', 1)
    if command in commands:
        commands[command]['function'](args)
    else:
        print 'command not recognized'

simple usage:

>>> show_commands()
attack: Attack, makes an attack
move: Move, moves somewhere
inspect: Inspect, inspect something
>>> get_command()
choose a command: attack monster
recieved attack command with args: monster
share|improve this answer
    
if its not to much bother could you expand on what you mean about the commands? –  Dreded Dec 30 '12 at 11:53

I don't think having global at the module level makes sense.

You should have it in a function/method if you need, but I would advise against having global variables at all, especially undocumented and called x.

Also try to avoid except: without specifying an exception class: it can suppress much more exceptions than you think can occur. Specify the kind of exception you actually are processing, like: except AttributeError:.

P.S. This question belongs on CodeReview, but it's OK, the mods can handle it.

share|improve this answer
    
there was no need for the global not sure what I was thinking at the time, thanks –  Dreded Dec 30 '12 at 11:55
    
I will definitely lookup more info on try and except I was mainly using try as a way to make sure that commands where entered is there a better way to do this? for example try: if command[1]: only exists to make sure that command[1] exists without crashing the program. –  Dreded Dec 30 '12 at 12:00
    
don't know why I didnt do this before but I removed the try/except and replaced with if len(command) > 1: to check for enough parameters... makes a lot more sense :p –  Dreded Dec 30 '12 at 12:09
    
@Dreded Both make sense, it's a viable use of try, but you need to avoid catching all exceptions if you don't mean it. except IndexError is fine for the case of retrieving a list item. –  Lev Levitsky Dec 30 '12 at 12:40

Well regarding the calling of all the different commands you might want to first of all wrap the whole thing into a class and then define the commands in different methods like this:

def command_go(self, direction, *args):
    # do something here
    # ...

def command_take(self, item, *args):
    # take the item or whatever

# And so on ...

And then when they need to be called do something like the following:

command = screen.getstr(20,3,80)
command = command.split(' ')
method = "command_" + command[0] # name of the method to be called for this command
if hasattr(self, method):
    # The method does exist, so call it using the command's args
    getattr(self, method)(*command[1:])

else:
    string = "I Dont Understand the command: %s" % command[0]
    info(string)
share|improve this answer

First off, I love the animation at the start!

  • char and b don't need to be global, they're only used in the flame animation. You also need to pick better names, say flame_animation_parts, top_line.

  • Use lowercase letters for the start of variables/functions (Lantern, Start etc), Uppercase for classes.

  • The entire main loop of the program is in the global namespace - it's a very good idea to separate this out in a function (say, playgame) and use the following idiom:

    if __name__ == "__main__":
        playgame()
    

    If nothing else, it will stop the program running if you import it. It also highlights that you have relevant code both at the start and the end of the file.

  • I'm also going to suggest wrapping the playgame call into an try...except block so the screen can be cleaned up if an error occurs. Note that I'm catching everything, which shouldn't be done normally (as Lev's advice), but I'm reraising it afterwards.

    if __name__ == "__main__":
        try:
            playgame()
        except:
            curses.endwin()
            raise
    
  • The main loop runs continually, without pausing for any input!!!! This suggests that you haven't actually run it... You may want to have a look at the curses.textpad module and use this for input (turn echoing off!).

  • The command should be 'normalised' to lowercase, to avoid checking against uppercase forms.

  • Since your commands are (mostly) verb noun, rather than checking against command1 in a try block, try something like:

    verb = command[0] if len(command) > 0 else ""
    noun = command[1] if len(command) > 1 else ""
    
  • The 'go' command uses repeated code for each direction. you might want to consider either having a dictionary translating word to direction,

    NORTH = 0
    WEST = 1
    SOUTH = 2
    EAST = 3
    
    dir_to_offset['west'] = WEST # etc
    

    or (preferably, imho) having the location.paths as a dictionary itself. This can then be referenced directly by the command.

    start.paths = { 'west': BearRoom, 'east': TreasureRoom }
    
  • You may want to add paths and items parameters to the Room constructor, to avoid adding them in later. But the paths in particular will mean changing how you deal with them (can't use objects directly).

  • When you start, you have a blank screen without any description!

  • There are lots of other "magic numbers" around - these need to be minimised, otherwise it's likely you'll cause a very hard to track error. Most of these are coordinates, so define constants and reference rows as offsets from these:

    ROOM_LINE = 2
    INVENTORY_START_LINE = 9
    INFO_LINE = 19
    COMMAND_LINE = 20
    

    Alternatively, you could look at using derwin and subwin to split the screen up into sections (command section, room description section etc), so after that all your coordinates are (0,0)

  • Once you've got it working and tidied up, try creating a class that will wrap around curses, so that your main program isn't cluttered up so much with curses calls.

share|improve this answer

Your Answer

 
discard

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