HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Battle simulator RPG

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
simulatorbattlerpg

Problem

I just learned about classes today and wanted to find a way to implement them that interested me. I whipped up this little battle simulator and would like any feedback and critiques as well as any ideas or similar projects I could pursue.

```
import random
import time
import decimal

rnd = 0

class Unit:
def __init__(self, n, hp, s, d, a, mp):
self.name = n
self.maxHP = hp
self.curHP = hp
self.strength = s
self.defence = d
self.agility = a
self.mp = mp

def describe(self):
print ("HP: "+ str(self.curHP) + " STRENGTH:"+ str(self.strength) + " DEFENCE:" + str(self.defence) + " AGILITY:" + str(self.agility) + " MP:" + str(self.mp))

def battle( a, b ):
turn = 1

if a.agility > b.agility:
attacker = a
defender = b
else:
attacker = b
defender = a

doBattle = True
while (doBattle):
time.sleep(.7)
print ("\n --- Turn " , turn , ": " , attacker.name , " ---" )
print (attacker.name , ": " , attacker.curHP )
print (defender.name , ": " , defender.curHP , "\n")

if defender.agility > attacker.agility and random.randint(1,4) == 4:
print (attacker.name , " missed." )
else:
if random.randint(1,5) != random.randint(1,6):
multiplier_1 = random.randint(70,150)/100
multiplier_2 = random.randint(70,150)/100
attackDamage = round(decimal.Decimal(attacker.strengthmultiplier_1),1) - round(decimal.Decimal(defender.defencemultiplier_2),1)
if attackDamage = 0:
defender.mp -= 1
defender.curHP += random.randint(4,16)
if defender.curHP > 0:
print("You try to use your MP to heal!")
time.sleep(2)
print("It worked! HP: " + str(defender.curHP))
else:
print("You try to use your MP to heal!")
time

Solution

Modules

Since you only use the sleep function from the time module, you should emphasize it by declaring from time import sleep and change your calls to only sleep. I find it cleaner even though it's not a strong rule of thumb, taste varies and both forms are acceptable.

I’d recommend getting rid of the decimal module. You have no need of strict floating point computation and since you are rounding (thus converting to floats) anyway, you loose it's interest right after using it. Better round the final result of the computation and compute intermediate results using good-old floats.

String representation

What do you do when you want to output the content of a dictionary? You print it. What about lists? You print them. Integers, sets, tuples? print them all.

Same goes for your Units. You should print(character) instead of character.describe(). How to do that without getting outputs like `? By implementing the __str__ special method.

Object-Oriented Programming

Your functions does a lot of things and nearly all of them are directly related to your
Units. You should both split them into smaller functions that (for now) do little but have meaning (and meaningful names) and make those function Unit's methods. You will be able to more easily extend them latter if you decide to add more mechanisms to your battles.

Check if a
Unit is alive, heal it, check whether of two Units is the swifter are all actions that can reasonably have their own function. For now the code will be simple but if you plan on adding a loot system with boost items, potions or even curses, you’ll have an easier time managing the whole logic because you will only have to include things in these tiny functions.

Similarly, since these functions are directly related to
Units and perform actions on them, they should be implemented as methods in your class.

Loops

Using a flag to end a
while loop on certain condition feels not quite right. You should either:

  • loop forever and break on such conditions;



  • use the condition as the condition of the while itself.



If the condition is complex, a function call might even make things more readable. In
battle the while condition should be “neither a nor b is dead”.

For your main loop, you should also try to sanitize your input. To me,
'y', 'Y', ' y', ' Yes ', '' are all valid values to continue. A common idiom is to use input(...).strip() and even input(...).strip().lower() if you want to check into a set of predefined values.

General improvements

-
Using functions/methods can also be a way to avoid repeating the same lines of code. You should also try to avoid it in
if...else. For instance when computing the result of an attack:

attackDamage = round(decimal.Decimal(attacker.strength*multiplier_1),1) - round(decimal.Decimal(defender.defence*multiplier_2),1) 
if attackDamage < 0: 
    attackDamage = 0 
defender.curHP = defender.curHP - attackDamage 
print (attacker.name , " did " , attackDamage , " damage to " , defender.name , "." )


and:

attackDamage = round(decimal.Decimal(attacker.strength*2),1) - round(decimal.Decimal(defender.defence/2),2)
if attackDamage < 0: 
    attackDamage = 0 
defender.curHP = defender.curHP - attackDamage 
print ("CRITICAL HIT!!!!!" + attacker.name , " did " , attackDamage , " damage to " , defender.name , "." )


are pretty similar. You can avoid it by setting only the multipliers in the test and performing the other operations “out” of the
if.

-
Python allow you to unpack iterables into several variables at once. See here and here for details. Combined with implicit tuples it allows to simply write:

