Text-based turn-based fighter game

Question

To get a better understanding of objects/methods, I wrote a text-based, turn-based fighting game.

I want to expand by adding new characters with new moves (one of which can alter the way other moves works - I'm thinking of using Decorators for this).

Anyways, I'm open to suggestions, advice, constructive criticism, etc.

import random
class Characters:
 class Fighter:
 def __init__(self,hp=100,atk=75,defen=50,speed=60):
 self.hp = hp
 self.atk = atk
 self.defen = defen
 self.speed = speed
 class Options:
 def punch(self,target,modifier=1.00):
 base = 50/100.0
 damage = (self.atk - target.defen)*base
 target.hp = target.hp - damage
 print(self.Name + " punched " + target.Name + " for %d HP" % damage)
 punch.priority = 1
 def heal(self,target,modifier=1.00):
 self.hp += 15
 if self.hp > 100:
 self.hp = 100
 print(self.Name + " healed 15 HP")
 heal.priority = 2
 def kick(self,target,modifier=1.00):
 base = 100/100.0
 damage = (self.atk-target.defen)*base
 target.hp = target.hp - damage
 self.hp = self.hp - 10
 print(self.Name + " kicked " + target.Name + " for %d HP" % damage)
 kick.priority = 2
# Declare Marco & Player 
Player = Characters.Fighter()
Player.Name = "You"
Marco = Characters.Fighter()
Marco.Name = "Marco"
# Define Turn Action
def play(Player1,Player2):
 # First - Check Options Priority
 if Player1.Turn < Player2.Turn:
 Player1.move(Player1,Player2)
 if Player2.hp > 0:
 Player2.move(Player2,Player1)
 else:
 Player2.hp = 0
 return Player2.hp
 elif Player1.Turn > Player2.Turn:
 Player2.move(Player2,Player1)
 if Player1.hp > 0:
 Player1.move(Player1,Player2)
 else:
 Player1.hp = 0
 return Player1.hp
 # Second - Check Player's Speed
 else:
 if Player1.speed > Player2.speed:
 Player1.move(Player1,Player2)
 if Player2.hp > 0:
 Player2.move(Player2,Player1)
 else:
 Player2.hp = 0
 return Player2.hp
 elif Player1.speed < Player2.speed:
 Player1.move(Player1,Player2)
 if Player2.hp > 0:
 Player2.move(Player2,Player1)
 else:
 Player2.hp = 0
 return Player2.hp
 # Third - Ranomdly Choose
 else:
 random.choice(range(2))
 if random.choice == 0:
 Player1.Turn = Player2.Turn+1
 play(Player1,Player2)
 else:
 Player1.Turn = Player2.Turn-1
 play(Player1,Player2)
### Fight Sequence ###
# Start
print("\n 3...2...1...")
print("FIGHT!")
# Begin Fight/Conditions for Ending
while Player.hp > 0 and Marco.hp > 0:
 # Situational Update
 print("\n \t You have %d health points." %Player.hp)
 print("\n \t Marco has %d health points." %Marco.hp)
 print("\n \t It is your move!!!")
 # Choosing the Options - Correctly
 # Place Holder let's the whole loop start/error check.
 playerchoice = 'place holder'
 while playerchoice not in [1,2,3]:
 playerchoice = input("\n Which move would you like to use: \
 \n 1) Punch -- Base Damage of 50 (Weak) \
 \n 2) Kick -- Base Damage of 100 (Moderate), but lose 10 HP\
 \n 3) Heal -- Heal for 15 HP. \
 \n -->")
 # Convert Player Choice to String for Dict-Search
 if playerchoice == 1:
 playerchoice = "punch"
 elif playerchoice == 2:
 playerchoice = "kick"
 else:
 playerchoice = "heal" 
 # Randomly Choose Marco's move from list of Options Options
 # Marco shouldn't be frequently healing at higher HP (Hence, if/else)
 if Marco.hp < random.choice( range(80,100) ):
 marcochoice = random.choice(Marco.Options.__dict__.keys()[1:4])
 else:
 marcochoice = random.choice(Marco.Options.__dict__.keys()[2:4])
 Marco.move = Marco.Options.__dict__[marcochoice]
 Player.move = Player.Options.__dict__[playerchoice]
 print(Marco.move)
 print (Player.move)
 ### Determine who goes first
 Player.Turn = Player.move.priority
 Marco.Turn = Marco.move.priority
 print Marco.Turn
 print Player.Turn
 # First - Check Options Priority: 
 play(Player,Marco)

Answer 1

• General

  • PEP8 calls for snake_case for methods, and reserves Capitalized names for classes. Using Player1 for variable is not right.

  • Returning values is much more explicit than state mutations. I strongly recommend passing players' Turns to play as variables rather then Player's properties.

  For the rest of review I'll use original style.

• Move selection

  belongs to the player, not the main loop. For example,

  class Human(Characters.Fighter):
   ....
   def chose(self):
   follow_the_logic_here()
   self.move = self.Options.__dict__[playerchoice]
   self.Turn = self.move.priority
  

  and similar for Marco (who is really Robot). The main loop simplifies to

   while (....):
   situation_update()
   Player.chose()
   Marco.chose()
   play(Player, Marco)
  

• play looks a bit convoluted, especially the recursive call. I recommend to factor out the rules of move order into a separate function, as in

  def play(Player1, Player2):
   p1, p2 = order_of_move(Player1, Player2)
   p1.move(p1, p2)
   if p2.hp > 0:
   p2.move(p2, p1)
  

  and

  def order_of_move(p1, p2):
   if p1.Turn < p2.turn:
   return (p1, p2)
   if p1.Turn > p2.Turn:
   return (p2, p1)
   # Here p1.Turn == p2.Turn for sure
   if p1.speed > p2.speed:
   return (p1, p2)
   if p1.speed < p2.speed:
   return (p2, p1)
   # Here we must randomize
   ....
  

• Dmitri Valentine: Thank you. For order_of_move(p1,p2), why did you choose to use several if statements instead of if/elif/else?
• vnp: @DmitriValentine I see it more clear. An immediate return supersedes else.
• Dmitri Valentine: but won't it check each if statement anyways? This would end up giving the turn to the final if statements which depend on the random choice (or what you have written the speed and not the move's internal priority).

