Python text adventure

Question

I am creating a prototype for a text-adventure to get into game development, and i wanted to take the opportunity to also try to use OOP concepts, since i have been struggling to understand the OOP design.

Here is my code so far (warning of funky names):

class Character:
    def __init__(self, name, sex):
        self.name = name
        self.sex = sex
        self.inventory = {
            "albums": [],
            "instruments": []
        }
        self.coordinates = {
            'n': 0,
            'e': 0,
            's': 0,
            'w': 0
        }

    def get_inventory(self):
        print(self.inventory)

    def get_coordinates(self):
        print(self.coordinates)

    def str_coordinates(self):
        to_string = [str(v) for k, v in self.coordinates.items()]

        return ' '.join(to_string)

    def __str__(self):
        return  self.name

class Map:
    def __init__(self, *args):
        self.rooms = [*args]

    def get_room(self, room_coordinates):
        current_room = ''

        for room in self.rooms:
            if room.coordinates == room_coordinates:
                current_room = room

        return current_room

    def get_rooms(self):
        rooms_str = ''
        for room in self.rooms:
            rooms_str += room.coordinates + "\n"

        return rooms_str

class Room:
    def __init__(self, coordinates, name, actions, items, next_rooms):
        self.coordinates = coordinates
        self.name = name
        self.actions = actions
        self.items = items
        self.next_rooms = next_rooms

    def str_actions(self):
        return ', '.join(self.actions)

    def get_items_types(self):
        items_types = [item.item_type for item in self.items]
        return items_types

    def get_items_names(self):
        items_names = [item.name for item in self.items]
        return items_names

    def __str__(self):
        return self.name

class Item:
    def __init__(self, item_type, name):
        self.item_type = item_type
        self.name = name

class Album(Item):
    def __init__(self, item_type, name, lyrics):
        super().__init__(item_type, name)
        self.lyrics = lyrics

    def __str__(self):
        return self.item_type

ttng_animals = Album("Album", "Animals", 
                     "Let's talk about facts, the very best moment we have")

math_rock_3 = Album("Album", "Math rock 3", 
                    "Doesn't__init__ know the lyrics, play awesome guitar solo with tapping")

sort_of = Album("Album", "A test", 
                    "Doesn't__init__ know the lyrics, play awesome guitar solo with tapping")

standard_actions = ["look", "move", "coordinates", "inventory", "take"]

player = Character("Mabuelalelelejando", "male")

start = Room("0 0 0 0", "Start", standard_actions, [math_rock_3, ttng_animals, sort_of], ['n'])
room_2 = Room("1 0 0 0", "Room 2", standard_actions, [("instrument", "nya")] , ['s'])

game_map = Map(start, room_2)

def update_north():
    if player.coordinates['s'] > 0:
        player.coordinates['s'] -= 1
    else:
        player.coordinates['n'] += 1

def update_east():
    if player.coordinates['w'] > 0:
        player.coordinates['w'] -= 1
    else:
        player.coordinates['e'] += 1

def update_south():
    if player.coordinates['n'] > 0:
        player.coordinates['n'] -= 1
    else:
        player.coordinates['s'] += 1

def update_west():
    if player.coordinates['e'] > 0:
        player.coordinates['e'] -= 1
    else:
        player.coordinates['w'] += 1

def update_coordinates(movement):
    if movement.lower() == 'n':
        return update_north()
    if movement.lower() == 'e':
        return update_east()
    if movement.lower() == 's':
        return update_south()
    if movement.lower() == 'w':
        return update_west()

def ask_for_action():
    current_room = game_map.get_room(player.str_coordinates())
    print(current_room.str_actions())
    action_decision = input('').split()

    while action_decision[0] not in current_room.actions:
        print(current_room.str_actions())
        action_decision = input('').split()

    return action_decision

def move(*args):
    current_room = game_map.get_room(player.str_coordinates())
    directions = current_room.next_rooms

    print("You can move to: {}. A direction alsjeblieft:".format(', '.join(directions)))
    direction = input('')

    while direction.lower() not in directions:
        print("{}. A direction alsjeblieft:".format(directions))
        direction = input('')

    update_coordinates(direction)

