3
\$\begingroup\$

I made a text based board game that takes inspiration from DnD/Eldritch Horror (another board game)/Lovecraftian literature. Below you'll see a small excerpt that handles the bulk of the game processes, and there were several things I feel that could have been done better.

One thing I am not particularly proud of and want to find a better solution for, is this last minute feature I thought of that allows the players to change the "doom" or the turn counter in the game. Basically, they can pay resources to change the current doom count, which is changed in the game_handler method by adding the return value of from the process_turn method. Originally, the process_turn method had no such return value, and only returns one for the sake of being able to change the doom counter... I thought of using a global variable for the doom counter instead, but I thought global variables are for constants, which the doom count clearly is not.

Thanks in advance for the feedback, also if you want the game can be played on this quick webpage I put together: https://jsm209.github.io/Lugmere-s-Loss-Webpage/

# Pre: Given an integer that represents the current turn,
# Post: Will carry out single player games for each of the players, and produce a scoreboard of final scores at the end.
def game_handler(doom):

    # Game setup
    player_count = "-1"
    while player_count.isdigit() is False:
        player_count = input("How many people are playing Lugmere's Loss? ")
        if player_count.isdigit() is False:
            print("Please enter a number greater than 0.")
    players_normal = []
    for x in range(0, int(player_count)):
        players_normal.append(Player())
    i = 0
    while i < len(players_normal):
        players_normal[i].name = input("What is player " + str(i + 1) + "'s name? ") + " the " + n.generate_suffix()
        i += 1

    opening_story()

    # Regular game play.
    players_insane = []
    while doom is not 0:
        text_box("The doom counter sits at " + str(doom) + "...")

        # Player has a normal condition
        for player in players_normal:
            if has_lost(player) is False:
                doom += process_turn(player)  # Player actions impact the doom counter.
                press_enter()
                player.update()
                press_enter()

            # Player has a disabled condition
            elif player in players_insane:
                print_slow(player.get_name() + " is currently insane. They try to snap out of it.")
                if d.roll(20) >= 15:
                    print_slow(player.get_name() + " has sucessfully come to their senses!")
                    player.resources[4] = 50
                    players_insane.remove(player)
                else:
                    print_slow(player.get_name() + " remains delusional...")
            else:

                # Checks to see if the player gains a disabled condition
                if player.resources[4] == 0:
                    loss_by_sanity()
                    print_slow(player.get_name() + " has gone insane...")
                    players_insane.append(player)
                else:
                    print_slow(player.get_name() + " has to spend a day repairing their broken airship.")
                    player.add([0, 0, 0, 2, 0, 0, 0])
        doom -= 1

    # The Maw (End game boss) encounters only players fit to encounter (does not have disabled condition)
    for player in players_normal:
        if has_lost(player) is False:
            text_box(player.get_name() + " encounters The Maw.")
            player.score += encounter_maw(player)
        else:
            text_box(player.get_name() + " is disabled and does not encounter The Maw.")
    text_box("FINAL SCORES")
    i = 0
    while i < len(players_normal):
        print_slow(players_normal[i].get_name() + "'s SCORE IS: " + str(players_normal[i].score))
        i += 1


# Pre: Given a valid player object,
# Post: Processes one full turn for the player. After getting a course of action from the player, it performs it
# assuming the player has the resources to do so. If the action was not able to be performed (due to a lack of
# resources) it asks the player again for a different decision. It will return an integer based on the player's choices
# to be added to the doom counter.
def process_turn(player):
    text_box(player.get_name() + ", you have: ")
    print()
    player.get_count()
    print()
    press_enter()
    if player.resources[0] < 0:
        print_slow(player.get_name() + " is in debt!")
    print_slow()
    print("How do you spend your day?")
    choice = pick_choice(["Mine", "Dock [Costs 10 credits per crew member, including yourself]",
                          "Recruit [Costs 50 credits]", "Work", "Tamper [Costs 1 Wisdom, Minimum 5 Flux or 5 Crew]",
                          "Encounter [Minimum 1 Flux and 1 Crew]", "HELP"])
    doom_mod = 0
    if choice == 1:
        player.mine(d)
    elif choice == 2 and player.resources[0] >= 10*(player.resources[5]+1):
        player.dock(d)
    elif choice == 3 and player.resources[0] >= 50:
        player.recruit(d)
    elif choice == 4:
        player.work(d)
    elif choice == 5 and (player.resources[5] >= 5 or player.resources[2] >= 5) and player.resources[6] >= 1:
        doom_mod += tamper(player, 2)
    elif choice == 6 and player.resources[5] > 0 and player.resources[2] > 0:
        player.resources[2] -= 1
        encounter = exploration_encounters.get_random_encounter()
        player.add(encounter.get_outcome())
    elif choice == 7:
        help()
        process_turn(player)
    else:
        print_slow("You don't have the resources to do that. Please select another course of action.")
        process_turn(player)
    return doom_mod


