Cactus Reborn - A game engine for text-based adventure games

I'm currently working on reincarnation of the Cactus project, Cactus Reborn, and I've gotten the basic implementation so far. At this point, you can create simple, playable games.

There are currently three usable classes/functions:

• Location - This class describes data about a location in a GameFlowchart class, like a title, description, location references, etcetera.

• GameFlowchart - This class describes the "map", or "flowchart" that the player traverses while playing the game.

• play_game - This function runs the user-created game.

I'd like to know the following things:

• Is it appropriate to require the user to specify argument names with *?

• Where am I making things too complex? What can be simplified?

• Am I over-documenting?

• Are there any design issues that I should be concerned about here?

• Is there a simpler way to have the user describe data for games?

• Anything else?

location.py

```
"""
location.py

The classes and methods in this file are used to describe
a location, contained in a GameFlowchart class.
"""
import re
import sys
import time

class Location:
"""Represents a location in a GameFlowchart map.

A location represents a "location" on the game's
map, and contains data like a title, locations
to other positions, etc.

Keyword arguments:
title -- The title of the position.
description -- The description of the position.
locations -- A dictionary of possible inputs, and reference keys.
"""
def __init__(self, *, title, description_enter, description_exit, on_exit_function, locations):
self.title = title
self.description_enter = description_enter
self.description_exit = description_exit
self.on_exit_function = on_exit_function
self.locations = locations

def on_enter(self):
"""This function is run when the user enters.

When the user "enters" a positions, this function will
display the title, and position desc

Is it appropriate to require the user to specify argument names with *?

Yes! I think this is excellent practice, and in my opinion a far underused feature of Python 3. It greatly enhances the readability of your function when called.

Reading just the invocation of play_game() in your simple example, I can immediately work out what all of the arguments mean. That would be impossible if these arguments were specified positionally. I don’t think it imposes much burden on the caller, but it’s easier to read.

Where am I making things too complex? What can be simplified?

A few suggestions for things I’d tidy up:

-
It’s great that you enforce keyword-only arguments in the play_game() function; I would also add default arguments so that I can omit keywords that I’m not using. That can cut down on visual noise when the function is called.

-
The presence of on_enter() and on_exit() methods in the Location class make me twitchy. I feel like that’s just asking to be tidied up into a context manager, so that in main_game.py I can type:

with current_location as loc:
    loc.get_user_input(prompt, error_message, case_sensitive, global_commands)

That’s a bit more Pythonic, and eliminates the risk that I’ll forget the on_exit() method.

-
In the on_enter() method of Location, you can tidy up the for loop:

print(", ".join(self.locations.keys()))

That’s more Pythonic, shorter, and gets rid of the trailing comma.

Am I over-documenting?

I don’t think you’re over-documenting, but some of the docstrings are quite wordy. People can have an aversion to reading lots of text; you can probably convey the same information with less words. There are also places where the docstring is getting mixed up with implementation details.

For example:


Find, and obtain the value of a position, by reference.


This function checks to see if a reference value is in the dictionary of locations. If it is, then return the value, and if it isn't, then return None.

This docstring essentially describes how the function works. Really, all the information it should contain is something like:


Get the value of a position from a reference. Returns None if the position is not found.

Separately, I’d consider wording this to sound less like ‘get a value by reference’, just because that choice of words has me thinking of C.

Are there any design issues that I should be concerned about here?

Nothing jumps out at me, but it’s a Sunday evening and I’m tired.

At best, that’s an endorsement that there aren’t any glaring holes, not that the design is flawless.

Is there a simpler way to have the user describe data for games?

As stated above, don’t force me to specify every argument for every location – have some sensible defaults.

The structure of the function calls looks quite like JSON – perhaps you could allow the user to provide a JSON file (with a specified structure) that initialises the game.

Anything else?

Here are other miscellaneous suggestions:

-
In game_flowchart.py, the docstring describes the locations argument as “a dictionary of socks”. Perhaps I’m missing something, but I have no idea what a sock is in this context. Some more explanation would be useful.

-
In a couple of places, you check for a value in a dictionary before retrieving it. Rather than doing

if key_foo in dict_bar:
    return dict_bar[key_foo]
return

look at using the .get() method on the dictionary:

return dict_bar.get(key_foo)

The get() method looks up a key and returns a default if the key is missing; if no default is supplied, then it falls back to None.

-
In the GameFlowchart class, I think there’s a bug in your iterate_locations() method. I think you want to iterate over self.locations.items(), not self.locations. If I try the current implementation, I get a TypeError:

>>> from cactus import game_flowchart as gf
>>> cities = gf.GameFlowchart(locations={1: "london", 2: "basel", 3: "milan"})
>>> for city in cities.iterate_locations():
...     print(city)
...
Traceback (most recent call last):
  File "", line 1, in 
  File "/cactusgame/cactus/game_flowchart.py", line 26, in iterate_locations
    for key, value in self.locations:
TypeError: 'int' object is not iterable

-
There are a few slightly snarly regexes. I could unpick them, but it would be easier if there was a comment explaining what they’re supposed to do.

-
Here’s some slightly weird output:

---------
The Shire
As you enter the Shire, you are surrounded by the endless rolling hills.
Locations: mordor, laketown,
> narnia
Enter the correct input!
As you leave the Shire, you look back and wish that you could stay longer.
--------------------------------------------------------------------------

---------
The Shire
As you enter the Shire, you are surrounded by the endless rolling hills.
Locations: mordor, laketown,
>


Since I entered incorrect input, I haven’t left the Shire yet. So why am I seeing the message a

StackExchange Code Review Q#105194, answer score: 14