Dice generator using class in Python

Question

Update Someone is probably going to drop the hammer on me for posting this way but there's not enough room in comments to cover this and they specifically tell you not to answer your own question with a follow-up, so here goes...

I've created the dice class like you guys were talking about.

class dice():
    def __init__(self, sides, number):
        self.sides = sides
        self.number = number

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

The number argument isn't doing anything though.

def att():
    d = dice(20, 2)
    base = d.roll()
    if base == 1:
        print 'Miss!'
    elif base == 20:
        crit = d.roll()
        if crit < 10:
            print 'Hit!'
        else:
            print 'Critical hit!\n'
            effect = super_crit()               
    else:
        print base

The only way I'm getting more than one die is if I do something like this:

def initiative():
    d = dice(10, 1)
    ini = d.roll(), d.roll()
    print ini

I've also tried:

def initiative():
    d = dice(10, 2)
    d.sides = 10
    d.number = 2
    ini = d.roll()
    print ini

That looks redundant to me but I get a Type Error: for having too few arguments without dice(10, 2). No matter which method I use, I get the same result - one die. Am I missing something?

Original post

I'm learning how to use classes in Python 2.7 and as an exercise I'm writing a combat module for a text-based RPG. It uses the old school dice roll method to determine outcomes and effects. A roll determines a hit or miss. If a natural 20 is rolled, another roll determines if it was a critical hit. If critical hit = TRUE, another die is rolled to determined which body part is effected. Each body part is paired with a number 1-12 in a dictionary. There are three possible output messages depending on the affected part. My problem is that the entire list of values is returned instead of a specific part. What am I doing wrong here?

Yes, I know this is super nerdy. Yes, I know the output is lame, but it's all place holder.

import sys, random

#dice generator
class dice():
    def d4(self):
        number = random.randint(1, 4)
        return number

    def d6(self):
        number = random.randint(1, 6)
        return number

    def d10(self):
        number = random.randint(0, 9)
        return number

    def d12(self):
        number = random.randint(1, 12)
        return number

    def d20(self):
        number = random.randint(1, 20)
        return number

    def d100(self):
        number = random.randint(0, 99)
        return number 

#critical hit effect generator        
class super_crit(dice):
    def __init__(self):
        roll = dice()
        loc = roll.d12()
        hit_loc = {1 : 'Head',
                   2 : 'Left Arm',
                   3 : 'Right Arm',
                   4 : 'Left Leg',
                   5 : 'Right Leg',
                   6 : 'Left Hand',
                   7 : 'Right Hand',
                   8 : 'Left Foot',
                   9 : 'Right Foot',
                   10 : 'Chest',
                   11 : 'Stomach',
                   12 : 'Body'}
        part = hit_loc.values()
        for w in part:
            if loc <= 9:
                print w, "has been severed!"
            elif loc == 10:
                print "You sink your blade into his", w, "and pierce the heart!"
            elif loc == 11:
                print "You slash him across the", w, "and eviscerate him!"
            elif loc == 12:
                print "You shred the enemy's", w, "to ribbons!"

class attackRoll(dice):
        pass

#Attack function        
def att():
    roll = attackRoll()
    base = roll.d20()
    if base == 1:
        print 'Miss!'
    elif base == 20:
        crit = roll.d20()
        if crit < 10:
            print 'Hit!'
        else:
            effect = super_crit()               
    else:
        print base

def main():
    att()


if __name__ == '__main__':

main()

Answer 1

You loop over all the values of the dict:

part = hit_loc.values()
for w in part:
    # loop over each part, so print a damage message for all 12 bodyparts

Perhaps you meant to pick the one that is affected instead?

part = hit_loc[loc]  # assign *one* body part to `part`
if loc <= 9:
    print part, "has been severed!"
elif loc == 10:
    print "You sink your blade into his", part, "and pierce the heart!"
elif loc == 11:
    print "You slash him across the", part, "and eviscerate him!"
elif loc == 12:
    print "You shred the enemy's", part, "to ribbons!"

In other words, you do not need a loop here at all.

Note that, whenever you find yourself using a sequential series of numbers as keys to a dictionary, you may as well make it a list instead:

hit_loc = [
    'Head', 'Left Arm', 'Right Arm', 'Left Leg', 
    'Right Leg', 'Left Hand', 'Right Hand', 'Left Foot', 'Right Foot',
    'Chest', 'Stomach', 'Body'
]

except now the indexes run from 0 to 11, so use loc - 1 to find the right body part:

part = hit_loc[loc - 1]

Answer 2

Tangent...

A simpler version of your dice class might be something like:

class Die(object):
    def __init__(self, sides = 6):
        self.sides = sides

    def roll(self):
        return randint(1, self.sides)

You now have a general purpose dice object that doesn't require you to add a new method every time you come up with a new roll mode. E.g.

d = Die(10);
d.roll() # ---> some value between 1 and 10

Adding optional start and increment attributes so that you can have a D20 that produces 5, 10, ..., 95, 100 left as an exercise to the student.

Answer 3

