Skip to main content
Add example of improved user input handling, add link to extended demo
Source Link
AlexV
  • 7.4k
  • 2
  • 24
  • 47
def change_relation(faction, type_of_change):
    """Create a message to describe the change in the relationship

    faction and type_of_change are case insensitive and have to correspond to
    class variables of Factions nd RelationshipChanges.
    """
    message_template = getattr(RelationshipChanges, type_of_change.upper())
    return message_template.format(getattr(Factions, faction.upper()))

With this, change_relation("civilians", "great_increase") would generate the same output as previously seen. The function uses Python's built-in getattr(...) function to programmatically access members of the class by their name. As an example, getattr(Factions, "ARMY") would be the same as Factions.ARMY. Neat, isn't it? 

If you were even more keen on saving some typing, this function would easily allow to add a "translation" dictionaries as an intermediate. These dict could then map '+++' to RelationshipChanges.GREAT_INCREASE or 'civ' to Factions.CIVILIANS and shorten the previous function call to change_relation('civ', '+++'). I will leave that as an exercise to you.I will leave that as an exercise to you. See the updated version below.

One way would be to put them into a class such as we did before with the other constant values. A dict might also be a viable solution. But wait! We have already started somethind related to those changes, haven't we? Well observed. Time to have another look at RelationshipChanges. At the moment this class simply holds the template message for each of the changes. With just one more level of "nesting", we can add the score modifiers as well. As we wrote a function to help us handle those relationship changes, everything will stay smooth.

ULTIMATE_SCORE_CHANGE = 15
MAJOR_SCORE_CHANGE = 2
NORMAL_SCORE_CHANGE = 1
SLIGHT_SCORE_CHANGE = 0.5

class RelationshipChanges:
    """Holds templates and modifiers to decribe changes in the relationships"""

    HEORISM = {
        'message': '{} looks at you as a hero.',
        'modifier': ULTIMATE_SCORE_CHANGE
    }
    GREAT_INCREASE = {
        'message': 'This greatly improves your relationship with {}.',
        'modifier': MAJOR_SCORE_CHANGE
    }
    INCREASE = {
        'message': 'This improves your relationship with {}.',
        'modifier': NORMAL_SCORE_CHANGE
    }
    SLIGHT_INCREASE = {
        'message': 'This slightly improves your relationship with {}.',
        'modifier': SLIGHT_SCORE_CHANGE
    }

    SLIGHT_DECREASE = {
        'message': 'This slightly decreases your relationship with {}.',
        'modifier': -SLIGHT_SCORE_CHANGE
    }
    DECREASE = {
        'message': 'This worsens your relationship with {}.',
        'modifier': -NORMAL_SCORE_CHANGE
    }
    GREAT_DECREASE = {
        'message': 'This greatly worsens your relationship with {}.',
        'modifier': -MAJOR_SCORE_CHANGE
    }
    TREASON = {
        'message': '{} wants you dead.',
        'modifier': -ULTIMATE_SCORE_CHANGE
    }

 
def change_relation(faction, type_of_change):
    """Create a message to describe the change in the relationship

    faction and type_of_change are case insensitive and have to correspond to
    class variables of Factions nd RelationshipChanges.
    """
    change_descr = getattr(RelationshipChanges, type_of_change.upper())
    faction_name = getattr(Factions, faction.upper())
    return change_descr['modifier'], change_descr['message'].format(faction_name)

Now that those messages and the actual changes to the score are linked more closely, it would be a great moment to remove those change messages from the static game text. A benefit of this would be that if you ever decided to change the effects of an action, you would only have to do this in on place, namely on of the event functions, and not there and somewhere else hidden in all the storyline text. Since those message are IIRC merely appended to the storyline text, the output should not change significantly. Of course the implementation of change_relation has to be adapted to fit these changes, and since all that stops change_relation from actually updating the relationship score is not knowing about relationships it is easy to adapt it to do more repetive work for us:

