18
\$\begingroup\$

I haven't been learning Python for too long and I was just wondering how this Pokémon style battle looks? It's based off of this: Turn Based Pokémon Style Game. It's my first proper time using classes so I'd love advice or critique on the usage. Also, where I make the CPU more likely to use heal when under 35 health, there must surely be a better way to do that.

# Simple battle simulator in the style of Pokemon. # author: Prendy import random moves = {"tackle": range(18, 26), "thundershock": range(10, 36), "heal": range(10, 20)} class Character: """ Define our general Character which we base our player and enemy off """ def __init__(self, health): self.health = health def attack(self, other): raise NotImplementedError class Player(Character): """ The player, they start with 100 health and have the choice of three moves """ def __init__(self, health=100): super().__init__(health) def attack(self, other): while True: choice = str.lower(input("\nWhat move would you like to make? (Tackle, Thundershock, or Heal)")) if choice == "heal": self.health += int(random.choice(moves[choice])) print("\nYour health is now {0.health}.".format(self)) break if choice == "tackle" or choice == "thundershock": damage = int(random.choice(moves[choice])) other.health -= damage print("\nYou attack with {0}, dealing {1} damage.".format(choice, damage)) break else: print("Not a valid move, try again!") class Enemy(Character): """ The enemy, also starts with 100 health and chooses moves at random """ def __init__(self, health=100): super().__init__(health) def attack(self, other): if self.health <= 35: # increasing probability of heal when under 35 health, bit janky moves_1 = ["tackle", "thundershock", "heal", "heal", "heal", "heal", "heal"] cpu_choice = random.choice(moves_1) else: cpu_choice = random.choice(list(moves)) if cpu_choice == "tackle" or cpu_choice == "thundershock": damage = int(random.choice(moves[cpu_choice])) other.health -= damage print("\nThe CPU attacks with {0}, dealing {1} damage.".format(cpu_choice, damage)) if cpu_choice == "heal": self.health += int(random.choice(moves[cpu_choice])) print("\nThe CPU uses heal and its health is now {0.health}.".format(self)) def battle(player, enemy): print("An enemy CPU enters...") while player.health > 0 and enemy.health > 0: player.attack(enemy) if enemy.health <= 0: break print("\nThe health of the CPU is now {0.health}.".format(enemy)) enemy.attack(player) if player.health <= 0: break print("\nYour health is now {0.health}.".format(player)) # outcome if player.health > 0: print("You defeated the CPU!") if enemy.health > 0: print("You were defeated by the CPU!") if __name__ == '__main__': battle(Player(), Enemy())
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Supereffective hits, special conditions (poison ...), critical hits, accuracy, stats personalized for each Pokemon etc.. coming in the next iterations? \$\endgroup\$ Commented Aug 13, 2015 at 21:34
  • 1
    \$\begingroup\$ Yeah, I definitely want to make it more complex. Just wanted to see how my basic code was as I'm not too experienced or confident in making complex things \$\endgroup\$ Commented Aug 13, 2015 at 21:41

2 Answers 2

Reset to default
14
\$\begingroup\$

Magic numbers

Right off the bat I see some magic numbers

moves = {"tackle": range(18, 26), "thundershock": range(10, 36), "heal": range(10, 20)}

What does that mean? Being familiar with Pokemon I'd assume damage, or something, but that won't necessarily be apparent to the user.

ABCs

You have your base class, Character. It would benefit from being an abstract base class

For example,

import abc class Character(metaclass=abc.ABCMeta): def __init__(self, starting_health): self.current_health = starting_health @abc.abstractmethod def attack(self, other): raise NotImplementedError

I've done a few things here. For one, I've used more clear variable names. The input parameter health is actually the starting_health of the character, while self.health is actually referring to the current_health of the character. Better variable names make code easier to read.

