Game code optimisation advice

Question

I'm wondering whether anybody would be willing to review this code for a game I'm working on with my friends. I feel quite confident with Python but am wondering whether there are some places that it could be made more efficient or errors I haven't spotted.

# Copyright 2015 Wet Ferret Studios

# Initialize Player Stats

playerstats = {
    'basehealth': 100,
    'name': 'Gerald Lagley',
    'inventory': {

    },
    'damage': 4,
    'equipped': {
        'weapon': {

        },
        'ring': {
            'wedding ring': {
                'name': 'Wedding Ring',
                'desc': 'A simple ring of gold that you received from your wife on your wedding day',
                'value': 20,
                'equippable': True
             }
        },
        'amulet': {

        },
        'armor': {

        },
        'armorclass': 0,
        'extrahealth': 0,
        'negativehealth': 0,
        'extradamage': 0,
        'negativedamage': 0,
        'negativearmor': 0,
        'totalhealth': 0,
    },
}

# Initialize Rooms
roomnumberint = 0
roomnumber = str(roomnumberint)
room = {
    '0': {
        'name': 'Temple Atrium',
        'items': {
            'noitems': False,
            'wooden stick': {
                'name': 'Wooden Stick',
                'desc': 'A wooden stick, could be useful',
                'value': 0,
                'equippable': True,
                'damagemult': 12,
                'type': 'weapon'
            },
        },
        'desc': 'Insert desc here',
        'entryinfo': 'Insert room desc here',
        'directions': {
            'n': True,
            's': False,
            'e': False,
            'w': False,
            'u': False,
            'd': False,
            'directions': ['N', 'S', 'E', 'W', 'U', 'D']
        }
    },
    '1000': {
        'name': 'Temple Antechamber',
        'items': {
            'noitems': True,
        },
        'desc': 'desc',
        'entryinfo': 'entryinfo',
        'directions': {
            'n': True,
            's': True,
            'e': True,
            'w': True,
            'u': False,
            'd': False,
            'directions': ['N', 'S', 'E', 'W', 'U', 'D']
        },

    },
    '2000': {
        'name': 'Temple Hallway',
        'items': {

        }
    }

}
roomtrapped = False
roomnoitems = room[roomnumber]['items']['noitems']
currentlyequipped = {
    'currentweapon': '',
    'currentring': '',
    'currentamulet': '',
    'currentarmor': '',
}


# Initialize Commands
def movefunc(direction):
    global roomnumberint
    global roomnumber
    if direction.lower() == 'n' and room[roomnumber]['directions'][direction.lower()] == True:
        roomnumberint += 1000
        roomnumber = str(roomnumberint)
    elif direction.lower() == 's' and room[roomnumber]['directions'][direction.lower()]:
        roomnumberint -= 1000
        roomnumber = str(roomnumberint)
    elif direction.lower() == 'e' and room[roomnumber]['directions'][direction.lower()]:
        roomnumberint += 10
        roomnumber = str(roomnumberint)
    elif direction.lower() == 'w' and room[roomnumber]['directions'][direction.lower()]:
        roomnumberint -= 10
        roomnumber = str(roomnumberint)
    elif direction.lower() == 'u' and room[roomnumber]['directions'][direction.lower()]:
        roomnumberint += 100000
        roomnumber = str(roomnumberint)
    elif direction.lower() == 'd' and room[roomnumber]['directions'][direction.lower()]:
        roomnumberint -= 100000
        roomnumber = str(roomnumberint)
    else:
        print('Please enter a valid direction')


def roomproperties():
    global roomnumber
    global roomnumberint
    global roomnoitems
    if room[roomnumber]['items']['noitems'] is True:
        roomnoitems = True
    else:
        roomnoitems = False


def examinefunc(item):
    global roomnumberint
    global roomnumber
    global roomnoitems
    if roomnoitems is False:
        print('Name: ' + room[roomnumber]['items'][item]['name'])
        print('Description: ' + room[roomnumber]['items'][item]['desc'])
        print('Value: ', end='')
        print(room[roomnumber]['items'][item]['value'], end='')
        print(' gold')
    elif roomnoitems is True:
        print('You look around the room but can\'t find a single item')
    else:
        assert isinstance(item, str)
        print('You look but fail to find ' + item)