def change_relation(relationships, faction, type_of_change):
    """Documentation omitted for brevity"""
    type_translation = {
        "---": "great_decrease", "--": "decrease", "-": "slight_decrease",
        "+++": "great_increase", "++": "increase", "+": "slight_increase"
    }
    if type_of_change in type_translation:
        # only apply the translation if it's own of ---/--/.../+++
        type_of_change = type_translation[type_of_change]

    change_descr = getattr(RelationshipChanges, type_of_change.upper())
    faction_name = getattr(Factions, faction.upper())
    relationships[faction_name] += change_descr['modifier']

    return change_descr['message'].format(faction_name)

You can now use something like print(change_relation(relationships, "civilians", "---")) to adapt the game state and tell the user about the consequences of his/her decision. (Note: The code above builds upon a change to relationships that will be explained in the following section.)

At the moment the user input handling is not very robust. Once you enter an invalid command, e.g. by smashing the enter key to long, the program bails out and you have to start all over. This can be very annoying. A better approach would be to ask for invalid input several times and only bail out if a termination character like q/Q is entered or the user did not provide a valid input six times in a row. An implementation of this approach might look like:

Mini demo

def prompt_for_input(prompt, valid_inputs, max_tries=6):
    print(prompt)
    for _ in range(max_tries):
        user_input = input('> ').upper()
        if user_input in valid_inputs:
            return user_input
        if user_input == 'Q':
            break
        # the input was not valid, show the roadblock
        roadblock()
    # Either Q or excessive retrying brought us here
    print('Seems like you are not willing to play. Goodbye!')
    sys.exit(0)

With those changes above you are now at a stage wher you can write code like:

Mini demo

modifier, message = change_relation("civilians", "great_increase")
relationships[Factions.CIVILIANS] += modifier
final_standing = get_final_standing(relationships[Factions.CIVILIANS], (-7, -4, -2, 2, 5, 7))
print(message)
print(f'You left the {Factions.CIVILIANS} feeling {final_standing}.')

The answer contains some a lof of proposals that change the code drastically. Since you requested to see more code to better understand it and the answer is already quite long, I decided to implement a reduced version of your game that implements those proposed changes and upload it into a gist. The gist is hidden from search engines but accessible to everyone with the link.

def change_relation(faction, type_of_change):
    """Create a message to describe the change in the relationship

    faction and type_of_change are case insensitive and have to correspond to
    class variables of Factions nd RelationshipChanges.
    """
    message_template = getattr(RelationshipChanges, type_of_change.upper())
    return message_template.format(getattr(Factions, faction.upper()))

With this, change_relation("civilians", "great_increase") would generate the same output as previously seen. The function uses Python's built-in getattr(...) function to programmatically access members of the class by their name. As an example, getattr(Factions, "ARMY") would be the same as Factions.ARMY. Neat, isn't it? If you were even more keen on saving some typing, this function would easily allow to add a "translation" dictionaries as an intermediate. These dict could then map '+++' to RelationshipChanges.GREAT_INCREASE or 'civ' to Factions.CIVILIANS and shorten the previous function call to change_relation('civ', '+++'). I will leave that as an exercise to you.

One way would be to put them into a class such as we did before with the other constant values. A dict might also be a viable solution. But wait! We have already started somethind related to those changes, haven't we? Well observed. Time to have another look at RelationshipChanges. At the moment this class simply holds the template message for each of the changes. With just one more level of "nesting", we can add the score modifiers as well. As we wrote a function to help us handle those relationship changes, everything will stay smooth.

ULTIMATE_SCORE_CHANGE = 15
MAJOR_SCORE_CHANGE = 2
NORMAL_SCORE_CHANGE = 1
SLIGHT_SCORE_CHANGE = 0.5