# Pre: Given a list of strings that represent a choice the player can make,
# Post: Returns an integer representing the choice the player picked.
def pick_choice(choices):
    for x in choices:
        print(str(choices.index(x)+1) + ": " + x)
    while True:
        decision = None
        try:
            decision = int(input("ENTER 1-" + str(len(choices)) + ": "))
        except ValueError or decision not in range(1, len(choices)+1):
            print("That isn't an option.")
            continue
        if decision in range(1, len(choices)+1):
            return decision
\$\endgroup\$
3
\$\begingroup\$

Printing a menu of choices

This is a poor way to present a menu of choices, and it doesn't fully work:

def pick_choice(choices):
    for x in choices:
        print(str(choices.index(x)+1) + ": " + x)
    while True:
        decision = None
        try:
            decision = int(input("ENTER 1-" + str(len(choices)) + ": "))
        except ValueError or decision not in range(1, len(choices)+1):
            print("That isn't an option.")
            continue
        if decision in range(1, len(choices)+1):
            return decision

There are several issues here:

  • Using index to find the index is inefficient. It would be better to use a counting loop. An even better technique is to use enumerate.
  • Checking if decision is within the range in the except statement doesn't work. It's also very unfortunate to duplicate logic, this range check appears again a few lines below.
  • Checking if a value is in a range(...) is inefficient, because it's a linear search.
  • The initialization decision = None is unnecessary.

A better way to write that menu:

def pick_choice(choices):
    for index, value in enumerate(choices):
        print("{}: {}".format(index + 1, value))

    while True:
        try:
            decision = int(input("ENTER 1-{}: ".format(len(choices))))
            if 1 <= decision <= len(choices):
                return decision

        except ValueError:
            pass

        print("That isn't an option.")

Coding style in Python

Python has a style guide called PEP8. The posted code violates several points in the guide, I highlight some of them, I suggest to read through the document carefully.

  • Instead of value is False, write not value
  • Instead of num is not 0, write num != 0 or num > 0 as appropriate
  • Instead of a while loop with a manual counter, as in i = 0; while i < len(players_normal): ..., it's simpler to use for i in range(len(players_normal)):
  • Instead of embedding a numeric value in a string like doom in this example: "The doom counter sits at " + str(doom) + "...", it's recommended to use the format function: "The doom counter sits at {} ...".format(doom)
  • When the counter variable in a loop is not used, as in for x in range(0, int(player_count)): ..., the convention is to name the variable _

Instead of iterating over indexes, it's better to iterate over list elements. For example instead of this:

i = 0
while i < len(players_normal):
    print_slow(players_normal[i].get_name() + "'s SCORE IS: " + str(players_normal[i].score))
    i += 1

This is simpler and better:

for player in players_normal:
    print_slow("{}'s SCORE IS: {}".format(player.name, player.score))

Object oriented programming

In some places the program checks if a player is insane by player in players_insane. It would be more natural if this state was part of the Player class. It would be more efficient too, because players_insane is a list, and searching in a list is linear.

Also, it's impossible to guess the meaning of this:

if player.resources[4] == 0:

What is resource number 4, and what does it mean when its value is 0? A good object-oriented program would ideally read like a story, even without comments.

The same goes for this:

player.add([0, 0, 0, 2, 0, 0, 0])

Use list comprehensions

This can be written more compactly with a list comprehension:

players_normal = []
for x in range(0, int(player_count)):
    players_normal.append(Player())

Like this:

players_normal = [Player() for _ in range(int(player_count))]

Other style issues

Instead of getters like player.get_name(), it would be more natural to use properties.


Consider this snippet:

player_count = "-1"
while player_count.isdigit() is False:
    player_count = input("How many people are playing Lugmere's Loss? ")
    if player_count.isdigit() is False:
        print("Please enter a number greater than 0.")

The initialization of player_count is unnecessary, and the duplicated check of player_count.isdigit() can be avoided:

while True:
    player_count = input("How many people are playing Lugmere's Loss? ")
    if not player_count.isdigit():
        print("Please enter a number greater than 0.")
\$\endgroup\$
2
  • \$\begingroup\$ Wow thank you so much for the helpful feedback. I've made the changes and ran into a few questions I have about your comments: (1) I initially learned string formatting with "%". Is it always better to use .format rather than %? (2) Are there any cases where it's ever appropriate to use str(variable) instead of .format(variable)? (3) "...because players_insane is a list, and searching in a list is linear." It seems that lists are a terrible data structure because the only way to search them is with linear search? Is there an alternative data structure that is more easily searchable? \$\endgroup\$ Jul 31 '18 at 3:39
  • 1
    \$\begingroup\$ @JoshuaMaza (1) % is obsolete, there is no more legitimate use case. Use .format or f-strings. (2) str(x) is better than '{}'.format(x) to convert the int value x to string. (3) list is a great data structure for the right purpose. There's always a tradeoff. All data structures are good for some things and bad for some things. List is good for simple ordering, bad for searching. Set is good for searching but inappropriate for ordering. And so on. \$\endgroup\$
    – janos
    Jul 31 '18 at 5:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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