def lookfunc():
    global roomnumber
    global roomnumberint
    global roomnoitems
    global room
    roomnumber = str(roomnumberint)
    print(room[roomnumber]['name'])
    print(room[roomnumber]['desc'])
    if roomnoitems is True:
        print('There are no items in this room')
    elif roomnoitems is False:
        print('Items you can see: ')
        for item in room:
            print('Name: ' + room[roomnumber]['items'][item]['name'])
            print('Description: ' + room[roomnumber]['items'][item]['desc'])
            print('Value: ', end='')
            print(room[roomnumber]['items'][item]['value'], end='')
            print(' gold')
    else:
        print('Error')


def userinput(func):
    global roomnumber
    global room
    if func == 'examine':
        examinefunc(input("What would you like to examine"))
    elif func == 'look':
        lookfunc()
    elif func == 'move':
        print('Valid Directions are: ')
        for items in room[roomnumber]['directions']['directions']:
            print(items + ': ', end='')
            print(room[roomnumber]['directions'][items.lower()])
        movefunc(input('Which direction would you like to go: '))
    else:
        print('Please enter a valid command')


def pickupfunc(item):
    global room
    global roomnumber
    global playerstats
    if item in room[roomnumber]['items']:
        if input('Do you want to pick up ' + item).lower() == 'yes' or 'y':
            playerstats['inventory'].append(room[roomnumber]['items'][item])
            room[roomnumber]['items'].remove(item)
        else:
            print('You look all around but utterly fail to find' + item)


def equipfunc(item):
    global room
    global playerstats
    global roomnumber
    global currentlyequipped
    if item in playerstats['inventory']:
        if playerstats['inventory'][item.lower()]['equippable']:
            if playerstats['inventory'][item.lower()]['type'] == 'weapon':
                for keys in playerstats['equipped']['weapon']:
                    playerstats['inventory'].append(playerstats['equipped']['weapon'][keys])
                playerstats['equipped']['weapon'].clear()
                playerstats['equipped']['weapon'].append(playerstats['inventory'][item.lower()])
                playerstats['inventory'].remove(item.lower())
                currentlyequipped['currentweapon'] = item.lower()
                print(playerstats['equipped']['weapon'][item.lower()]['name'] + ' has been equipped to your weapon slot')
            elif playerstats['inventory'][item.lower()]['type'] == 'ring':
                for keys in playerstats['equipped']['ring']:
                    playerstats['inventory'].append(playerstats['equipped']['ring'][keys])
                playerstats['equipped']['ring'].clear()
                playerstats['equipped']['ring'].append(playerstats['inventory'][item.lower()])
                playerstats['inventory'].remove(item.lower())
                currentlyequipped['currentring'] = item.lower()
                print(playerstats['equipped']['ring'][item.lower()]['name'] + ' has been equipped to your ring slot')
            elif playerstats['inventory'][item.lower()]['type'] == 'amulet':
                for keys in playerstats['equipped']['amulet']:
                    playerstats['inventory'].append(playerstats['equipped']['amulet'][keys])
                playerstats['equipped']['amulet'].clear()
                playerstats['equipped']['amulet'].append(playerstats['inventory'][item.lower()])
                playerstats['inventory'].remove(item.lower())
                currentlyequipped['currentamulet'] = item.lower()
                print(playerstats['equipped']['amulet'][item.lower()]['name'] + ' has been equipped to your amulet slot')
            elif playerstats['inventory'][item.lower()]['type'] == 'armor':
                for keys in playerstats['equipped']['armor']:
                    playerstats['inventory'].append(playerstats['equipped']['armor'][keys])
                playerstats['equipped']['armor'].clear()
                playerstats['equpped']['armor'].append(playerstats['inventory'][item.lower()])
                playerstats['inventory'].remove(item.lower())
                currentlyequipped['currentarmor'] = item.lower()
                print(playerstats['equipped']['armor'][item.lower()]['name'] + ' has been equipped to your armor slot')
            else:
                # Creating variable for failure notice as it exceeds character limit
                failure1 = "You rummage through the seemingly endless depths of your rucksack but fail to find"
                print(failure1 + item.lower)


def statscalc():
    global playerstats
    global currentlyequipped
    playerstats['damagemult'] = playerstats['equipped']['weapon'][currentlyequipped['currentweapon']]['damagemult'] + \
                            playerstats['basedamage'] - playerstats['negativedamage']
    playerstats['armorclass'] = playerstats['equipped']['armor'][currentlyequipped['currentarmor']]['armorvalue'] - \
                            playerstats['negativearmor']
    playerstats['totalhealth'] = playerstats['basehealth'] - playerstats['negativehealth'] + playerstats['extrahealth']