class RelationshipChanges:
    """Holds templates and modifiers to decribe changes in the relationships"""

    HEORISM = {
        'message': '{} looks at you as a hero.',
        'modifier': ULTIMATE_SCORE_CHANGE
    }
    GREAT_INCREASE = {
        'message': 'This greatly improves your relationship with {}.',
        'modifier': MAJOR_SCORE_CHANGE
    }
    INCREASE = {
        'message': 'This improves your relationship with {}.',
        'modifier': NORMAL_SCORE_CHANGE
    }
    SLIGHT_INCREASE = {
        'message': 'This slightly improves your relationship with {}.',
        'modifier': SLIGHT_SCORE_CHANGE
    }

    SLIGHT_DECREASE = {
        'message': 'This slightly decreases your relationship with {}.',
        'modifier': -SLIGHT_SCORE_CHANGE
    }
    DECREASE = {
        'message': 'This worsens your relationship with {}.',
        'modifier': -NORMAL_SCORE_CHANGE
    }
    GREAT_DECREASE = {
        'message': 'This greatly worsens your relationship with {}.',
        'modifier': -MAJOR_SCORE_CHANGE
    }
    TREASON = {
        'message': '{} wants you dead.',
        'modifier': -ULTIMATE_SCORE_CHANGE
    }

 
def change_relation(faction, type_of_change):
    """Create a message to describe the change in the relationship

    faction and type_of_change are case insensitive and have to correspond to
    class variables of Factions nd RelationshipChanges.
    """
    change_descr = getattr(RelationshipChanges, type_of_change.upper())
    faction_name = getattr(Factions, faction.upper())
    return change_descr['modifier'], change_descr['message'].format(faction_name)

Now that those messages and the actual changes to the score are linked more closely, it would be a great moment to remove those change messages from the static game text. A benefit of this would be that if you ever decided to change the effects of an action, you would only have to do this in on place, namely on of the event functions, and not there and somewhere else hidden in all the storyline text. Since those message are IIRC merely appended to the storyline text, the output should not change significantly.

At the moment the user input handling is not very robust. Once you enter an invalid command, e.g. by smashing the enter key to long, the program bails out and you have to start all over. This can be very annoying. A better approach would be to ask for invalid input several times and only bail out if a termination character like q is entered or the user did not provide a valid input six times in a row.

Mini demo

With those changes above you are now at a stage wher you can write code like:

modifier, message = change_relation("civilians", "great_increase")
relationships[Factions.CIVILIANS] += modifier
final_standing = get_final_standing(relationships[Factions.CIVILIANS], (-7, -4, -2, 2, 5, 7))
print(message)
print(f'You left the {Factions.CIVILIANS} feeling {final_standing}.')
def change_relation(faction, type_of_change):
    message_template = getattr(RelationshipChanges, type_of_change.upper())
    return message_template.format(getattr(Factions, faction.upper()))

With this, change_relation("civilians", "great_increase") would generate the same output as previously seen. The function uses Python's built-in getattr(...) function to programmatically access members of the class by their name. As an example, getattr(Factions, "ARMY") would be the same as Factions.ARMY. Neat, isn't it? 

If you were even more keen on saving some typing, this function would easily allow to add a "translation" dictionaries as an intermediate. These dict could then map '+++' to RelationshipChanges.GREAT_INCREASE or 'civ' to Factions.CIVILIANS and shorten the previous function call to change_relation('civ', '+++'). I will leave that as an exercise to you. See the updated version below.

One way would be to put them into a class such as we did before with the other constant values. A dict might also be a viable solution. But wait! We have already started somethind related to those changes, haven't we? Well observed. Time to have another look at RelationshipChanges. At the moment this class simply holds the template message for each of the changes. With just one more level of "nesting", we can add the score modifiers as well.

ULTIMATE_SCORE_CHANGE = 15
MAJOR_SCORE_CHANGE = 2
NORMAL_SCORE_CHANGE = 1
SLIGHT_SCORE_CHANGE = 0.5

class RelationshipChanges:
    """Holds templates and modifiers to decribe changes in the relationships"""

    HEORISM = {
        'message': '{} looks at you as a hero.',
        'modifier': ULTIMATE_SCORE_CHANGE
    }
    GREAT_INCREASE = {
        'message': 'This greatly improves your relationship with {}.',
        'modifier': MAJOR_SCORE_CHANGE
    }
    INCREASE = {
        'message': 'This improves your relationship with {}.',
        'modifier': NORMAL_SCORE_CHANGE
    }
    SLIGHT_INCREASE = {
        'message': 'This slightly improves your relationship with {}.',
        'modifier': SLIGHT_SCORE_CHANGE
    }

    SLIGHT_DECREASE = {
        'message': 'This slightly decreases your relationship with {}.',
        'modifier': -SLIGHT_SCORE_CHANGE
    }
    DECREASE = {
        'message': 'This worsens your relationship with {}.',
        'modifier': -NORMAL_SCORE_CHANGE
    }
    GREAT_DECREASE = {
        'message': 'This greatly worsens your relationship with {}.',
        'modifier': -MAJOR_SCORE_CHANGE
    }
    TREASON = {
        'message': '{} wants you dead.',
        'modifier': -ULTIMATE_SCORE_CHANGE
    }

