Suggestions for a Dungeons and Dragons-like game

Question

This game is similar to a Dungeons and Dragons game. How could I improve the efficiency of the program? For example, where can I add functions, records etc? This is for a school Controlled Assessment so any help would really be appreciated.

import random
play1 = True 

str1 = int(input("Enter a value for strength"))
print("you have chosen",str1,)

skl1=int(input("Please enter a value of skill, and then press enter"))
print("you have chosen: ",skl1,)      

str2 =int(input("Enter a value for strength"))
print("you have chosen: ",str2,)

skl2=int(input("Please enter a value of skill, and then press enter"))
print("you have chosen: ",skl2,)

strdif=str1-str2
    if strdif<0:
    strdif=str2-str1
    strdif=strdif//5

skldif=skl1-skl2
    if skldif<0:
    skldif=skl2-skl1
    skldif=skldif//5

while play1 == True:

   num1=random.randint(1,6)
   num2=random.randint(1,6)

   if num1>num2:
        str1+=strdif
        skl1+=skldif
        str2-=strdif
        skl2-=skldif

    if num2>num1:
        str2+=strdif
        skl2+=skldif
        str1-=strdif
        skl1-=skldif

    if skl1<0:
        skl1=0
    if skl2<0:
        skl2=0

    if str1<=0:
        play1 = False
        print("Character 2 Won the game")
   elif str2<=0:
        play1 = False
        print("Character 1 Won the game")

   if str1 > str2:
       print("Character 1 won the encounter, now has",str1,"strength and",skl1,"skill")
       print("Character 2 lost the encounter, now has",str2,"strength and",skl2,"skill")
   else:
       print("Character 2 won the encounter, now has",str2,"strength and",skl2,"skill")
       print("Character 1 lost the encounter, now has",str1,"strength and",skl1,"skill")

Answer 1

There are two huge improvements you can make to increase readability of your code:

1. Choose better names for some variables – skill1 is better than skl1, and strength2 is better than str2 (str usually means “string”, which adds even more confusion). play1 should be play or play_on.

2. Put whitespace around operators rather than trying to write your code as compact as possible. For example, change

   strdif=str1-str2
   if strdif<0:
       strdif=str2-str1
   strdif=strdif//5
   

   to

   strength_difference = strength1 - strength2
   if strength_difference < 0:
       strength_difference = strength2 - strength1
   strength_difference = strength_difference // 5
   

Next, there are a number of small issues that could be improved:

• while play1 == True could be abbreviated to while play1. However, setting a variable to False in order to exit a loop isn't very elegant: You could break out of the loop instead, or put the loop into a function from which you can return.

• When calculating the differences of strength and skill, you could use the abs function instead: strength_difference = abs(strength1 - strength2) // 5.

• You roll two random numbers from 1–6, then compare whether they are greater or smaller than each other. Instead, we could directly figure out the probabilities:

  • 2/12: draw
  • 5/12: player 1 deals a blow
  • 5/12: player 2 deals a blow

  which means we only have to roll one value from 1–12

• Instead of naming our variables foo1 or foo2, let's create dictionaries that hold variables for each player:

  player1 = dict()
  player1["name"] = "Character 1"
  player1["strength"] = ...
  player1["skill"] = ...
  

  This looks more verbose, but allows us to remove a lot of duplication in a minute.

• We can stuff the character initialization into a function that returns a player dictionary:

  def generate_character():
      character = dict()
      character["name"] = raw_input("name: ")
      character["strength"] = int(raw_input("strength: "))
      character["skill"] = int(raw_input("skill: "))
  
      print("Your character is %s (strength: %d, skill: %d)" % (character["name"], character["strength"], character["skill"]))
  
      return character
  

  Note that I use raw_input instead of input, as this avoids executing the input as if it were code.

  We can then initialize the players like this:

  player1 = generate_character()
  player2 = generate_character()
  

  Because all info about a player is conveniently bundled, we can do neat stuff like this:

  (loser, winner) = sorted((player1, player2), key = lambda player: player["strength"])
  

  which sorts the players by their strength. This simplifies printing out the stats at the end of a round:

  for player, status in [(winner, "won"), (loser, "lost")]:
      print("%s %s the encounter, now has %d strength and %d skill" % 
            (player["name"], status, player["strength"], player["skill"]))
  

