tassaron/dnd-character

Dice roll functions

gergo-szabo opened this issue · 2 comments

Only the setInitialAbilityScore function has dice roll simulation in the Character object.
https://github.com/tassaron/dnd-character/blob/main/dnd_character/character.py#L636

Might be useful for later features (eg.: #11 ) to have nice functions for rolls. I propose two new function:

    def __sum_dice_roll(self, d20: int = 0, d12: int = 0, d10: int = 0, d8: int = 0, d6: int = 0, d4: int = 0,
                        drop_lowest: bool = False):
        """ Expected use: attack calculation, initial ability score """
        rolls = [random.randint(1, 20) for _ in range(d20)]
        rolls += [random.randint(1, 12) for _ in range(d12)]
        rolls += [random.randint(1, 10) for _ in range(d10)]
        rolls += [random.randint(1, 8) for _ in range(d8)]
        rolls += [random.randint(1, 6) for _ in range(d6)]
        rolls += [random.randint(1, 4) for _ in range(d4)]

        if drop_lowest:
            rolls = sorted(rolls)[1:]

        return sum(rolls)
    def __one_value_dice_roll(self, dice: int = 20, advantage: bool = False, disadvantage: bool = False):
        """
        Expected use:
        - D20 (ability checks, saving throws, attack rolls)
        - D100 (effects like item properties, madness, etc.)
        """
        if advantage == disadvantage:
            result = random.randint(1, dice)
        else:
            rolls = [random.randint(1, dice) for _ in range(2)]
            result = max(rolls) if advantage else min(rolls)
        return result

The useage in setInitialAbilityScore function:

   @staticmethod
    def setInitialAbilityScore(stat: Optional[int]) -> int:
        """
        Set ability score to an int. If the argument is None, then this method
        instead rolls for the initial starting ability score.
        """
        if stat is None:
            return self.__sum_dice_roll(d6=4, drop_lowest=True)  # roll 4d6, drop lowest
        else:
            return int(stat)

Yeah, there used to be a file called roll.py but I recently deleted it because I didn't like the code. I like yours a lot more; it's shorter and more flexible.

My only feedback is that, since the Character object is not needed in any way, these should probably be standalone functions in a new file, maybe one called dice.py

I also think one_value_dice_roll is a confusing name. I would suggest the names sum_dice_rolls (add an s) and roll_die_with_advantage ... or maybe exclude the word "dice". I think it would be easy to guess the purpose of dnd_character.dice.roll_with_advantage

dice.py is a good idea.

I agree on the confusing names. What if we move the d100 functionality to the sum_rolls and then roll_with_advantage_disadvantage has a clear meaning and usecase.

dnd_character.dice.sum_rolls
dnd_character.dice.roll_with_advantage_disadvantage