Now that those messages and the actual changes to the score are linked more closely, it would be a great moment to remove those change messages from the static game text. A benefit of this would be that if you ever decided to change the effects of an action, you would only have to do this in on place, namely on of the event functions, and not there and somewhere else hidden in all the storyline text. Since those message are IIRC merely appended to the storyline text, the output should not change significantly. Of course the implementation of change_relation has to be adapted to fit these changes, and since all that stops change_relation from actually updating the relationship score is not knowing about relationships it is easy to adapt it to do more repetive work for us:

def change_relation(relationships, faction, type_of_change):
    """Documentation omitted for brevity"""
    type_translation = {
        "---": "great_decrease", "--": "decrease", "-": "slight_decrease",
        "+++": "great_increase", "++": "increase", "+": "slight_increase"
    }
    if type_of_change in type_translation:
        # only apply the translation if it's own of ---/--/.../+++
        type_of_change = type_translation[type_of_change]

    change_descr = getattr(RelationshipChanges, type_of_change.upper())
    faction_name = getattr(Factions, faction.upper())
    relationships[faction_name] += change_descr['modifier']

    return change_descr['message'].format(faction_name)

You can now use something like print(change_relation(relationships, "civilians", "---")) to adapt the game state and tell the user about the consequences of his/her decision. (Note: The code above builds upon a change to relationships that will be explained in the following section.)

At the moment the user input handling is not very robust. Once you enter an invalid command, e.g. by smashing the enter key to long, the program bails out and you have to start all over. This can be very annoying. A better approach would be to ask for invalid input several times and only bail out if a termination character like q/Q is entered or the user did not provide a valid input six times in a row. An implementation of this approach might look like:

def prompt_for_input(prompt, valid_inputs, max_tries=6):
    print(prompt)
    for _ in range(max_tries):
        user_input = input('> ').upper()
        if user_input in valid_inputs:
            return user_input
        if user_input == 'Q':
            break
        # the input was not valid, show the roadblock
        roadblock()
    # Either Q or excessive retrying brought us here
    print('Seems like you are not willing to play. Goodbye!')
    sys.exit(0)

Mini demo

The answer contains some a lof of proposals that change the code drastically. Since you requested to see more code to better understand it and the answer is already quite long, I decided to implement a reduced version of your game that implements those proposed changes and upload it into a gist. The gist is hidden from search engines but accessible to everyone with the link.

Source Link
AlexV
  • 7.4k
  • 2
  • 24
  • 47

Style

Before diving in the actual code, some general style considerations first. Python comes with an official Style Guide. The most relevant parts for your code would be the sections on how to structure code using blank lines where appropriate (two blank lines between separate functions and classes, only single blank line within functions and classes) and the recommendations on how to document your functions using documentation strings """enclosed in triple quotes""". The code examples in the following answer will demonstrate both of these style elements.


Note: For convenience, some of the code below uses f-strings, which is a new Python feature introduced with Python 3.6. If you're not yet there, it should be obvious how to transform those pieces to use .format(...) instead.


Don't repeat yourself

Your game has a lot of duplicate text, e.g. where you start to describe the possible changes in the relationship with the other factions. I would propose to collect those templates in a "dumb" class, or maybe a dictionary if you don't like classes, and then put in the factions as you need them. This could be done like so:

class Factions:
    """Class to represent the factions found in the game"""

    ARMY = "ARMY & GOVERNMENT"
    CIVILIANS = "CIVILIANS"
    EVERYBODY = "EVERYBODY"


