3
\$\begingroup\$

I wrote a fighting game. My code is rough. I would appreciate constructive and critical feedback please. The goal is to become a better programmer/developer.

(I wrote this using 2 Files - The Main Game File & Then a Character File):

pyfight_characters.py

import math
import random

class Berserker(object):
        MAX_hp = 250
        base_hp = 250
        base_speed = 75
        priority = 'Normal'
        mindset_multiplier = 1.00
        move = ''

        def Jab(self,opponent):
            """ Jab - A fast, light attack. Does 30 Basic Damage to Opponent"""
            self.priority = 'Fast'
            opponent.base_hp -= math.floor( 30*self.mindset_multiplier )

        def Haymaker(self,opponent):
            """ Haymaker - A Slow, strong attack. Does 65 Base to Opponent and 30 Base Damage to User"""
            self.priority = 'Slow'
            opponent.base_hp -= math.floor (65*self.mindset_multiplier)
            self.base_hp -= math.floor(30*self.mindset_multiplier)

        def PumpUp(self,opponent):
            """ PumpUp - Normal speed. Improves the Mindset of the User ( x 1.25 )"""
            self.priority = 'Normal'
            self.mindset_multiplier *= 1.25

        move_pool = {'1':Jab,'2':Haymaker,'3':PumpUp}

class Yogi(object):
        MAX_hp = 220
        base_hp = 220
        base_speed = 80
        priority = 'Normal'
        mindset_multiplier = 1.00
        move = ''

        def Sukha(self,opponent):
            """ Sukha - A Normal speed move. Restores up to a Base of 30 HP to User"""
            self.priority = 'Normal'

            if self.base_hp<=self.MAX_hp:
                self.base_hp += math.floor(30*self.mindset_multiplier)
            else:
                self.base_hp = self.MAX_hp

        def Dukha(self,opponent):
            """ Dukha - A Normal-speed attack. Does 40 Base Damage to the Opponent"""
            self.priority = 'Normal'
            opponent.base_hp -= math.floor(40*self.mindset_multiplier)

        def Meditate(self,opponent):
            """ Meditate - A Normal-speed move. Improves the Mindset of the User (x 1.35)"""
            self.priority = 'Normal'
            self.mindset_multiplier *= 1.35

        move_pool = {'1':Sukha,'2':Dukha,'3':Meditate}

class Trickster(object):
        MAX_hp = 175
        base_hp = 175
        base_speed = 100
        priority = 'Normal'
        mindset_multiplier = 1.00
        move = ''

        def Bag_o_tricks(self,opponent):
            """ Bag-O-Tricks - A Normal-speed move. Increases or Decreases Opponent HP anywhere from 0 to 100 and User HP from 0 to 50"""
            self.priority = 'Normal'
            opponent.base_hp -= math.floor ( random.uniform(-1,1)*100*self.mindset_multiplier )
            if opponent.base_hp > opponent.MAX_hp:
                opponent.base_hp = opponent.MAX_hp

            if self.base_hp <= self.MAX_hp:
                self.base_hp += math.floor ( random.uniform(-1,1)*50*self.mindset_multiplier )
            else:
                pass

            if self.base_hp > self.MAX_hp:
                self.base_hp = self.MAX_hp

        def Tease(self,opponent):
            """ Tease - A Normal-speed move. Reduces the Mindset of the Opponent (x 0.8)"""
            self.priority = 'Normal'
            opponent.mindset_multiplier *= 0.9
            self.mindset_multiplier *= 1.05

        def Swapper(self,opponent):
            """ Swapper - A Normal-speed move. Swaps the User HP with the Opponent HP"""
            self.priority = 'Normal'
            self.base_hp, opponent.base_hp = opponent.base_hp, self.base_hp
            self.MAX_hp, opponent.MAX_hp = opponent.MAX_hp, self.MAX_hp

        move_pool = {'1':Bag_o_tricks,'2':Tease,'3':Swapper}

pyfight.py

# PyFight!

import random
import math
import sys
import os
import time
from pyfight_characters import *

