New to Python and trying to make a game want to be sure im not doing anything that should be avoided

Question

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)

Answer 1

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

Answer 2

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.

Answer 3

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)

Answer 4

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.