def take(*args):
    current_room = game_map.get_room(player.str_coordinates())
    items_types = list(map(str.lower, current_room.get_items_types()))
    items_names = list(map(str.lower, current_room.get_items_names()))

    separated_items_names = []

    for item_name in items_names:
        for word in item_name.split():
            separated_items_names.append(word)

    if args:
        arg_in_name = []
        arg_not_in_name = set()
        for arg in args:
            if arg in separated_items_names:
                arg_in_name.append(arg)
            else:
                arg_not_in_name.add(arg)

        print(arg_in_name, arg_not_in_name)

        if len(arg_not_in_name) > 0 and len(arg_in_name) > 0:
            for i in range(0, len(items_names)):
                if arg_in_name[0] in items_names[i]:
                    item_index = i
            print("Do you want to take the {}?".format(items_names[item_index]))

        if len(arg_not_in_name) == 0 and len(arg_in_name) > 0:
            asked_items = set()
            for i in range(0, len(items_names)):
                for name in arg_in_name:
                    if name in items_names[i]:
                        asked_items.add(items_names[i])
                        item_index = i

            if len(asked_items) > 1:
                print("Do you want to take the {}?".format(items_names[item_index]))
            else:
                player.inventory["albums"].append(current_room.items[item_index])
                player.get_inventory()

actions = {
    "move": move,
    "take": take,
    "inventory": player.get_inventory,
    "coordinates": player.get_coordinates
}

def main():

    while True:
        action = ask_for_action()
        if action[0] in actions:
            args = action[1:]
            if len(args):
                actions[action[0]](*args)
            else:
                actions[action[0]]()
        else:
            print("Sorry please, alsjeblieft ):")

main()

Please, let me know about any way you think this can be further improved. I haven't introduced things like puzzles, so items are not very useful. I just implemented the take function.

Also, any suggestion to improve the OOP design on this game, as well as my understanding in OOP will be very appreciated.

Answer 1

Let's start from some small comments about your code :)

Character class

• get_invetory, get_coordinates - When you create a class and you want to have access to some private fields (we can say that your inventory field is something like a private field), then a popular technique is to create functions which are known as getter and setter. The typical convention is naming this function set_x/setX and get_x/getX. In your code, you created something like a getter (because you named this get_invetory), but this is not a normal getter (because you return nothing). Also there is another thing, in python creating getter/setter methods in the classic way is not so popular, because classes in python don't have something like private fields (maybe this is not exactly true, but this is not important now). Using fields of an object directly is not a problem (in common cases). So, I think that you should change get_invetory to either return something or maybe try to rename these methods.

class Map

• In the constructor, you convert from tuple to list. Why? Your code doesn't need that. This is not a bug/error, but just a bad code smell.

• get_room should return None when it find nothing, not the empty string.

class Room

• This is also not a bug, but I think that changing get_items_types to a one liner would improve the readability of this code.

Now, let's talk about how to improve this code to be more OOP.

The strength of Object Oriented Programming is not the fact that you can collect some variables in one object. The true power is that you can use polymorphism. In your code there is not a lot of use of that (because you use inheritance only once), so you don't get the big advantages of using OOP in this program.

But there is another thing.

The methods which change state of some object should be part of that object. For example, methods like update_north should be inside of the character class (because this is part of the implementation of moving your character). This is a great example of a method which should be part of a class. You should do the same for the take and move methods. Putting this method into the character class, you will be able to use self syntax, which will be very useful if you want to start using polymorphism in your code. Also you will be able to create more than one player and be able to move this character. Using a global variable like player is not good and should be the first thing to fix if you want to make this code more OOP.

Also:

• Missing unit tests ! ;) It is a very very important thing! Good units tests will save your time when you will be debugging or, what is more important, refactoring your code.

• Start using the logging module, not print :)

• If I correctly understand you code, you don't need 4 coordinates. You need only x and y :)

• The take method is too long and too complicated, in my opinion. Try to split this method into some smaller methods to improve readability.