Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I am trying to write a text adventure as my first Python project. For this segment, I wanted a random enemy (part of a class with its own attributes) to appear and then enter a turn based battle with the player until either the enemies health is 0 or the player's health is 0. The player can use one of three moves to attack the enemy. The difficulty to beat the enemy is based on its strength, defence and health attribute.

Feel free to pull it apart and mention improvements or constructive criticisms that you may come up with.

import random, time

player_health = 100

class Enemy:

    def __init__(self, name, strength, defense, health):
        self.name = name
        self.strength = strength
        self.defense = defense
        self.health = health

    def attack_enemy(self):
        time.sleep(1)
        print ("what move would you like to make? (punch, kick or headbutt?")
        print("")
        answer = input()

        if answer == "punch":
            self.health = self.health -     (random.randint(1,100)/(random.uniform(0,1)*     self.defense))
            self.health = int(self.health)
        elif answer == "kick":
            self.health = self.health - (random.randint(1,100)/(random.uniform(0,1)* self.defense))
            self.health = int(self.health)
        elif answer == "headbutt":
            self.health = self.health -   (random.randint(1,100)/(random.uniform(0,1)* self.defense))
            self.health = int(self.health)
        else:
            print("you stumble...")

        time.sleep(1)

        print (self.name + "'s health is now: " + str(int(self.health)))
        print("")

        return int(self.health)

    def enemy_attack(self):
        global player_health
        time.sleep(1)
        print ("The enemy " + self.name + " " + "attacks...")
        print("")
        player_health = player_health - (self.strength * random.uniform(0.1, 1.4))
        player_health = int(player_health)
        time.sleep(1)
        print ("Your health is now " + str(int(player_health)))
        print ("")
        return int(player_health)

    def battle_script(self):
        global player_health
        print ("An enemy " + self.name + " appears...")
        print ("")
        time.sleep(1)
        while player_health > 0 and self.health > 0:
            self.attack_enemy()
            if self.health <=0:
               break
            self.enemy_attack()
            if player_health <=0:
                break
        if self.health <= 0:
            time.sleep(1)
            print ("You have killed the " + self.name)

        if player_health <= 0:
            time.sleep(1)
            print ("Sorry, you are dead")


enemies = [Enemy("Boar", 10, 5, 100), Enemy("Wolf", 20, 10, 100), Enemy("Lion", 30, 20, 100), Enemy ("Dragon", 40, 30, 130)]

random_enemy = random.choice(enemies) 
random_enemy.battle_script()
share|improve this question
    
Welcome to Code Review! Even though we can read your code it would be very helpful if you could add a little description outside of your code about what the purpose of your code is. Doing so will make it easier for reviewers to tear your code apart :) –  Simon André Forsberg Aug 20 at 12:24
    
Hi, thank you :) I am trying to write a text adventure as my first Python project. For this segment, I wanted a random enemy (part of a class with its own attributes) to appear and then enter a turn based battle with the player until either the enemies health is 0 or the player's health is 0. The player can use one of three moves to attack the enemy. The difficulty to beat the enemy is based on its strength, defence and health attributes –  George Wells Aug 20 at 12:41
4  
@GeorgeWells : please add the description of your code to the question. –  JaDogg Aug 20 at 12:50

1 Answer 1

A positive to begin with - your code generally follows the style guide, which is an excellent start. Some docstrings would be nice, though (I have also omitted these, but they are generally a good idea). Also, your imports should really be on separate lines:

import random
import time

Having a global is generally a bad sign. Why not make your Player a class too? This would be very similar to Enemy, so we can do some inheritance:

class Character:

    def __init__(self, health):
        self.health = health

    def attack(self, other):
        raise NotImplementedError

Note that all Characters will have health and be able to attack, although at this point we don't supply an implementation for the latter. This separates the enemy_attack and attack_enemy methods into separate classes, which makes much more sense (the use of self is currently a bit confusing).

class Player(Character):

    def __init__(self, health=100):
        super().__init__(health)

    def attack(self, other):
        answer = input("What move would you like to make (punch, kick or headbutt)? ")
        if answer.lower() in ('punch', 'kick', 'headbutt'):
            other.health -= int(random.randint(1, 100) / 
                                (random.uniform(0, 1) * other.defense))
        else:
            print("you stumble...")

The Player will only have health, and its attack is based on user input. Note the use of super to call the parent __init__, and the use of in to check the user's input.

class Enemy(Character):

    def __init__(self, name, strength, defense, health):
        super().__init__(health)
        self.name = name
        self.strength = strength
        self.defense = defense

    def attack(self, other):
        print("The {0.name} attacks...".format(self))
        other.health -= int(self.strength * random.uniform(0.1, 1.4))

The Enemy has some additional attributes, and the attack is entirely random. Note the use of str.format to provide output.

This isn't perfect, because:

  • a Player relies on other having a defense attribute, so two Players can't attack each other; and
  • health can fall below zero, which doesn't make much sense.

Perhaps you could refactor to solve these problems?


The battle logic is independent of which Characters are involved, so should become a separate function rather than an instance method:

def battle(player, enemy):
    print ("An enemy {0.name} appears...".format(enemy))
    # Combat loop
    while player.health > 0 and enemy.health > 0:
        player.attack(enemy)
        print("The health of the {0.name} is now {0.health}.".format(enemy))
        if enemy.health <= 0:
            break
        enemy.attack(player)
        print("Your health is now {0.health}.".format(player))
    # Display outcome
    if player.health > 0:
        print("You killed the {0.name}.".format(enemy))
    elif enemy.health > 0:
        print("The {0.name} killed you.".format(enemy))

Note that there is no need to break for the player.health, as the loop will end there automatically.


Now the overall game becomes:

if __name__ == '__main__':
    enemies = [Enemy("Boar", 10, 5, 100), Enemy("Wolf", 20, 10, 100),
               Enemy("Lion", 30, 20, 100), Enemy ("Dragon", 40, 30, 130)]
    battle(Player(), random.choice(enemies))

Note the use of if __name__ == '__main__', to allow e.g. import elsewhere without running the loop (so you can use the Characters in other scripts more easily), and the line break in enemies to keep line lengths reasonable.

share|improve this answer
1  
Jon, thank you for you help. Making a player a class will make it a lot easier when I begin writing story-flow! Incredibly helpful to know which new concepts to learn next. –  George Wells Aug 20 at 12:51
    
Hi Jon, quick follow up question. If I was to give the character/player an "inventory",I'm thinking I should store the inventory within the character class? Or would inventory be it's in it's own class, where I would define functions for picking up and using items? –  George Wells Aug 22 at 12:41
    
@GeorgeWells I'm afraid the answer is "it depends". If you want the inventory to have methods, it should be a class, but if not you could just as well use a list or dictionary within the Player to hold the items it has. This question's answers offer a few options, as does my answer to another question. I suggest you pick one and have a go, then refactor if you get stuck! –  jonrsharpe Aug 22 at 12:49

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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