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
asked Jul 29, 2018 at 7:27
\$\endgroup\$

1 Answer 1

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.")
answered Jul 29, 2018 at 12:57
\$\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\$ Commented Jul 31, 2018 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\$ Commented Jul 31, 2018 at 5:01

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.