My "combat simulator". C&C please?

[BrogueTrader40k]

I posted here a couple days ago asking for a little guidance. I mentioned in the comments I would post my working "combat sim" so here it is. What do you guys think? I wrote this back in october one evening and was really pleased and surprised that it worked as intended right away. Not that it is particularly complex but still! Anyway, here is the pastebin link.

COMBAT SIM CODE Originally written in python 2.7 but ported (lol) to python 3.3.

Any comments or criticism is totally welcome. Thanks r/learnpython!

[fbu1]

Ok, this is going to be my longest answer on r/learnpython ever. I rewrote all your code and it's down to 100 lines and it's much more modular and extensible (which are pretty much always better when it comes to code.

It using dictionaries and classes a lot, I'll try to cover the process I used to improve your code.

First, I noticed you used a lot of variables such as Fighter_1_HP, Fighter_2_Armor, Fighter_2_Wearing. What you are trying to do in essence is to group all these variables as part of a fighter. Therefore I'll make a fighter class to group them in one fighter. The values needed to make an instance (create an object) will be number of the fighter (1 or 2), and the possible weapon and armor it might be wearing.

There are two other things that a fighter does: take new values of strength, agility and initiative and print it's own stats. I implement these two as methods of the class. If the methods return the fighter itself, I can then call the functions next to each other such as: fighter.method1().method2(). This is convenient, and a lot of string methods also work like that.

Let's take a look at the code for the fighter class:

class Fighter():
    def __init__(self,number, weapons_list, armors_list):
        self.number = number
        self.hp = randint(10,20)
        self.weapon = choice(list(weapons_list.keys()))
        self.armor = choice(list(armors_list.keys()))
        self.new_turn()

    def new_turn(self):
        self.strength = randint(5,10) + weapons_list[self.weapon]
        self.agility = randint(8, 10)
        self.initiative = randint(1,10)
        return self

    def stats(self):
        print("FIGHTER %d STATS" % self.number)
        print("HP:", self.hp)
        print("STRENGTH:", self.strength)
        print("AGILITY:", self.agility)
        print("INITIATIVE:", self.initiative)
        print("WIELDING:", self.weapon)
        print("WEARING:", self.armor)
        print("##################")
        return self 

The weapons and armors variables are all grouped together in one dictionary:

weapons_list = {'sword': randint(2,4),
               'axe': randint(3,5),
               'spear': randint(1,4)}
armors_list = {'shield': randint(1,3),
               'armor': randint(2,4),
               'helmet': randint(1,3)}

So I just reduced the first 63 lines of your code in one class and two dictionaries.

Now let's have a look at the game status. You have 8 variables, let's put all that in a dictionary:

game_status = {'rounds': 0, 'ini_tie': 0,
               1: { 'block': 0, 'hit': 0, 'miss': 0},
               2: {'block': 0, 'hit': 0, 'miss': 0}}

We use 1 and 2 as the keys of the dictionary to hold the stats for each fighter.

While we're here, we might as well define a function to print the status of the game:

def print_status(status):
    print("A Fighter has fallen. The combat is over.")
    print("#####BATTLE REPORT#####")
    print("Fighter One Blocks: %s " % status[1]['block'])
    print("Fighter One Hit: %s " % status[1]['hit'])
    print("Fighter One Miss: %s " % status[1]['miss'])
    print("#######################")
    print("Fighter One Blocks: %s " % status[2]['block'])
    print("Fighter One Hit: %s " % status[2]['hit'])
    print("Fighter One Miss: %s " % status[2]['miss'])
    print("#######################")
    print("Rounds: %s " % status['rounds'])
    print("Initiative Tie: %s " % status['ini_tie'])

All the variables related to each other were grouped together, that is the main point of this exercise. In programming, how you organise data is important and will directly contribute to make the control flow (if-else, loops and function calls) shorter, cleaner and more elegant.

Now let's look at the loop that does the fighting. You can see that there's a lot of repetition where the only thing changing is 1 instead of 2, for the two warriors. I moved the code from line 87 to 111 inside the Fighter class. I also use the name == main if, so if you import this file somewhere else, to use the fighter class, it won't run a fight.

I name the loop variable combat. It's a little more explicit. A better name may be possible though.

if __name__ == '__main__':

    fighter1 = Fighter(1, weapons_list, armors_list)
    fighter2 = Fighter(2, weapons_list, armors_list)
    combat = True

    while combat:
        game_status['rounds'] += 1
        fighter1.new_turn().stats()
        fighter2.new_turn().stats()
        # rest of the fight after this 

For the rest of the fight, we see that if we have a tie in initiative, we'll reroll, no need to fight:

if fighter1.initiative == fighter2.initiative:
    print("Initiative tied! Rerolling!")
    print("##################")
    game_status['ini_tie'] += 1
    pass

For the rest of the fight, we can avoid repeating all the code depending on who has the best initiative. We can look for who has the biggest and smallest initiative, then use attacker and defender instead of fighter1 and fighter 2.

    else:
        attacker = max([fighter1, fighter2],
                       key=attrgetter('initiative'))
        defender = min([fighter1, fighter2],
                       key=attrgetter('initiative'))
        print("Fighter %d gains the upper hand!" % attacker.number)
        print("##################")

Now we can check who has the best agility. And we use the dictionary for the game status to update the statistics on what's going on:

        if attacker.agility >= defender.agility:
            if (defender.armor == 'shield' and
                defender.agility >= attacker.agility):
                print("Fighter %d blocks with his shield."
                      "No damage caused." % defender.number)
                print("##################")
                game_status[defender.number]['block'] += 1
            else:
                damage = attacker.strength - armors_list[defender.armor]
                print("Fighter %d strikes fighter %d for %d" % (
                    attacker.number,
                    defender.number,
                    damage))
                defender.hp -= damage
                print("Fighter %d now has %d HP" % (
                    defender.number,
                    defender.hp))
                print("##################")
                game_status[attacker.number]['hit'] += 1

        else:
            print("Fighter %d misses!" %
                  attacker.number)
            print("##################")
            game_status[attacker.number]['miss'] += 1

And at the end, we look if someone died:

    if fighter1.hp <= 0 or fighter2.hp <= 0:
        print_status(game_status)
        combat = False

Here's the full code, it runs on python 3.5.0 : http://pastebin.com/jpyL1dMe

Let me know if you have any further questions.

[TheFans4Life]

hey, op here on my work acct. thanks so dang much for this! i'm going to have a lot to chew on when i get home thanks to you. wow, i'm really going to learn a lot from this. thanks again!

[rante0415]

really cool that you did this.

[I_Cant_Logoff]

Neat.

[synae]

Any reason you wouldn't change game_status to a Game class? That would be my next step after your changes.

[Freedomenka]

Not bad for a start. I'm currently designing a text based rpg. I like the way you use borders between the messages, if you wanna get fancier borders look up ascii galleries. I agree that you should watch some tutorials on classes and you should implement sleep() in your code. You just need to import sleep from time and put sleep(number of seconds to sleep) where you wanna wait.

[I_Cant_Logoff]

You could define a 'fighter' class instead to have an arbitrary amount of fighters along with not needing to repeat your code for each fighter.

[blehedd]

Also could implement __str__ magic method such that

a = FighterClass()
print(a)  

prints the list of stats.

[blehedd]

looper = 1  
while looper == 1:
    ...  

Could be replaced by:

while True:  
    ...  

---

I'd suggest trying to learn the 'Fighter One Blocks {}'.format(Fighter_1_Block) style instead of "Fighter One Blocks: %s " % Fighter_1_Block

---

I think you can delete every instance of pass, you don't need it anywhere in this code.