class RelationshipChanges:
    """Holds templates to decribe changes in relationships"""

    HEROISM = '{} looks at you as a hero.'
    GREAT_INCREASE = 'This greatly improves your relationship with {}.'
    INCREASE = 'This improves your relationship with {}.'
    SLIGHT_INCREASE = 'This slightly improves your relationship with {}.'

    SLIGHT_DECREASE = 'This slightly decreases your relationship with {}.'
    DECREASE = 'This worsens your relationship with {}.'
    GREAT_DECREASE = 'This greatly worsens your relationship with {}.'
    TREASON = '{} wants you dead.'

and then do RelationshipChanges.GREAT_INCREASE.format(Factions.CIVILIANS) instead of defining civil_great_increase and all of its companions. The code would generate

This greatly improves your relationship with CIVILIANS.

It might be also a good idea to define a function as a shorthand for this, since it's not quite a pleasure to type.

def change_relation(faction, type_of_change):
    """Create a message to describe the change in the relationship

    faction and type_of_change are case insensitive and have to correspond to
    class variables of Factions nd RelationshipChanges.
    """
    message_template = getattr(RelationshipChanges, type_of_change.upper())
    return message_template.format(getattr(Factions, faction.upper()))

With this, change_relation("civilians", "great_increase") would generate the same output as previously seen. The function uses Python's built-in getattr(...) function to programmatically access members of the class by their name. As an example, getattr(Factions, "ARMY") would be the same as Factions.ARMY. Neat, isn't it? If you were even more keen on saving some typing, this function would easily allow to add a "translation" dictionaries as an intermediate. These dict could then map '+++' to RelationshipChanges.GREAT_INCREASE or 'civ' to Factions.CIVILIANS and shorten the previous function call to change_relation('civ', '+++'). I will leave that as an exercise to you.

The relationship levels themselves could be handled similarly.

class RelationshipLevels:
    """Class to represent the player's relationship to other factions"""

    VENGEFUL = "VENGEFUL"
    HATEFUL = "HATEFUL"
    DISAPPOINTED = "DISAPPOINTED"
    CONFLICTED = "CONFLICTED/NEUTRAL"
    SATISFIED = "SATISFIED"
    HAPPY = "HAPPY"
    PROPEROUS = "PROPEROUS"

    ALL = [VENGEFUL, HATEFUL, DISAPPOINTED, CONFLICTED, SATISFIED, HAPPY, PROSPEROUS]
    #^--- this will become handy in a moment

army_left and civil_left are another instance where you tend to repeat the same pieces of code/text over and over again. If you think about those two for a moment, the common pattern will become clear: For a given faction and its relationship score, you want to determine the relationship level. Therefor, you essentially check if the score is below a certain treshold, format the message and print it. A way to generalize that idea would be as follows:

def get_final_standing(relation_score, thresholds):
    """Determine how the faction thinks about the player at the end"""
    for threshold, feeling in zip(thresholds, RelationshipLevels.ALL):
        if relation_score <= threshold:
            return feeling

    return RelationshipLevels.ALL[-1]

The function uses zip(...) two iterate over two sequences in parallel, and stops the loop (break) if it has found the appropriate relationship level. It becomes a little bit tricky if you don't want to put an upper limit to the threshold so I decided to implement this in a way that whenever the score is greater than the last threshold you gave, the most positive (i.e. rightmost) level will be returned. To realize the same funcationality as army_left formerly implemented you would then do

final_standing = get_final_standing(relationships[Factions.CIVILIANS], (-7, -4, -2, 2, 5, 7))
print(f'You left the {Factions.ARMY} feeling {final_standing}.')

I leave civil_left as an exercise to you.

All the score increments/decrements should also be bundled somehow. At the moment you have slight, slightly, and relationships[5] to express a slight change in the score in either direction. The same pattern is more or less to be found for normal and major changes, as well as for hero/traitor. That's madness!

One way would be to put them into a class such as we did before with the other constant values. A dict might also be a viable solution. But wait! We have already started somethind related to those changes, haven't we? Well observed. Time to have another look at RelationshipChanges. At the moment this class simply holds the template message for each of the changes. With just one more level of "nesting", we can add the score modifiers as well. As we wrote a function to help us handle those relationship changes, everything will stay smooth.