The meat of this is the abc stuff. By giving the Character class a metaclass of abc.ABCMeta (don't worry about what a metaclass is) we're saying that it cannot be instantiated directly if it has any abstract methods or properties. With this definition, if you then tried to do this

char = Character(100)

you would get the following error:

TypeError: Can't instantiate abstract class Character with abstract methods attack

This carries into sub-types as well. It is a way of guaranteeing that all classes you instantiate that should override a method do override it.

Moves

Your moves should probably be classes. This will make it much, much easier to extend this, and simplify some other behaviors. I'd look at something like this

from enum import Enum DamageTypes = Enum('DamageTypes', 'DAMAGING HEALING STATUS') Types = Enum('Types', 'ELECTRIC NORMAL') class Move(metaclass=abc.ABCMeta): @abc.abstractproperty def damage_type(self): return NotImplemented @abc.abstractproperty def move_type(self): return NotImplemented @abc.abstractmethod def health_change(self, modifiers=None): return NotImplemented class Thundershock(Move): _max = 36 _min = 10 @property def damage_type(self): return MoveTypes.DAMAGING @property def move_type(self): return Types.ELECTRIC def health_change(self, modifiers=None): if modifiers is None: return random.randint(self._min, self._max) else: # Do something here if they have some ability that reduces electric damage, or whatever

I did a few things here. The first was the Enums. Enums let you group related constants together. For example, now I can do something like

if move.move_type is Types.ELECTRIC: # Has lightningrod ability, immune to electric moves return 0

without having a magic number. I then gave the class some properties (i.e. attributes that I can get/set without using parentheses) as well as some methods (attributes that are functions). This gives you a slightly more well organized codebase, and is easy to extend. Just add a new subclass.

Now in your Player class you can do something like

def attack(self, opponent) while True: try: move = moves[str.lower(input("stuff"))] except KeyError: print("Not a valid move, try again!") else: if move.move_type is MoveTypes.HEAL: self.health += move.health_change(None) elif move.move_type is MoveTypes.DAMAGIN: opponent.health -= move.health_change(None) else: opponent.status = move.status_effect(None) break

which to me is much cleaner, and also easier to extend if you add move types. It doesn't rely on the specific strings of the names (can you imagine typing all ~600 moves that exist in Pokemon?) just on what sort of effect they have.

Weighted randomness

I won't repeat everything here, but Ned Batchelder has a good suggestion for how to handle weighted randomness here

Attack order

You should randomise this. Good guys don't always go first :)

String formatting

Instead of doing 

print("The health of the CPu is now {0.health}.".format(enemy)

just do

print("The health of the CPU is now {}.".format(enemy.health)

Improving your base class

As per @200_success's answer below, you should expand the functionality of your Character base class. For example, attacking, healing, and taking damage are all shared between characters (mostly). What if you did this?

class Character(metaclass=abc.ABCMeta): def __init__(self, starting_health): self.current_health = starting_health def attack(self, other, modifiers): move = self.get_move() if move.move_type is MoveTypes.DAMAGING: other.damage(move.health_change(modifiers)) elif move.move_type is MoveTypes.HEAL: self.heal(move.health_change(modifiers)) elif move.move_type is MoveTypes.STATUS: other.status(move.status_effect(modifiers)) else: raise NotImplementedError @abc.abstractmethod def get_move(self): raise NotImplementedError

I've consolidated the act of attacking and abstracted away the only part that changes - how they pick their move. Now you can do 

 class Player(Character): def get_move(self): while True: move = moves[str.lower(input(""))] except KeyError: print("No such move") else: return move class Enemy(Character): def get_move(self): # do some pseudo random stuff return move

And you don't have to repeat the attacking logic, and new types of characters only need to override how they pick a move.

\$\endgroup\$
8
  • 1
    \$\begingroup\$ Bloody hell! Thanks for all this, its really helpful. I definitely forgot about the going first thing, although the CPU definitely doesn't deserve to go first. :) \$\endgroup\$ Commented Aug 13, 2015 at 22:04
  • 1
    \$\begingroup\$ @Prendy I've updated my answer to address the issue mentioned in 200_success's answer. Please take a look and let me know what you think. \$\endgroup\$ Commented Aug 14, 2015 at 16:29
  • \$\begingroup\$ Looks good, I've been trying to understand the Enum thing, I can't seem to get it to work. Like if I try to reference Thundershock.damage_type it doesn't seem to exist. \$\endgroup\$ Commented Aug 14, 2015 at 20:54
  • 1
    \$\begingroup\$ @Caridorc that isn't strictly true. It works with small integers, booleans, interned strings (in CPython), and things that are actually the same. It never "behaves weirdly" - it behaves exactly like it should. In the case of Enums it is referring to the same member variable, in which case is is appropriate \$\endgroup\$ Commented Aug 15, 2015 at 23:59
  • 1
    \$\begingroup\$ @Peter move_type is a property, meaning that while you access it as if it were an attribute, it's really a function. In this case we just return Types.ELECTRIC all the time - but what if one of them used a move/ability/something that changes a Pokemon's type? Then we wouldn't want this to be hardcoded - we'd rather do some computation to determine what the actual type is. In this simple example, you could just do what you asked about, but making it a property allows for more flexibility in the future (although tbh switching to a property is super easy in Python). \$\endgroup\$ Commented Feb 5, 2016 at 1:28
6
\$\begingroup\$

The Character base class doesn't really serve a purpose here. In Java, for example, an abstract base class would be essential. In Python, not so much. There are two approaches you could take.

The simple change would just be to eliminate the Character class and use duck typing. The two subclasses aren't inheriting any useful functionality. You could just set self.health in their respective constructors. The attack() method is just an unimplemented stub.

Alternatively, you could define a Character class that has all of the healing and attacking logic. (After all, that logic is shared between Player and Enemy by copy-and-pasting, which is not ideal.) The HumanPlayer and CPU classes would just be responsible for the decision-making process — by prompting for user input and by making random choices, respectively.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.