4
\$\begingroup\$

I'm new to learning OOP in python and played around with it, making this quick battle terminal based game-ish, but I'm wondering how I can optimize this to maybe less lines of code and just overall being more elegant?

import random

class Enemy():
    def __init__(self, species, enemy_health):
        self.species = species
        self.enemy_health = enemy_health

    def heal(self):
        self.enemy_health += random.randint(0, 5)

    def lose_enemy_health(self):
        self.enemy_health -= random.randint(0, 20)

class Player():
    def __init__(self, player, player_health):
        self.player = player
        self.player_health = player_health

    def heal(self):
        self.player_health += random.randint(0, 5)

    def lose_player_health(self):
        self.player_health -= random.randint(0, 20)

enemies = ["Vampire", "Zombie", "Wassup", "1N4RR"]
enemy_fight = random.choice(enemies)
enemy = Enemy(enemy_fight, 50)
player = input("Enter your player name: ")
player = Player(player, 50)
print(">Welcome",player.player,"to text based RPG!!\n")
print(">A wild",enemy.species,"has appeared! You must fight it!!\n")


while enemy.enemy_health or player.player_health > 0:
    player_move = input(">Attack or heal?: \n").lower()
    if player_move == "attack":
        enemy.lose_enemy_health()
        if enemy.enemy_health < 1:
            print(">The enemy died!\n")
            break
        else:
            print(">The enemy now only has",enemy.enemy_health,"health!\n")

    elif player_move == "heal":
        player.heal()
        print(">You healed and now have",player.player_health,"health\n")

    enemy_moves = ["attack", "heal"]
    enemy_move = random.choice(enemy_moves)

    if enemy_move == "attack":
        player.lose_player_health()
        if player.player_health < 1:
            print(">You died loser!!\n")
            break
        else:
            print(">The enemy attacked!\n")
            print(">You now only have",player.player_health,"health!\n")

    elif enemy_move == "heal":
        enemy.heal()
        print(">The enemy healed and now has",enemy.enemy_health,"health!\n")
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Fun little game, easy to win though :P

As for the code:
Naming things enemy in enemy class is a bit double. Same goes for player

class Enemy():
    moves = ["attack", "heal"]
    
    def __init__(self, species, health):
        self.species = species
        self.health = health

    def heal(self):
        self.health += random.randint(0, 5)

    def lose_health(self):
        self.health -= random.randint(0, 20)

Since its called enemy_moves, why not add it to you enemy class. moves = ["attack", "heal"]

def make_a_move(self):
        move = random.choice(self.moves)
        return move

I put moves as a variable of the class. This way you don't create a new list each loop.

self.player = player should be self.name = name (because it is a name)

The is no validation if the input is correct. if you make a typing mistake you skip a turn

player_move = input(">Attack or heal?: \n").lower()
if player_move == "attack":
    enemy.lose_enemy_health()
    if enemy.enemy_health < 1:
        print(">The enemy died!\n")
        break
    else:
        print(">The enemy now only has",enemy.enemy_health,"health!\n")

elif player_move == "heal":
    player.heal()
    print(">You healed and now have",player.player_health,"health\n")

So keep on repeating the question till user gives valid option

player_move = ""
while player_move not in ['attack', 'heal']:
    player_move = input(">Attack or heal?: \n").lower()

Since you wanna go full OOP, try put everything in classes

class Game():
    def play():
        enemies = ["Vampire", "Zombie", "Wassup", "1N4RR"]
        enemy_fight = random.choice(enemies)
        enemy = Enemy(enemy_fight, 50)
        player = input("Enter your player name: ")
        player = Player(player, 50)
        print(">Welcome",player.player,"to text based RPG!!\n")
        print(">A wild",enemy.species,"has appeared! You must fight it!!\n")


        while enemy.health or player.health > 0:
            player_move = ""
            while player_move not in ['attack', 'heal']:
                player_move = input(">Attack or heal?: \n").lower()
            if player_move == "attack":
                enemy.lose_health()
                if enemy.health < 1:
                    print(">The enemy died!\n")
                    break
                else:
                    print(">The enemy now only has",enemy.health,"health!\n")

            elif player_move == "heal":
                player.heal()
                print(">You healed and now have",player.health,"health\n")

            enemy_move = enemy.make_a_move()

            if enemy_move == "attack":
                player.lose_health()
                if player.health < 1:
                    print(">You died loser!!\n")
                    break
                else:
                    print(">The enemy attacked!\n")
                    print(">You now only have",player.health,"health!\n")

            elif enemy_move == "heal":
                enemy.heal()
                print(">The enemy healed and now has",enemy.health,"health!\n")