ULTIMATE_SCORE_CHANGE = 15
MAJOR_SCORE_CHANGE = 2
NORMAL_SCORE_CHANGE = 1
SLIGHT_SCORE_CHANGE = 0.5

class RelationshipChanges:
    """Holds templates and modifiers to decribe changes in the relationships"""

    HEORISM = {
        'message': '{} looks at you as a hero.',
        'modifier': ULTIMATE_SCORE_CHANGE
    }
    GREAT_INCREASE = {
        'message': 'This greatly improves your relationship with {}.',
        'modifier': MAJOR_SCORE_CHANGE
    }
    INCREASE = {
        'message': 'This improves your relationship with {}.',
        'modifier': NORMAL_SCORE_CHANGE
    }
    SLIGHT_INCREASE = {
        'message': 'This slightly improves your relationship with {}.',
        'modifier': SLIGHT_SCORE_CHANGE
    }

    SLIGHT_DECREASE = {
        'message': 'This slightly decreases your relationship with {}.',
        'modifier': -SLIGHT_SCORE_CHANGE
    }
    DECREASE = {
        'message': 'This worsens your relationship with {}.',
        'modifier': -NORMAL_SCORE_CHANGE
    }
    GREAT_DECREASE = {
        'message': 'This greatly worsens your relationship with {}.',
        'modifier': -MAJOR_SCORE_CHANGE
    }
    TREASON = {
        'message': '{} wants you dead.',
        'modifier': -ULTIMATE_SCORE_CHANGE
    }


def change_relation(faction, type_of_change):
    """Create a message to describe the change in the relationship

    faction and type_of_change are case insensitive and have to correspond to
    class variables of Factions nd RelationshipChanges.
    """
    change_descr = getattr(RelationshipChanges, type_of_change.upper())
    faction_name = getattr(Factions, faction.upper())
    return change_descr['modifier'], change_descr['message'].format(faction_name)

Now that those messages and the actual changes to the score are linked more closely, it would be a great moment to remove those change messages from the static game text. A benefit of this would be that if you ever decided to change the effects of an action, you would only have to do this in on place, namely on of the event functions, and not there and somewhere else hidden in all the storyline text. Since those message are IIRC merely appended to the storyline text, the output should not change significantly.

Make it harder to be wrong

Ah, that damn army ... where was their score in relationships again? Was it the first or the second position? Maybe the third?

To avoid situations like this, I would recommend to use a dictionary. This leaves you with something like relationships = {"army": 0, "civil": 0} or even relationships = {Factions.ARMY: 0, Factions.CIVILIANS: 0}. Using relationships[Factions.ARMY] leaves absolutely no doubt what you're trying to do. It will also make it waaaay easier to spot copy and paste errors.

Avoid globals

Global variables are best avoided, since it's harder to see which parts of the code modify them, which leads to all kind of problems. The core object of your game would be relationships and it would be easy to transform all your game functions to accept it as an argument instead of relying on it to be present on a global scope. The most common approach would be to somehow define a main function which does all the needed initialization stuff, like displaying the synopsis or initializing relationships. relationships is then passed to story which again passes it onwards depending on how the player chooses his actions.

All the game text should wrightfully stay in global variables. For them I would recommend to CAPITALIZE_THEIR_NAMES to make it clear that they are supposed to be used/seen as constant values.

User input handling

At the moment the user input handling is not very robust. Once you enter an invalid command, e.g. by smashing the enter key to long, the program bails out and you have to start all over. This can be very annoying. A better approach would be to ask for invalid input several times and only bail out if a termination character like q is entered or the user did not provide a valid input six times in a row.

Mini demo

With those changes above you are now at a stage wher you can write code like:

modifier, message = change_relation("civilians", "great_increase")
relationships[Factions.CIVILIANS] += modifier
final_standing = get_final_standing(relationships[Factions.CIVILIANS], (-7, -4, -2, 2, 5, 7))
print(message)
print(f'You left the {Factions.CIVILIANS} feeling {final_standing}.')