Using dictionaries for this is easy, but awkward. Instead we could write a class that represents characters, and create one instance of this class for each player:

class Character:
    def __init__(self, name, strength, skill):
        self.name = name
        self.strength = strength
        self.skill = skill

Our generate_character changes to this:

def generate_character():
    name = raw_input("name: ")
    strength = int(raw_input("strength: "))
    skill = int(raw_input("skill: "))

    character = Character(name, strength, skill)

    print("Your character is %s (strength: %d, skill: %d)" % (character.name, character.strength, character.skill))

    return character

If we combine all these points (and a few other minor quibbles), we might end up with something like the following code:

import random

class Character:
    def __init__(self, name, strength, skill):
        self.name = name
        self.strength = strength
        self.skill = skill

    def hit(self, other):
        strength_difference = abs(self.strength - other.strength)
        self.strength += strength_difference
        other.strength = max(0, other.strength - strength_difference)

        skill_difference = abs(self.skill - other.skill)
        self.skill += skill_difference
        other.skill = max(0, other.skill - skill_difference)

def generate_character():
    name = raw_input("name: ")
    strength = int(input("strength: "))
    skill = int(input("skill: "))

    character = Character(name, strength, skill)

    print("Your character is %s (strength: %d, skill: %d)" % (character.name, character.strength, character.skill))

    return character

def run(player1, player2):
    while True:
        print("Encounter!")

        roll = random.randint(1, 12)
        if roll <= 5:
            player1.hit(player2)
        elif roll <= 10:
            player2.hit(player1)
        else:
            pass # draw

        if player1.strength <= 0:
            print("%s won the game" % player1.name)
            return
        if player2.strength <= 0:
            print("%s won the game" % player2.name)
            return

        (loser, winner) = sorted((player1, player2), key = lambda player: player.strength)
        for player, status in [(winner, "won"), (loser, "lost")]:
            print("%s %s the encounter, now has %d strength and %d skill" % 
                  (player.name, status, player.strength, player.skill))

if __name__ == "__main__":
    print("Initialize the first character:")
    player1 = generate_character()
    print("Initialize the second character:")
    player2 = generate_character()
    run(player1, player2)

Answer 2

I don't see any performance bottlenecks in the traditional sense. Namely I don't see any tight loops that have to do a lot of work where you are likely to be waiting for the computer to finish. Instead what I see are a lot of small opportunities for you to make the code a little easier to read. Here I'll go over a few ways you might approach doing so:

Finding alternate ways to perform arithmetic

After getting strength and skill values from the person running the program, you compute the difference between the values entered. If it's negative, you invert it, and then divide by five. This sounds a lot like taking the absolute value of the difference, then dividing by five. The advantage of using an absolute value function to do this is that you hide the if/else code so you don't have to read through it later. A very similar approach is available for ensuring that the skills don't go below zero, using a maximum or minimum function.

Finding ways to avoid repetition

After you have the strengths, skills, differences, and "rolls" (num1 and num2), you have two branches of code which are largely the same. If num1 is bigger than num2 then the first character wins the encounter. If num2 is larger, the second character wins.Since both of the if statements execute the same operation, but on different variables, it would be nice if you could give the variables a new name, say winner and loser, then add points to the winner and subtract them from the loser. However, in order to do that, you would have to change the storage of your strength and skill.

Small things

It might be helpful to include which character's skill the program is requesting you enter at the beginning. Without that, it might look weird that the program asks you two questions twice.

It's typically less idiomatic to compare a value to True as the condition of a while loop. Instead prefer to use the value itself as the condition: while play1: .... Or, better still, it may make sense to incorporate your later conditions into the while loop, for example making its condition whether both of the character's strength are still above zero. If they are not, the while loop will end, and you can print the winner.