Game.play()

Some extra things you might wanna look in:

move the actions to the enemy/player. player.attack(enemy) this will also make it easier in the future if you wanna add multiple enemies player.attack(monster5)

Putting player moves as variable of player. Also cool if you wanna add new moves

Also a little bug I just noticed

while enemy.health or player.health > 0: should be while enemy.health > 0 or player.health > 0:
The old code, would check if(enemy.health) which will return true if enemy_health is set and not 0 (0 and False are pretty much the same thing in python). So anything below 0 will be True (but should be false)

\$\endgroup\$
0
4
\$\begingroup\$

Enemy and Player are really just the same class, right? Nothing is different between them. species and player are really just name.

Your global code should be moved to functions.

You should make use of interpolated ("f") strings.

Your combat loop has some repeated code. You should factor this out into common functions.

Health and damage are probably better-modelled by floating-point quantities generated by random.uniform rather than integer quantities. You can do this even while printing rounded quantities, if you want.

player_health < 1 is more accurately player_health <= 0, particularly if you introduce floating-point health quantities.

Consider converting your stringly-typed action words ("heal", "attack") to an Enum.

You're halfway-OOP. To drink the cool-aid, you could introduce Move and Game classes.

import random
from enum import Enum
from typing import Optional, Iterator, Iterable


class Combatant:
    def __init__(self, name: str, health: float = 50) -> None:
        self.name = name
        self.health = health

    def add_hp(self, delta: float) -> None:
        self.health = max(0., self.health + delta)

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

    def __str__(self) -> str:
        return self.name


class Action(Enum):
    HEAL = 'heal'
    ATTACK = 'attack'


class Move:
    MAX_HP = {
        Action.HEAL: 5,
        Action.ATTACK: -20,
    }

    def __init__(self, actor: Combatant, opponent: Combatant, action: Optional[Action] = None) -> None:
        self.actor = actor
        self.opponent = opponent
        if action is None:
            action = random.choice(tuple(Action))
        self.action = action
        self.hp = random.uniform(0, self.MAX_HP[action])
        if action == Action.HEAL:
            self.target = actor
        else:
            self.target = opponent

    def apply(self) -> None:
        self.target.add_hp(self.hp)

    def __str__(self) -> str:
        desc = f'{self.actor} {self.action.value}ed for {abs(self.hp):.0f} HP.\n'
        if self.target.alive:
            desc += f'{self.target} now has {self.target.health:.0f} health.'
        else:
            desc += f'{self.target} died.'
        return desc


class Game:
    def __init__(self, player_name: str, player_actions: Iterable[Action]) -> None:
        self.player = Combatant(name=player_name)
        self.enemy = Combatant(
            name=random.choice(("Vampire", "Zombie", "Wassup", "1N4RR")),
        )
        self.combatants = (self.player, self.enemy)
        self.player_actions = player_actions

    def play(self) -> Iterator[Move]:
        for action in self.player_actions:
            for move in (
                Move(*self.combatants, action=action),
                Move(*reversed(self.combatants))
            ):
                move.apply()
                yield move
                if not move.target.alive:
                    return


def ask_player() -> Iterator[Action]:
    while True:
        yield Action(input("\nAttack or heal?: ").lower())


def main() -> None:
    game = Game(
        player_name=input("Enter your player name: "),
        player_actions=ask_player(),
    )
    print(f"Welcome {game.player} to text based RPG!")
    print(f"A wild {game.enemy} has appeared! You must fight it!")
    for move in game.play():
        print(move)


if __name__ == '__main__':
    main()
\$\endgroup\$
1
  • \$\begingroup\$ If Enemy and Player are not exactly the same. You might wanna look at Inheritance. And use Combatant as a parent Class \$\endgroup\$ Commented May 10, 2022 at 8:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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