class Game(object):

    mode_options = {'1':('Player 1','CPU'), '2':('CPU 1','CPU 2'), '3':('Player 1','Player 2')}
    game_mode = '' # Preallocation
    game_mode_choice = '' # Preallocation and Stroage

    character_options = {'1':Berserker, '2':Yogi, '3':Trickster}
    move_order = '' # Preallocation

    def __init__(self):
        """ """

    def clear_screen(self):
        os.system('cls' if os.name == 'nt' else 'clear')

    def introduction(self):
        game.clear_screen()
        print """
        ______       ______  _         _      _              
        | ___ \      |  ___|(_)       | |    | |             
        | |_/ /_   _ | |_    _   __ _ | |__  | |_  ___  _ __ 
        |  __/| | | ||  _|  | | / _` || '_ \ | __|/ _ \| '__|
        | |   | |_| || |    | || (_| || | | || |_|  __/| |   
        \_|    \__, |\_|    |_| \__, ||_| |_| \__|\___||_|   
                __/ |            __/ |                       
               |___/            |___/                        """
        time.sleep(0.3)
        game.clear_screen()

    def choose_game_mode(self):
        while True:
            self.game_mode_choice = raw_input("What versus mode would you like to play: \
                \n1) Player v. CPU \
                \n2) CPU1 v. CPU2 \
                \n3) Player 1 v. Player 2 \
                \n0) Exit Game.\n\n")

            game.clear_screen()

            if self.game_mode_choice == '0':
                sys.exit()
            try:
                self.game_mode = self.mode_options[ self.game_mode_choice ]
                return None
            except KeyError:
                print "Please choose one of the listed options.\n"
                time.sleep(1)

    def character_selection(self,controller_index):
        while True:
            character_choice = raw_input("Who should " + self.game_mode[controller_index] + " play as:\
                \n1) The Berserker\
                \n2) The Yogi\
                \n3) The Trickster\
                \n4) Random\
                \n\
                \n0) Exit Game.\n\n")

            game.clear_screen()

            if character_choice == '0':
                sys.exit()
            elif character_choice == '4':
                character_choice = str (random.randint(1,3))

            try:
                return self.character_options[ character_choice ]
            except KeyError:
                print "Please choose one of the listed options.\n"
                time.sleep(1)

    def hp_check(self,controller_1, controller_2):
        if controller_1.base_hp <= 0:
            if controller_2.base_hp >0:
                self.clear_screen
                print "Controller 2 Wins!"
                sys.exit()
            else:
                self.clear_screen
                print "It's a Draw!!!"
                sys.exit()
        elif controller_2.base_hp <= 0:
            self.clear_screen
            print "Controller 1 Wins!"
            sys.exit()
        else:
            pass

    def player_choose_move(self,controller,controller_index):
        while True:
            move_choice = raw_input(self.game_mode[controller_index] + ", choose your move:\
            \n1)" + controller.move_pool['1'].__doc__ + "\
            \n2)" + controller.move_pool['2'].__doc__ + "\
            \n3)" + controller.move_pool['3'].__doc__ + "\n\n")

            try:
                controller.move = controller.move_pool[move_choice]
                return None
            except KeyError:
                game.clear_screen()
                print "Please choose one of the listed options.\n"
                time.sleep(1)

    def comp_choose_move(self,controller):
        controller.move = controller.move_pool[ random.choice( ['1','2','3'] )]

    def priority_and_speed_check(self,controller1,controller2):
        if controller1.priority > controller2.priority:
            return True
        elif controller1.priority == controller2.priority:
            if controller1.base_speed > controller2.base_speed:
                return True
            elif controller1.base_speed == controller2.base_speed:
                return bool ( random.randint(0,1) )
            else:
                return False
        else:
            return False

    def update(self,controller1,controller2):
        print self.game_mode[0], " HP:", str(Player_1.base_hp), "\n", self.game_mode[0], "Mindset: x", str(Player_1.mindset_multiplier), "\n"
        print self.game_mode[1], " HP:", str(Player_2.base_hp), "\n", self.game_mode[], "Mindset: x", str(Player_2.mindset_multiplier), "\n"
        print "\n\n"

    def move_collection(self,controller1,controller2):
        if self.game_mode_choice == '1':
            self.update(Player_1,Player_2)
            self.player_choose_move(Player_1,0)
            self.clear_screen()

            self.update(Player_1,Player_2)
            self.comp_choose_move(Player_2)
            self.clear_screen()

        elif self.game_mode_choice == '2':
            self.update(Player_1,Player_2)
            self.comp_choose_move(Player_1)
            self.clear_screen()

            self.update(Player_1,Player_2)
            self.comp_choose_move(Player_2)
            self.clear_screen()

        else:
            self.update(Player_1,Player_2)
            self.player_choose_move(Player_1,0)
            self.clear_screen()

            self.update(Player_1,Player_2)
            self.player_choose_move(Player_2,1)
            self.clear_screen()

    def move_application(self,controller1,controller2):
        if self.priority_and_speed_check(Player_1,Player_2):
            Player_1.move(Player_1,Player_2)
            self.hp_check(Player_1,Player_2)

            Player_2.move(Player_2,Player_1)
            self.hp_check(Player_1,Player_2)
        else:
            Player_2.move(Player_2,Player_1)
            self.hp_check(Player_1,Player_2)

            Player_1.move(Player_1,Player_2)
            self.hp_check(Player_1,Player_2)