Answer 2

So, I'm no Pythonista, but it's not often that you see nested classes. That's not to say there aren't uses that I don't know of. To change your class into what I would think is a more Pythonic layout, I might do something like:

class Fighter:
 def __init__(self, name, hp=100, attack=75, defense=50, speed=60):
 self.name = name
 self.hp = hp
 self.attack = attack
 self.defense = defense
 self.speed = speed
 def punch(self, target, modifier=1.00):
 base = 50/100.0
 damage = (self.attack - target.defense) * base
 target.hp = target.hp - damage
 print(self.name, "punched", target.name, "for %d HP" % damage)
 punch.priority = 1
 def heal(self, target, modifier=1.00):
 self.hp += 15
 if self.hp > 100:
 self.hp = 100
 print(self.name, "healed 15 HP")
 heal.priority = 2
 def kick(self, target, modifier=1.00):
 base = 100/100.0
 damage = (self.attack - target.defense) * base
 target.hp = target.hp - damage
 self.hp = self.hp - 10
 print(self.name, "kicked", target.name, "for %d HP" % damage)
 kick.priority = 2

Note that this uses the fact that, in Python, a function can have member data. The line punch.priority = 1 works perfectly fine. I also expanded your variable names like atk into attack as it's more readable and you won't have to remember "how did I abbreviate that" later on. Also, I added a "name" attribute. Finally, I changed the use of print to use comma-separated arguments. The default delimiter is a space, so this will work in the same way and reads better.

Next, we need to update the following few lines based on the previous changes. This is now more concise.

# Declare Marco & Player 
Player = Fighter("you")
Marco = Fighter("Marco")