I think it is okay to post another question when you have another question. Since you updated your question to ask about how to do multiple dice, I am putting another answer. (I doubt I will get any points for this, but I don't really care about the points.)

Some games need to know how the dice rolls came out. For example, in the super-hero game Champions, you sometimes need to count how many six-sided dice rolled ones. (I think... been a long time since I played.)

Anyway, instead of summing the dice and returning the sum, your dice class should return a list or tuple of dice roll results. I will show you both. If the tuple one is confusing, just ignore it until you know more about Python; you don't need to learn this now.

Python has a very convenient way to make a list, called a "list comprehension". If you Google for "list comprehension" you will find many pages of explanations. (Which is why I'm not getting any points for this... StackOverflow doesn't really need another list comprehensions explanation!) Basically, it works like this:

[expressionforvariableinsequenceifcondition]

Start looking in the middle, at the "for" part. This means "Set a variable to each member of a sequence. Check the condition, and if it is true, evaluate the expression and append the result to the new list."

A simple example: making a list of the squares of odd numbers from 1 to 10. This will do it:

lst_odd_squares = [x**2 for x in range(10) if (x % 2) == 1]
print(lst_odd_squares) # prints: [1, 9, 25, 49, 81]

Here is how to do this using a for loop:

lst_odd_squares = []  # start with an empty list
for x in range(10):  # just like the for part
    if (x % 2) == 1:  # just like the condition part
        lst_odd_squares.append(x**2)  # append the expression part

The "condition" part is optional, and this particular example could be done this way:

lst_odd_squares = [x**2 for x in range(1, 10, 2)]

Here we start the range at 1, and step by 2, so we just directly produce the numbers we wanted.

The expression usually has the for variable in it, but it doesn't have to:

counters = [0 for x in range(10)]

This would produce a list of 10 integers, initially all set to 0. You could then use them as counters or something.

Anyway, here's the way to make a dice rolling class that returns a list:

import random

class Dice():
    def __init__(self, sides):
        self.sides = sides

    def roll(self, number):
        """return a list of length number, with random values from 1 to self.sides"""
        return [random.randint(1, self.sides) for _ in range(number)]

    def roll_one(self):
        """return a single value from 1 to self.sides"""
        return random.randint(1, self.sides)

d6 = Dice(6)
d10 = Dice(10)

print d6.roll(3)  # roll 3d6
print d10.roll(2)  # roll 2d10

print d10.roll(1)  # roll 1d10 -- returns a list of length 1
print d10.roll_one() # roll a single d10 -- returns a single integer

I changed your code a bit. Instead of making a different class instance for each time you need to roll a different number of dice, I just made the .roll() method function take a number argument. Your way isn't necessarily wrong, but I think this way is a little easier.

It is usually best to use upper-case letters for class names. There is a list of rules that most Python folks like to use, called "PEP 8", and upper-case for class names is one of the rules:

http://www.python.org/dev/peps/pep-0008/#class-names

(By the way, the Learning Python book will give you all its examples in good PEP 8 style. So you will learn the PEP 8 way if you learn from that book. That's one of the ways that book teaches you all about Python.)

I used a single underscore (_) as the loop variable. That is legal, and it is sometimes done as a way to signal your intention to not use that value. We need a loop variable, but we don't actually use it, just like the counters example above.

Python has a "list comprehension" but doesn't have a "tuple comprehension". So here is one way to make a tuple: first make the list, then build a tuple out of the list.

lst = [random.randint(1, self.sides) for _ in range(number)]
return tuple(lst)

You can do it all in one go, with no explicit variable name for the list:

return tuple([random.randint(1, self.sides) for _ in range(number)])

This is kind of ugly. If we are going to build the list, why not just return it? Why feed it into tuple() and then destroy it after using it that one time?

Python offers something called a "generator expression" that is sort of like a list comprehension. The important two differences: 0) you use parentheses, not square braces; and 1) it doesn't build anything, it returns an "iterator" object that can produce a series of values. (When Python pulls out the series of values, we say that Python is "iterating" this object.) If you then pass this "iterator" to tuple(), you can directly build a tuple without first building a list and then destroying the list. The cool trick here: you need to use parentheses to call tuple(), and you can just use those parentheses as the ones you need for the generator expression.

return tuple(random.randint(1, self.sides) for _ in range(number))

Again, if the generator expression stuff seems weird or hard right now, just ignore it. The list comprehension is all you need for quite a lot of stuff.

Answer 4

Instead of the for-loop over hit_loc.values(), just set loc=hit_loc.

There are also many other issues with the code. Here are a few:

• Use function definitions instead of a class for dice
• Indent the call to main()
• The dictionary with number keys can be replaced with a list or tuple

Here's one possible refactoring of the first part of the code:

from functools import partial
from random import randint

d4 = partial(randint, 1, 4)
d6 = partial(randint, 1, 6)
d10 = partial(randint, 0, 10)
d12 = partial(randint, 1, 12)
d20 = partial(randint, 1, 20)
d100 = partial(randint, 0, 99)

def critical_hit_effect():
    locations = ('Head', 'Left Arm', 'Right Arm', 'Left Leg', 'Right Leg',
                 'Left Hand', 'Right Hand', 'Left Foot', 'Right Foot',
                 'Chest', 'Stomach', 'Body')
    roll = d12()
    if roll == 10:
        msg = "You sink your blade into his %s and pierce the heart!"
    elif roll == 11:
        msg = "You slash him across the %s and eviscerate him!"
    elif roll == 12:
        msg = "You shred the enemy's %s to ribbons!"
    else:
        msg = "%s has been severed!"
    print msg % locations[roll-1]