Answer 3

You have a problem in that your code fails when the strength of your characters is equal. The strength difference is zero and this is applied to each character at each encounter. If its zero then neither character is affected.

Also, skill seems to have no effect on anything. I would expect it to modify the damage and/or the 'dice roll'.

With that in mind, I would create a class for your character

class Player(object):
    def __init__(self, name, strength, skill):
        self.name = name
        self.strength = strength
        self.skill = skill

    def alive(self):
        return self.strength > 0

    def roll(self):
        return random.randint(1,6)

    def __str__(self):
        return "%s(strength=%i, skill=%i)" % (self.name, self.strength, self.skill)

It's not strictly necessary to have the dice roll in the character class but that's how I've coded it. The __str__ method provides a nice way to present your character. e.g.

p = Player('Graeme', 100, 1000)
print(p)

Graeme(strength=100, skill=1000)

Now you need to run the fight. I've created a function that takes two player instances. I've kept some of your logic here (though using abs() is better). The fight function calls the encounter function in a while loop which checks they are both alive.

def fight(p1, p2):
    strdiff = abs(p1.strength - p2.strength) // 5
    skldiff = abs(p1.skill - p2.skill) // 5
    while p1.alive() and p2.alive():
        encounter(p1, p2, strdiff, skldiff)
    if p1.alive():
        print("%s won the game" % p1)
    else:
        print("%s won the game" % p2)

The encounter function takes your difference variables as input.

def encounter(p1, p2, strdiff, skldiff):
    num1 = p1.roll()
    num2 = p2.roll()

    if num1 == num2:
        return        
    if num1 > num2:
        winner = p1
        loser = p2
    else:
        winner = p2
        loser = p1

    winner.strength += strdiff
    winner.skill += skldiff
    loser.strength -= strdiff
    loser.skill -= skldiff
    loser.strength = max(0, loser.strength)
    loser.skill = max(0, loser.skill)

    print("%s won the encounter" % winner)
    print("%s lost the encounter" % loser)

Now the whole thing can be run using a simple function to get input.

def get_value(attribute):
    """get user input as integer"""
    return int(raw_input("Enter a value for %s: " % attribute))    

p1 = Player("Character 1", get_value('strength'), get_value('skill'))
print(p1)
p2 = Player("Character 2", get_value('strength'), get_value('skill'))
print(p2)

fight(p1, p2)

Answer 4

The major one, right off the back that I see (and knowing how most teachers are) is the lack of comments in your code, I'm not sure if your teacher requires it, but it certainly couldn't hurt.

Use # for one line comments, or

""" to span multiple lines """

For instance:

#This is a one-line comment, it ends when the line ends, and cannot be used for any code

#import necessary modules and set variables
import random
play1 = True 

#Get user input
str1 = int(input("Enter a value for strength"))
print("you have chosen",str1,)
""" This makes a comment.
It spans multiple lines!
"""

Also, if the user enters a number that is a decimal your program will fail because of this:

str1 = int(input("Enter a value for strength" ))

"""int returns an integer - which in python cannot be a decimal.
If the user enters a decimal it will return the error:
ValueError: invalid literal for int() with base 10: '87.94'
"""

If you want to keep this as an integer, you will first need to 'float' the input, then you will need to convert it to an integer. The following will work:

str1 = int(float(input("Enter a value for strength" )))

When you 'float' a number, you are basically saying that the value is the exact value. So if you float "1/3" you will get .333 repeating. 1.2 is not an integer, and I have no idea how to explain why that is. Bottom line: decimals are not integers.

You could also merely 'float' the input, assuming your program has no need for the number to be an integer (at this stage it doesn't).

str1 = float(input("Enter a value for strength" ))

That is the extent of my pea-sized knowledge. Most importantly though is the comments, always comment, comment, and comment some more; it shows the teacher you 'understand' what's going on, and I can't even tell you how many times it's saved me when debugging