Answer 1

While reading your code I had one thought coming over and over:

Get yourself acquainted with Object-Oriented Programming.

Python allows you to build your own classes. Use that to your advantage.

Why?

It will remove constructions like:

def movefunc(direction):
    global roomnumberint
    global roomnumber

Which doesn't do what you think it does. If you'd make a Player class, that class could have a move function which is only allowed to move the current Player.

class Player:
    def __init__(self, room_number, stats):
        self.room_number = room_number
        self.stats = stats
    def move(self):
        # rewrite movefunc so it fits here

Where in the main of your program, you could create a Player like:

main_character = Player(0, {
'basehealth': 100
})

Something along those lines. Note that snake_case is the preferred method of writing function names in Python.

The moment you see so many global, you know your code won't survive in the long run. It's not maintainable and not extensible. Classes are your friend.

So what's the deal with global?

global is required if you want to modify a global variable inside a function's scope. You can read a variable without that keyword, but modifying it without one only modifies the local version of that variable. Basically, you're potentially shadowing variables with multiple versions of themselves depending on what local scope they're currently in.

This means you'll have to declare your variable is global in every function where you modify it. This could have it's cases in variables which aren't changed often, but I hope we can agree a room_number is a variable which is supposed to change. Keeping up your current method of writing will lead to big lists of variables at the top of each function, which is among other things considered needless repetition. Simplified your code looks like this and it will only get worse:

def foo():
    global w
    global x
    global y
    global z

def bar():
    global w
    global x
    global y
    global z

def faz():
    global w
    global x
    global y
    global z

Nobody wants their code to look like that. Re-factoring it now will prevent you from having to re-factor it when it looks worse.

Answer 2

This function has so many almost identical if statements, it's hard to follow. It would be far easier if you just did one test for the key existing at the start, like so:

def movefunc(direction):
    try:
        if not room[roomnumber]['directions'][direction.lower()]:
            print('Please enter a valid direction')
    except KeyError:
        print('Please enter a valid direction')

This attempts to test if room[roomnumber]['directions'][direction.lower()] is True, and if it's not prints the error. But it also prints the error if the user put input that doesn't match any of the directions.

You could test if the key exists by calling if direction.lower() in room[roomnumber]['directions']. But it's actually faster to use try except instead and it's more Pythonic. This is the "Easier to Ask Forgiveness than Permission" principle.

But why not make this even simpler? Instead of having a complicated arithmetic based system and storing booleans in room[roomnumber]['directions'][direction.lower()], just make each room's direction key have the ID of an actual room. Then you can directly set roomnumber, and your function becomes tiny:

def movefunc(direction):
    try:
        roomnumber = room[roomnumber]['directions'][direction.lower()]
    except KeyError:
        print('Please enter a valid direction')

When you're making functions, try and think of ways to improve the clarity and efficiency like this. If you have a long list of lines that seem very similar then you can probably improve it. Don't be afraid to change your data structure to make other code easier. Use Python's dynamic nature to make your code easier. Especially look at places like your equipfunc, where it's huge and clearly could be shortened a lot since so much of it is so similar.

Also about names, you can just call them move and equip. Python is designed to be readable, and usually it'll be clear from how you use the basic data types what something is. var["string"] is clearly a dictionary for example, so putting func or dict in a name is usually redundant or possible points to another problem if the usage is confusing enough that you do need to distinguish what it is.

Answer 3

Alongside @Mast's answer on the class structure, you should consider implementing a better inventory system.

Currently, you have a huge mess of inventory values that are empty and unused.

Consider a structure where the configurations are optional, and added if not present during configuration checking.

For example, instead of a structure like the following (with empty fields):

player = {
    'name': 'Quill',
    'occupation': 'Programmer',
    'skills': {

    },
    'inventory': {

    },
    ...
}

Add the empty fields when the config is added to the main class during initialisation.

So that way you can just pass in what you need:

player = {
    'name': 'Quill',
    'occupation': 'Programmer',
    'skills': [
        'Slayer of Dragons',
        'Talk to Dogs',
        'Make really good tacos'
    ],
    'inventory': {
        'Money': 1000000
        'Rep': 5000
    }
}

I would also consider taking a look at the Cactus project, a text-based game engine, with similar ideas.