Now, for you function play, we're going to need to make some structural changes. Manually binding the Turn class attribute, when it hasn't been defined, makes the book-keeping here more complicated than it needs to be. Maybe, we could add a class attribute like next_move to keep track of this.

class Fighter:
 def __init__(...):
 # ...
 self.next_move = None

Then, to also help things out, let's embed the hp check into the move definition. We can do this to all of the moves with minimal code using a construct called a decorator. Let's add a function called hp_check, which takes a function and returns a function, outside of our class. Then, let's update our class methods using @hp_check right before the def line.

def hp_check(function):
 def decorator(self, *args, **kwargs):
 if self.hp > 0:
 function(self, *args, **kwargs)
 else:
 print(self.name, "is dead and cannot perform actions")
 return decorator
class Fighter:
 def __init__(...):
 # method body
 @hp_check
 def punch(...):
 # method body
 @hp_check
 def heal(...):
 # method body
 @hp_check
 def kick(...):
 # method body

With this work done, we can make the play function easier to read.

def play(Player1,Player2):
 # First - Check Options Priority
 if Player1.next_move is None and Player2.next_move is None:
 pass
 elif Player1.next_move is None:
 Player2.next_move(Player1)
 elif Player2.next_move is None:
 Player1.next_move(Player2)
 elif Player1.next_move.priority > Player2.next_move.priority:
 Player1.next_move(Player2)
 Player2.next_move(Player1)
 elif Player1.next_move.priority < Player2.next_move.priority:
 Player2.next_move(Player1)
 Player1.next_move(Player2)
 # Second - Check Player's Speed
 elif Player1.speed > Player2.speed:
 Player1.next_move(Player2)
 Player2.next_move(Player1)
 elif Player1.speed < Player2.speed:
 Player2.next_move(Player1)
 Player1.next_move(Player2)
 # Third - Randomly Choose
 else:
 if random.choice(range(2)) == 0:
 Player1.next_move(Player2)
 Player2.next_move(Player1)
 else: 
 Player2.next_move(Player1)
 Player1.next_move(Player2)

Finally, we can do make the main fight sequence more concise by using an OrderedDict.

from collections import OrderedDict
move_choices = OrderedDict((('1', 'punch'), ('2', 'kick'), ('3', 'heal')))
while Player.hp > 0 and Marco.hp > 0:
 player_choice = 'place holder'
 while player_choice not in move_choices:
 player_choice = input('')
 player_method = getattr(Player, player_choice) 
 setattr(Player, 'next_move', player_method)
 if Marco.hp < random.choice(range(80,100)):
 marco_choice = random.choice(move_choices.values())
 else:
 marco_choice = random.choice(move_choices.values()[:-1])
 marco_method = getattr(Marco, marco_choice)
 setattr(Marco, 'next_move', marco_method)
 play(Player, Marco)

Now, this last snippet ramps things up a bit. Instead of referencing __dict__, which I would try to avoid, it uses the built-in functions getattr and setattr which can be used to get and set object attributes. It also uses an OrderDict in order to simplify the input process. I can comment further on anything you want, but hopefully you're at the point where you can go through line by line and figure out what's going on. It's usually helpful to open up an interpreter to test things out. I removed the comments and input lines for brevity, but the functionality should be the same.

In addition to what was mentioned above, I also made a lot of small style changes for readability. Let me know if you have any questions or curiosities. At the end, my updated version of your code looks like:

def hp_check(function):
 def decorator(self, *args, **kwargs):
 if self.hp > 0:
 function(self, *args, **kwargs)
 else:
 print(self.name, "is dead and cannot perform actions")
 return decorator
class Fighter:
 def __init__(self, name, hp=100, attack=75, defense=50, speed=60):
 self.name = name
 self.hp = hp
 self.attack = attack
 self.defense = defense
 self.speed = speed
 @hp_check
 def punch(self, target, modifier=1.00):
 base = 50/100.0
 damage = (self.attack - target.defense) * base
 target.hp = target.hp - damage
 print(self.name, "punched", target.name, "for %d HP" % damage)
 punch.priority = 1
 @hp_check
 def heal(self, target, modifier=1.00):
 self.hp += 15
 if self.hp > 100:
 self.hp = 100
 print(self.name, "healed 15 HP")
 heal.priority = 2
 @hp_check
 def kick(self, target, modifier=1.00):
 base = 100/100.0
 damage = (self.attack - target.defense) * base
 target.hp = target.hp - damage
 self.hp = self.hp - 10
 print(self.name, "kicked", target.name, "for %d HP" % damage)
 kick.priority = 2
