I'm new to coding and working on a text based game moving from room to room. The code works in pycharm but according to an instant feedback program I entered it into, it gave some tips on it and I am not exactly sure how how to improve upon it. Here is what it gave me:

  1. The Great Hall string is a key in main dictionary. Revise code so it is not a key in main dictionary.
  2. It said it can not classify my code (not sure what that means) do I need a def main() command?
  3. Consolidate multiple print commands into one function
  4. Make condition simpler by using non-complex conditions. With simple operators that don't look for a contain within the string 5.Better practice to use while True: and the break reserved word to stop the loop when you require it to stop

data setup

rooms = {'Great Hall': {'name': 'Great Hall', 'South': 'Bedroom', 'North': 'Dungeon', 'West': 'Library', 'East': 'Kitchen'},
     'Bedroom': {'name': 'Bedroom', 'East': 'Cellar', 'North': 'Great Hall'},
     'Cellar': {'name': 'Cellar', 'West': 'Bedroom'},
     'Library': {'name': 'Library', 'East': 'Great Hall'},
     'Kitchen': {'name': 'Kitchen', 'West': 'Great Hall', 'North': 'Dining Room'},
     'Dining Room': {'name': 'Dining Room', 'South': 'Kitchen'},
     'Dungeon': {'name': 'Dungeon', 'South': 'Great Hall', 'East': 'Gallery'},
     'Gallery': {'name': 'Gallery', 'West': 'Dungeon'},
directions = ['North', 'South', 'East', 'West']
current_room = rooms['Great Hall']

# game loop
while True:
   # display current location
   print('You are in the {}.'.format(current_room['name']))

  # get user input
  command = input('\nWhat direction do you want to go? ').strip()
  # movement
  if command in directions:
     if command in current_room:
         current_room = rooms[current_room[command]]
        # bad movement
        print("You can't go that way.")
  # Exit game
  elif command.lower() in ('q', 'quit'):
  # bad command
     print("I don't understand that command.")

1 Answer 1


Overall I don't think the code is that bad, considering what is the purpose; but if you want to expand your game I think the greatest issue is the data structure.

It'd be very easy to misplace some room or call it in a different way if you have to repeat yourself every time, so I suggest you to use a class to represent a Room:

class Room:
    name: str
    north: 'Room'
    east: 'Room'
    south: 'Room'
    west: 'Room'

    def __init__(self, name, north=None, east=None, south=None, west=None):
        self.name = name
        self.north = north
        self.east = east
        self.west = west
        self.south = south

        if north:
            north.south = self
        if east:
            east.west = self
        if south:
            south.north = self
        if west:
            west.east = self

    def go_to(self, direction):
        if direction in ['north','east','south','west']:
            return getattr(self, direction) 
            return None

    def __str__(self):
        return self.name

I'm using types because is very helpful to find out problems before it's too late, in Python if you have a recursive type (like Room) you need to use the quotes, but it just a syntactic notation.

By default a room does not have any north,east,south,west Room (hence the =None) but the important thing that happens in the __init__ is that when you add a room it automatically set the opposite direction. So if a room goes to another by east, the other room does the opposite by west. Thus we are reducing errors in directions. If this will not be the case for some room, you will be able to override that using another class (SelfLockingRoom that closes all the other directions when you enter, for example).

The __str__ method is just to handle the display of the room to the room itself; at one point you could have more than just the name to display.

I'm adding also a go_to method, it is the room responsibility to decide where to go given the direction; in the future you could have a TrapRoom that extends Room and in case of direction "East" will do nasty things, for example. The getattr is just to avoid having to write if direction=='north': return self.north but it would be the same and maybe even clearer.

Actually, if the game develops further, you may need more complex rules to decide what do to given a direction so probably you will need a House class:

class House:
    current_room: Room
    rooms: list[Room]  # at the moment is not used but could be in the future?

    def __init__(self, current_room: Room, rooms: list[Room]):
        self.current_room = current_room
        self.rooms = rooms

    def go_to(self, direction):
        if next_room := self.current_room.go_to(direction):
            self.current_room = next_room
        return next_room

At the moment is not very useful, but I think it's going to be.

To initialize the house you just create the rooms:

house_rooms = [cellar := Room('Cellar'), library := Room('Library'), dining_room := Room('Dining Room'),
               gallery := Room('Gallery'), bedroom := Room('Bedroom', east=cellar),
               dungeon := Room('Dungeon', east=gallery),
               kitchen := Room('Kitchen', north=dining_room),
               great_hall := Room('Great Hall', south=bedroom, north=dungeon, west=library, east=kitchen)]

house = House(current_room=great_hall, rooms=house_rooms)

(As you can see I'm not repeating names or directions)

The game loop is OK, we can collect some helpful methods, just in case you want to expand them in the future:

def prompt(text: str):

def ask(text: str) -> str:
    return input(text).strip()

and this would be your game loop (I'm always lowering the direction so you can write East east or EAST).

commands = {
    "directions": ['north', 'south', 'east', 'west'],
    "quit": ['q', 'quit']

def game_loop():
    prompt(f"\nYou are in the {house.current_room}")
    command = ask('\nWhat direction do you want to go? ').lower()
    if command in commands["directions"]:
        if not house.go_to(command):
            prompt("You can't go that way.")
    elif command in commands["quit"]:
        return False
        prompt("I don't understand that command.")
    return True

if __name__ == '__main__':
    while game_loop():
  • 1
    \$\begingroup\$ Plenty of good advice here, worthy of an upvote. But do not use __getattribute__() for simple situations where you just need to get an attribute by name. It's too low level, appropriate only under specialized circumstances. The right tool is getattr. \$\endgroup\$
    – FMc
    Apr 9, 2021 at 23:04
  • \$\begingroup\$ You're right! Thanks, I've edited the answer. \$\endgroup\$ Apr 9, 2021 at 23:14

Your Answer

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

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