5
\$\begingroup\$

I was wondering if anyone had anything to recommend for how to clean up the code I made below. Essentially I am asking if my code is messy or not and if there are any recommendations for my improvement.

# Most basic fantasy combat simulator
import random

p_health = 5
g_health = 5
goblin_alive = True

def status():
    if g_health == 5:
        return "\nA menacing goblin stands before you..."
    elif g_health >= 3:
        return "\nThe goblin is looking a little tired, and is bleeding..."
    elif g_health >= 1:
        return "\nThe goblin is bleeding horribly and looks enraged..."

while goblin_alive:
    if g_health <= 0:
        goblin_alive = False
        print("\nCongrats you slayed the goblin!")
        again = input("Play again? Y/N: ")

        if again == 'y' or again == 'Y':
            goblin_alive = True
        elif again == 'N' or again == 'n':
            print("\nGoodbye")
            exit()
    if p_health <= 0:
        print("Oh dear you have died horribly and the goblin cuts your head off for a trophy...")
        again = input("Play again? Y/N: ")

        if again == 'y' or again == 'Y':
            p_health = 5
            g_heath = 5
            goblin_alive = True
        elif again == 'N' or again == 'n':
            print("\nGoodbye")
            exit()

    desc = status()
    print(desc)
    print("You have " + str(p_health) + ' hit points.')

    attack = input("Press enter to attack: ")
    if attack == '':
        print("\nYou swing your sword fiercely at the goblin!")
        hit_type = random.randint(1, 2)
        if hit_type == 1:
            damage = random.randint(1, 3)
            print("You deal a fierce blow for " + str(damage) + " damage to the goblin.")
            g_health = g_health - damage
        elif hit_type == 2:
            damage = random.randint(1, 3)
            print("The goblin slashes you for " + str(damage) + " damage, uh oh...")
            p_health = p_health - damage
    else:
        print("\nYou better do something...")
\$\endgroup\$
  • \$\begingroup\$ @close voters, why? \$\endgroup\$ – IEatBagels Feb 12 at 18:57
1
\$\begingroup\$

The code was clear and readable. Good job.

Bug?

That said, there appears to be a bug in the "play again" logic:

    if again == 'y' or again == 'Y':
        goblin_alive = True
    elif again == 'N' or again == 'n':
        print("\nGoodbye")
        exit()

It seems like you mark the goblin as alive, but don't give it any health. So the next game the goblin will start out ... dead?

More Functions!