def play(Player1,Player2):
 # First - Check Options Priority
 if Player1.next_move is None and Player2.next_move is None:
 pass
 elif Player1.next_move is None:
 Player2.next_move(Player1)
 elif Player2.next_move is None:
 Player1.next_move(Player2)
 elif Player1.next_move.priority > Player2.next_move.priority:
 Player1.next_move(Player2)
 Player2.next_move(Player1)
 elif Player1.next_move.priority < Player2.next_move.priority:
 Player2.next_move(Player1)
 Player1.next_move(Player2)
 # Second - Check Player's Speed
 elif Player1.speed > Player2.speed:
 Player1.next_move(Player2)
 Player2.next_move(Player1)
 elif Player1.speed < Player2.speed:
 Player2.next_move(Player1)
 Player1.next_move(Player2)
 # Third - Randomly Choose
 else:
 if random.choice(range(2)) == 0:
 Player1.next_move(Player2)
 Player2.next_move(Player1)
 else: 
 Player2.next_move(Player1)
 Player1.next_move(Player2)
Player = Fighter("you")
Marco = Fighter("Marco")
from collections import OrderedDict
move_choices = OrderedDict((('1', 'punch'), ('2', 'kick'), ('3', 'heal')))
while Player.hp > 0 and Marco.hp > 0:
 player_choice = 'place holder'
 while player_choice not in move_choices:
 player_choice = input('')
 player_method = getattr(Player, player_choice) 
 setattr(Player, 'next_move', player_method)
 if Marco.hp < random.choice(range(80,100)):
 marco_choice = random.choice(move_choices.values())
 else:
 marco_choice = random.choice(move_choices.values()[:-1])
 marco_method = getattr(Marco, marco_choice)
 setattr(Marco, 'next_move', marco_method)
 play(Player, Marco)

Answer 3

I think you have a few odd patterns to try and solve the problem of choosing an attack and discerning its priority, but there are simpler, more Pythonic approaches to doing this. I would instead make a function for Character called choose_attack.

(As an aside I don't know why you even have Character here. It seems like you'd want to set up proper inheritance down the line more than anything else.)

So in choose_attack, you can ask the player what attack to use, then based on the string you can return the relevant function and its priority. Python allows you to pass references to practically anything, and allows you to return multiple parameters at once, so it's trivial to do something like:

return self.punch, 1 # Attack, Priority

This makes it a lot easier to avoid some of your stranger choices. Now you don't need to attach parameters to functions, nor do you need a subclass of Options, and you've also added the attack choosing logic to the Character class instead of leaving it in the main game logic.

For the most robust solution, I'd store the priority values in a dictionary, paired with the string command the user enters, and then I can return that dictionary value.

Like this:

class Character:
 PRIORITY = {
 "punch": 1
 "heal": 2
 "kick": 2
 }
 def choose_attack():
 while playerchoice not in (1,2,3):
 playerchoice = input("\n Which move would you like to use: \
 \n 1) Punch -- Base Damage of 50 (Weak) \
 \n 2) Kick -- Base Damage of 100 (Moderate), but lose 10 HP\
 \n 3) Heal -- Heal for 15 HP. \
 \n -->")
 # Convert Player Choice to String for Dict-Search
 if playerchoice == 1:
 return self.punch, PRIORITY['punch']
 elif playerchoice == 2:
 return self.kick, PRIORITY['kick']
 else:
 return self.heal, PRIORITY['heal']

Now you can just call ie like this:

attack, priority = Player.choose_attack()

And you can just call attack like a normal function, but it will specifically refer to Player.