As I mentioned in my bio, I do not code for a living and too old to switch careers but I do enjoy it as a hobby and I am trying to get better. Also trying to get my teenage son more interested. Anyhow, the code below does want I want it to but I am looking for tips to improve it. Any advice would be much appreciated before I add more to it with High Score tables and more functionality.
import random
weapons = ["Sword", "Spell", "Fire"]
shields = ["Armour", "Magic", "Water"]
class Player:
def __init__(self, name):
self.name = name
self.health = 100
self.weapon = 0
self.shield = 0
def damage(self):
points = random.randint(10, 35)
self.health -= points
def selectWeapon(self):
choice = int(input("Choose your weapon 1-Sword, 2-Spell or 3-Fire: "))
self.weapon = choice - 1
def selectShield(self):
choice = int(input("Choose your shield 1-Armour, 2-Magic or 3-Water: "))
self.shield = choice - 1
# Child class of player with override methods for weapon
# and shield selection
class AiPlayer(Player):
def __init__(self,name):
super().__init__(name)
def selectWeapon(self):
choice = random.randint(1, 3)
self.weapon = choice - 1
def selectShield(self):
choice = random.randint(1, 3)
self.shield = choice - 1
class Game:
def __init__(self):
self.gameOver = False
self.round = 0
def newRound(self):
self.round += 1
print("\n*** Round: %d ***\n" %(self.round))
# Check if either or both Players is below zero health
def checkWin(self, player, opponent):
if player.health < 1 and opponent.health > 0:
self.gameOver = True
print("You Lose")
elif opponent.health < 1 and player.health > 0:
self.gameOver = True
print("You Win")
elif player.health < 1 and ai.health < 1:
self.gameOver = True
print("*** Draw ***")
def displayResult(self, player, opponent):
print("%s used a %s, %s used a %s Shield\n" %(player.name, weapons[player.weapon], opponent.name, shields[opponent.shield]))
print("%s caused damage to %s\n" %(player.name, opponent.name))
def takeTurn(self, player, opponent):
# Decision Array
#
# Armour| Magic | Water
# ______|________|_______
# Sword: False | True | True
# Spell: True | False | True
# Fire : True | True | False
decisionArray = [[False, True, True], [True, False, True], [True, True, False]]
if decisionArray[player.weapon][opponent.shield]:
opponent.damage()
currentGame.displayResult(player, opponent)
else:
print("\n%s used a %s, %s used a %s Shield" %(player.name, weapons[player.weapon], opponent.name, shields[opponent.shield]))
print("%s blocked %s's attack - No Damage" %(opponent.name, player.name))
# Setup Game Objects
currentGame = Game()
human = Player("Mark")
ai = AiPlayer("Computer")
players = [human, ai]
# Main Game Loop
while not currentGame.gameOver:
for player in players:
player.selectWeapon()
player.selectShield()
currentGame.newRound()
currentGame.takeTurn(human, ai)
currentGame.takeTurn(ai, human)
print("%s's health = %d" %(human.name, human.health))
print("%s's health = %d" %(ai.name, ai.health))
currentGame.checkWin(human, ai)
2 Answers 2
f-strings
Beginning with Python 3.6, there is a friendlier way of formatting strings. Instead of using "format string" % (tuple_of_args)
where the argument and the format codes are separated by significant distance, the arguments are embedded in the string itself, surrounded by {}
's. The string itself is prefixed with an f
.
For example, instead of:
print("\n*** Round: %d ***\n" %(self.round))
you can write:
print(f"\n*** Round: {self.round} ***\n")
Enumerations
This code doesn't convey a lot of meaning by itself. It needs to be described by your 7 comment lines.
decisionArray = [[False, True, True], [True, False, True], [True, True, False]]
Some of the problem stems from indices having semantic meaning. For example, 1 is Magic. But 1 is also Spell. Confusing.
Let's make what is happening a lot clearer. Let's define enumerations for our weapons and shields:
from enum import Enum
Weapon = Enum("Weapon", "Sword, Spell, Fire")
Shield = Enum("Shield", "Armour, Magic, Water")
Now we have Weapon.Sword
, Weapon.Spell
, and Weapon.Fire
, which conveniently have values 1, 2 and 3 respectively. We also have Shield.Armour
, Shield.Magic
and Shield.Water
, also with values 1, 2, and 3.
When we have our player select their choice
of weapon, we use to store it as the number 0, 1 or 2:
self.weapon = choice - 1
but now we can store an actual enumeration object, like Weapon.Fire
directly. Conveniently, passing the value 1, 2, or 3 to Weapon(...)
will return the corresponding enumeration object:
self.weapon = Weapon(choice)
Similarly, we can store our shield choice
as a Shield
enumeration object:
self.shield = Shield(choice)
The AI is similar, but instead of randint
, we can use choice
and select a random Weapon
and random Shield
:
self.weapon = random.choice(list(Weapon))
self.shield = random.choice(list(Shield))
But how about that decisionArray
? First of all, that is a lousy name. What does "decision array" mean? A Shield
will block one (or perhaps more) Weapon
attacks. Let's add a .blocks
attribute to our Shield
objects:
Shield.Armour.blocks = { Weapon.Sword }
Shield.Magic.blocks = { Weapon.Spell }
Shield.Water.blocks = { Weapon.Fire }
(I'm using a set()
, so more that one Weapon
can be added. For example, Shield.Magic
could block both { Weapon.Spell, Weapon.Fire }
... after all, it's MAGIC! Feel free to experiment.)
What would the takeTurn
method now look like?
def takeTurn(self, player, opponent):
if player.weapon not in opponent.shield.blocks:
# apply damage
else:
# damage was blocked
That's pretty straight forward; no table required. If the weapon is not in the set of things the shield blocks, we apply damage.
PEP-8
PEP 8 is a Style Guide for Python. It describes things all Python programs should conform to, such as:
- variables and function names should be
snake_case
notmixedCase
.
Updated Code
import random
from enum import Enum
Weapon = Enum("Weapon", "Sword, Spell, Fire")
Shield = Enum("Shield", "Armour, Magic, Water")
Shield.Armour.blocks = { Weapon.Sword }
Shield.Magic.blocks = { Weapon.Spell }
Shield.Water.blocks = { Weapon.Fire }
class Player:
def __init__(self, name):
self.name = name
self.health = 100
self.weapon = None
self.shield = None
def damage(self):
points = random.randint(10, 35)
self.health -= points
def select_weapon(self):
weapons = [f"{weapon.value}-{weapon.name}" for weapon in Weapon]
weapons = ", ".join(weapons[:-1]) + " or " + weapons[-1]
choice = int(input(f"Choose your weapon {weapons}: "))
self.weapon = Weapon(choice)
def select_shield(self):
shields = [f"{shield.value}-{shield.name}" for shield in Shield]
shields = ", ".join(shields[:-1]) + " or " + shields[-1]
choice = int(input(f"Choose your shield {shields}: "))
self.shield = Shield(choice)
class AiPlayer(Player):
def select_weapon(self):
self.weapon = random.choice(list(Weapon))
def select_shield(self):
self.shield = random.choice(list(Shield))
class Game:
def __init__(self):
self.game_over = False
self.round = 0
def new_round(self):
self.round += 1
print(f"\n*** Round: {self.round} ***\n")
# Check if either or both Players is below zero health
def check_win(self, player, opponent):
if player.health < 1 and opponent.health > 0:
self.game_over = True
print("You Lose")
elif opponent.health < 1 and player.health > 0:
self.game_over = True
print("You Win")
elif player.health < 1 and ai.health < 1:
self.game_over = True
print("*** Draw ***")
def display_result(self, player, opponent):
print(f"{player.name} used a {player.weapon.name}, {opponent.name} used a {opponent.shield.name} Shield\n")
print(f"{player.name} caused damage to {opponent.name}\n")
def take_turn(self, player, opponent):
if player.weapon not in opponent.shield.blocks:
opponent.damage()
current_game.display_result(player, opponent)
else:
print(f"{player.name} used a {player.weapon.name}, {opponent.name} used a {opponent.shield.name} Shield\n")
print(f"{opponent.name} blocked {player.name}'s attack - No Damage")
# Setup Game Objects
current_game = Game()
human = Player("Mark")
ai = AiPlayer("Computer")
players = [human, ai]
# Main Game Loop
while not current_game.game_over:
for player in players:
player.select_weapon()
player.select_shield()
current_game.new_round()
current_game.take_turn(human, ai)
current_game.take_turn(ai, human)
print(f"{human.name}'s health = {human.health}")
print(f"{ai.name}'s health = {ai.health}")
current_game.check_win(human, ai)
Notice in this code, if you added additional weapons and/or shields, nothing need be changed other than the Enum(...)
declarations and the .blocks = {}
lines. The code itself creates the "1-Armour, 2-Magic or 3-Water" type choice strings, and will properly work with more than 3 choices.
-
3\$\begingroup\$ This is awesome thanks very much for all the feedback. It's funny..... The snake case mixed case thing I did read PEP-8, started changing everything to snake case then changed it back. I will stick with the PEP-8 recommendations for future though. \$\endgroup\$CelestialMark– CelestialMark2020年02月20日 14:07:23 +00:00Commented Feb 20, 2020 at 14:07
To add to @AJNeufeld's answer, select_weapon
and select_shield
should probably validate their input. Those two functions could be refactored to remove duplicate code.
def select_enum(enum_class):
items = [f"{item.value}-{item.name}" for item in enum_class]
items = ", ".join(items[:-1]) + " or " + items[-1]
valid_values = set(item.value for item in enum_class)
while True:
choice = int(input(f"Choose your {enum_class.__name__.lower()}: {items}"))
if choice in valid_values:
break
print(f"'{choice}' is not a valid selection. Please try again.")
return enum_class(choice)
def select_weapon(self):
self.weapon = self.select_enum(Weapon)
def select_shield(self):
self.shield = self.select_enum(Shield)
-
\$\begingroup\$ Thank you for your suggestion, much appreciated. I will add it in once I get it straight in my head what each line is doing. \$\endgroup\$CelestialMark– CelestialMark2020年02月22日 01:12:07 +00:00Commented Feb 22, 2020 at 1:12
-
\$\begingroup\$ Okay I get it I was confused by the enum_class.__name__.lower(). I think it is returning the name of module so either Weapon or Shield so that the code works for both selections. Thank you. Learning some cool new code. \$\endgroup\$CelestialMark– CelestialMark2020年02月22日 01:23:29 +00:00Commented Feb 22, 2020 at 1:23
-
1\$\begingroup\$ @CelestialMark, you got it. Every class has a
__name__
attribute, that is the name of the class. There are several handy "dunder" attributes. \$\endgroup\$RootTwo– RootTwo2020年02月22日 01:28:18 +00:00Commented Feb 22, 2020 at 1:28 -
\$\begingroup\$ Nice addition. If you are validating input, however, you should go further. Add a
try: ... except ValueError:
to catch non-integer input, like"quit"
. \$\endgroup\$AJNeufeld– AJNeufeld2020年02月22日 06:45:39 +00:00Commented Feb 22, 2020 at 6:45