attacker, defender = defender, attacker


if you want to swap the two variables.

-
Similarly, in some places, using the ternary operator
x if condition else y will make your code shorter and cleaner.

-
The builtin function
max will also help you make the code more readable.

-
Spacing is important for readability and should be consistent: no space before an opening parenthesis in a function call, one space after a coma… You should also try to limit your line length to 80 characters. Read PEP 8 for a full list of recommendations and some working examples.

Also
print already uses a space as separator between arguments. You don't need to add them manually. Or, if you want to explicitly position them, you can override print default separator with the sep keyword:

print('Hello', 'world!', sep='-') # outputs 'Hello-world!'


Bugs

-
Unit's maxHP is set but never used. It sounds to me like you forgot to implement what you intended it for.

-
A
battle will continue if the defender died and failed to use it’s mp to heal itself.

Proposed Improvements

I used
getattr and setattr to dynamically update the attribute represented by the string chosen by random.choice`. If you’re not used to it and don't quite get it, your approach is fine. J

Code Snippets

attackDamage = round(decimal.Decimal(attacker.strength*multiplier_1),1) - round(decimal.Decimal(defender.defence*multiplier_2),1) 
if attackDamage < 0: 
    attackDamage = 0 
defender.curHP = defender.curHP - attackDamage 
print (attacker.name , " did " , attackDamage , " damage to " , defender.name , "." )
attackDamage = round(decimal.Decimal(attacker.strength*2),1) - round(decimal.Decimal(defender.defence/2),2)
if attackDamage < 0: 
    attackDamage = 0 
defender.curHP = defender.curHP - attackDamage 
print ("CRITICAL HIT!!!!!" + attacker.name , " did " , attackDamage , " damage to " , defender.name , "." )
attacker, defender = defender, attacker
print('Hello', 'world!', sep='-') # outputs 'Hello-world!'
import random
from time import sleep


class Unit: 
    def __init__(self, n, hp, s, d, a, mp): 
        self.name = n
        self.maxHP = hp
        self.curHP = hp
        self.strength = s
        self.defence = d
        self.agility = a
        self.mp = mp

    def __str__(self):
        descr = "[{}] HP: {}, STRENGTH: {}, DEFENSE: {}, AGILITY: {}, MP: {}"
        return descr.format(self.name, self.curHP, self.strength,
                            self.defence, self.agility, self.mp)

    def is_dead(self):
        return self.curHP <= 0

    def perform_attack(self, ennemy):
        if random.randint(1,5) != random.randint(1,6):
            strength_multiplier = random.randint(70, 150) / 100
            defence_multiplier = random.randint(70, 150) / 100
        else:
            strength_multiplier = 2
            defence_multiplier = 0.5
            print("CRITICAL HIT!!!!")
        attackDamage = max(self.strength * strength_multiplier -
                           ennemy.defence * defence_multiplier, 0)
        ennemy.curHP = round(ennemy.curHP - attackDamage, 1)
        print(self.name, "did", attackDamage, "damage to", ennemy.name, ".")

    def heal(self):
        self.maxHP = self.curHP
        self.curHP += 3

    def heal_with_mp(self):
        if self.mp > 0:
            self.mp -= 1
            print(self.name, "try to use an MP to heal!")
            self.curHP += random.randint(4, 16)
            sleep(2)
            if self.is_dead():
                print("No effect…", self.name, "died")
            else:
                print("It worked! HP:", self.curHP)

    def swifter(self, ennemy):
        return self.agility > ennemy.agility

    def battle(self, ennemy):
        # Resolve first attacker based on agility
        # we attack first if there is a draw
        attacker, defender = ((ennemy, self)
                              if ennemy.swifter(self)
                              else (self, ennemy))

        turn = 0
        while not (self.is_dead() or ennemy.is_dead()):
            turn += 1
            print()
            print("---  Turn", turn, ":", attacker.name, "---" )
            print(attacker.name, ":", attacker.curHP)
            print(defender.name, ":", defender.curHP)
            print()
            sleep(.7)

            # Allow less agile defenders to avoid the attack
            # with a lower probability chance
            miss_factor = 4 if defender.swifter(attacker) else 20
            if random.randint(1,miss_factor) == miss_factor: 
                print(attacker.name, "missed.")
            else:
                attacker.perform_attack(defender)

            if defender.is_dead():
                defender.heal_with_mp()

            attacker, defender = defender, attacker

        winner = ennemy.name if self.is_dead() else self.name
        print()
        print()
        print(winner, "won the battle!")

    def encounter(self, ennemy):
        self.battle(ennemy)
        if not self.is_dead():

Context

StackExchange Code Review Q#109839, answer score: 3

Revisions (0)

No revisions yet.