game = Game()
game.introduction()
game.choose_game_mode()

( Player_1, Player_2 ) = ( game.character_selection(0)(), game.character_selection(1)() )
while True:
    game.move_collection(Player_1,Player_2)
    game.move_application(Player_1,Player_2)
\$\endgroup\$
  • \$\begingroup\$ @Cornchip All suggestions for improving the code, even trivial observations, should be written as answers, not comments. \$\endgroup\$ – 200_success Mar 24 '17 at 5:29
3
\$\begingroup\$

This is just a review of your characters. As User319 already noted in his answer, you should make a base Character class and inherit from that. I also implemented a Move class which is able to deal with most of your standard moves, and can easily be modified to accommodate more complicated moves.

class Character(object):

    def __init__(self, name, MAX_hp, base_speed, priority="Normal", mindset_multiplier=1.0, move=''):
        self.MAX_hp = MAX_hp
        self.base_hp = MAX_hp
        self.base_speed = base_speed
        self.priority = priority
        self.mindset_multiplier = mindset_multiplier
        self.name = name

        self.move_pool = {}

    def __repr__(self):
        return "{self.name} - {self.base_hp} / {self.MAX_hp} HP, {self.mindset_multiplier} mindset, {self.base_speed} speed".format(self=self)

Each character has a name now, which makes building your list of possible characters easier, because you don't need to hard-code them there, anymore.

The Move class is slightly more complicated. Its initializer has a lot of keyword arguments, which all default to no change of the corresponding attribute. Just like currently, you can call a move with the attacker as the first and (optionally) the opponent as the second argument.

class Move(object):

    def __init__(self, description="", self_damage=0, damage=0, mindset_multiplier=1.0, opponent_mindset_multiplier=1.0, speed_boost=0, opponent_speed_boost=0, priority="Normal"):
        self.__doc__ = description
        self.self_damage = self_damage
        self.damage = damage
        self.mindset_multiplier = mindset_multiplier
        self.opponent_mindset_multiplier = opponent_mindset_multiplier
        self.speed_boost = speed_boost
        self.opponent_speed_boost = opponent_speed_boost
        self.priority = priority

    def __call__(self, attacker=None, opponent=None):
        if attacker is not None:
            attacker.priority = self.priority
            attacker.mindset_multiplier *= self.mindset_multiplier
            attacker.base_speed += self.speed_boost
            attacker.base_hp -= math.floor(self.self_damage *
                                           attacker.mindset_multiplier)
            attacker.base_hp = min(max(attacker.base_hp, 0), attacker.MAX_hp)

        if opponent is not None:
            opponent.mindset_multiplier *= self.opponent_mindset_multiplier
            opponent.base_speed += self.speed_boost
            opponent.base_hp -= math.floor(self.damage *
                                           attacker.mindset_multiplier)
            opponent.base_hp = min(max(opponent.base_hp, 0), opponent.MAX_hp)