I suggest that you write some more functions. There are places in your code where you are "repeating yourself", and this violates the DRY principle (don't repeat yourself)!

Specifically, there is this code in the "goblin health" section:

    again = input("Play again? Y/N: ")

    if again == 'y' or again == 'Y':
        goblin_alive = True
    elif again == 'N' or again == 'n':
        print("\nGoodbye")
        exit()

There is similar code (but better) in the "player health" section. I'd suggest that you write a function called play_again() that asks the question and evaluates the response and returns either True or False.

Then you can write another function, reset_game() that resets the global variables for you (you'll want to use the global keyword for this).

You can then write code like:

if play_again():
    reset_game()
else:
    exit()

Magic Numbers

Finally, I encourage you to define a pair of constants to use in place of the magic numbers 1 and 2:

# Near the top of the module
PLAYER_HITS = 1
GOBLIN_HITS = 2

    # Later in the code:
    if hit_type == PLAYER_HITS:
        damage = random.randint(1, 3)
        print("You deal a fierce blow for " + str(damage) + " damage to the goblin.")
        g_health = g_health - damage
    elif hit_type == GOBLIN_HITS:
\$\endgroup\$
  • \$\begingroup\$ Thank you! The magic numbers is definitely something I should be aware of, I also hadn't noticed the play again bug, and I believe you were correct, the goblin was indeed dead upon playing again. I appreciate the tips! My next goal is to create similar programs to practice the above concepts \$\endgroup\$ – wildslothin Feb 11 at 17:17
1
\$\begingroup\$

If you want to continue expanding this game, you should consider moving the actors to classes. So you can have a Unit class, which is subclassed by both Player and Goblin:

First, the Unit class. A unit should have health, maximum health, attack and a name. It can attack other things, be attacked, display its status, be dead or alive and die:

class Unit:
    def __init__(self, name, health, attack_die):
        self.name = name
        self.max_health = health
        self.health = health
        self.attack_damage = attack_die

    def __str__(self):
        return f"{self.name} ({self.health}/{self.max_health} HP, {self.attack_damage[0]}-{self.attack_damage[1]} ATK)"

    @property
    def alive(self):
        return self.health > 0

    def attack(self, other):
        damage = random.randint(*self.attack_damage)
        other.being_attacked(damage)
        return damage

    def being_attacked(self, damage):
        self.health -= damage

    def die(self, from_what=None):
        if from_what is None:
            print(f"\nCongrats you slayed the {self.name}!")
        else:
            print(f"{self.name} was slayed by {from_what}!")

Now, the Goblin class just adds your flavor texts on top of that as well as setting the values fro health and attack:

class Goblin(Unit):
    def __init__(self):
        super().__init__("Goblin", 5, (1, 3))

    def __str__(self):
        if self.health == self.max_health:
            return "\nA menacing goblin stands before you..."
        elif self.health >= 0.5*self.max_health:
            return "\nThe goblin is looking a little tired, and is bleeding..."
        elif self.health >= 0:
            return "\nThe goblin is bleeding horribly and looks enraged..."

    def attack(self, other):
        damage = super().attack(other)
        print(f"The goblin slashes you for {damage} damage, uh oh...")

And finally the Player class. It also mostly just adds nice prints:

class Player(Unit):
    def __init__(self):
        super().__init__("Player", 5, (1, 3))

    def __str__(self):
        return f"You have {self.health} hit points."

    def attack(self, other):
        damage = super().attack(other)
        print(f"You deal a fierce blow for {damage} damage to the {other.name}.")

    def die(self, from_what):
        if isinstance(from_what, Unit):
            print(f"\nOh dear you have died horribly and the {from_what.name} cuts your head off for a trophy...")
        else:
            print(from_what)

Now most of the complicated stuff is abstracted away and actually running the game becomes quite minimal:

def play_round(player, enemy):
    units = [player, enemy]
    while player.alive and enemy.alive:
        print(enemy)
        print(player)
        attack = input("Press enter to attack: ")
        if attack == '':
            random.shuffle(units)
            attacker, defender = units
            print(f"\nYou swing your sword fiercely at the {enemy.name}!")
            attacker.attack(defender)
        else:
            print("\nYou better do something...")
    if player.alive:
        enemy.die()
    else:
        player.die(enemy)


def play_game():
    again = True
    while again:
        play_round(Player(), Goblin())
        again = input("Play again? Y/N: ").lower() == "y"
    print("\nGoodbye")
    exit()

if __name__ == "__main__":
    play_game()
\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you, I found this very helpful as I've considered fidgeting with the code to try and expand it, currently learning a bit about modules, this has been a great exercise to get some practical knowledge on some of the basic concepts i've learned with python so far. \$\endgroup\$ – wildslothin Feb 11 at 17:11
1
\$\begingroup\$

Your code looks good and was easy to understand, at least for me :)

A few things you can do to improve it:

Python String Format and Upper

Consider using python format() method, instead of:

print("You have " + str(p_health) + ' hit points.')

try:

print('You have {} hit points.'.format(p_health))

And when checking if a char equals to a given char, you don't have to check both cases you can just use Python upper/lower function so instead of:

if again == 'y' or again == 'Y':

try:

if again.lower() == 'y':
    do stuff

Better Practice

The status() function could be improved by assigning a value to result and returning it, this is just better practice:

def status():
    res = ''
    if g_health == 5:
        res = "\nA menacing goblin stands before you..."
    elif g_health >= 3:
        res = "\nThe goblin is looking a little tired, and is bleeding..."
    elif g_health >= 1:
        res = "\nThe goblin is bleeding horribly and looks enraged..."
    return res

Python Class

Perhaps this code will be better in a GoblinGame class, this class will have properties such as goblin_health, player_health and goblin_alive that can be assigned when initializing the class instance, and a run() method to play a round of the game, I've wrote a quick sample class you can follow along from there:

class GoblinGame:
    def __init__(self):
        self.player_health = 5
        self.goblin_health = 5

    def goblin_alive(self):
        return self.goblin_health > 0

    @property
    def status(self):
        res = ''
        if self.goblin_health == 5:
            res = "\nA menacing goblin stands before you..."
        elif self.goblin_health >= 3:
            res = "\nThe goblin is looking a little tired, and is bleeding..."
        elif self.goblin_health >= 1:
            res = "\nThe goblin is bleeding horribly and looks enraged..."
        return res

    def run(self):
        while self.goblin_alive:
         # write the game logic...

Good Luck!

\$\endgroup\$
  • 1
    \$\begingroup\$ I fail to see how the changes improve the status function. Can you expand a bit on why you consider it's better? \$\endgroup\$ – Mathias Ettinger Feb 10 at 17:17
  • \$\begingroup\$ I feel like the changes for the status function might help me get in the practice of using variables that are a bit easier to change? I also was wondering about the status function changes. \$\endgroup\$ – wildslothin Feb 11 at 17:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.