13
\$\begingroup\$

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)
AJNeufeld
35.2k5 gold badges41 silver badges103 bronze badges
asked Feb 20, 2020 at 3:30
\$\endgroup\$

2 Answers 2

22
\$\begingroup\$

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 not mixedCase.

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.

answered Feb 20, 2020 at 6:21
\$\endgroup\$
1
  • 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\$ Commented Feb 20, 2020 at 14:07
3
\$\begingroup\$

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)
answered Feb 21, 2020 at 18:55
\$\endgroup\$
4
  • \$\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\$ Commented 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\$ Commented 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\$ Commented 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\$ Commented Feb 22, 2020 at 6:45

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.