A possible improvements would be the ability to use place holders in the docstring, so you could write something like:

Move(" Jab - A fast, light attack. Does {self.damage} Basic Damage to Opponent",
     damage=30, priority="Fast")

With these definitions, your first two classes are quite straight-forward to implement:

class Berserker(Character):

    def __init__(self):
        Character.__init__(self, "Berserker", 250, 75)
        self.move_pool = {"1": Move(" Jab - A fast, light attack. Does 30 Basic Damage to Opponent",
                                    damage=30, priority="Fast"),
                          "2": Move(" Haymaker - A Slow, strong attack. Does 65 Base to Opponent and 30 Base Damage to User",
                                    self_damage=30, damage=65, priority="Slow"),
                          "3": Move(" PumpUp - Normal speed. Improves the Mindset of the User ( x 1.25 )",
                                    mindset_multiplier=1.25)}


class Yogi(Character):

    def __init__(self):
        Character.__init__(self, "Yogi", 220, 80)
        self.move_pool = {"1": Move(" Sukha - A Normal speed move. Restores up to a Base of 30 HP to User",
                                    self_damage=-30),
                          "2": Move(" Dukha - A Normal-speed attack. Does 40 Base Damage to the Opponent",
                                    damage=40),
                          "3": Move(" Meditate - A Normal-speed move. Improves the Mindset of the User (x 1.35)",
                                    mindset_multiplier=1.35)}

The Trickster is slightly more complicated. Here we define two inner classes which inherit from Move for his special moves:

class Trickster2(Character):

    class Bag_o_tricks(Move):

        def __call__(self, attacker, opponent):
            self.damage = random.uniform(-1, 1) * 100
            self.self_damage = random.uniform(-1, 1) * 50
            Move.__call__(self, attacker, opponent)

    class Swapper(Move):

        def __call__(self, attacker, opponent):
            attacker.priority = self.priority
            attacker.base_hp, opponent.base_hp = opponent.base_hp, attacker.base_hp
            attacker.MAX_hp, opponent.MAX_hp = opponent.MAX_hp, attacker.MAX_hp

    def __init__(self):
        Character.__init__(self, "Trickster", 175, 100)
        self.move_pool = {"1": Trickster.Bag_o_tricks(" Bag-O-Tricks - A Normal-speed move. Increases or Decreases Opponent HP anywhere from 0 to 100 and User HP from 0 to 50"),
                          "2": Move(" Tease - A Normal-speed move. Reduces the Mindset of the Opponent (x 0.8)",
                                    mindset_multiplier=1.05, opponent_mindset_multiplier=0.8),
                          "3": Trickster.Swapper(" Swapper - A Normal-speed move. Swaps the User HP with the Opponent HP")}
\$\endgroup\$
3
\$\begingroup\$

I'm not sure what Sukha is supposed to do, but I'm finding it odd. I think what you meant to do was raise the HP, then reset it if it's higher than the max?

It looks like whenever health changes, it's a multiple of the mindset_multiplier. I'd suggest having that in a new function such as:

def adjust_health(self, amount):
    self.base_hp += math.floor(amount * self.mindset_multiplier)

It looks like there are several fields getting initialized to the same thing. I would have a base Character class that the other three inherit from. The base class would set default stats and define the adjust_health method.

I had trouble figuring out why each character had an empty move variable. I'd recommend a comment. Actually, there's probably a better design for that, but I can't think of it atm.

I don't find the "Preallocation" comments useful.

You might have a bug in the character variables. Base HP etc don't have self, so that makes them static variables. I think odd things will happen if you have, for example, two Tricksters fight each other.

You can cut down on code re-use in move_collection() by having a choose_move function that can tell whether the character is human or computer and doing the right thing from there.

\$\endgroup\$

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.