Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dice roll functions #12

Closed
gergo-szabo opened this issue Jul 20, 2023 · 2 comments
Closed

Dice roll functions #12

gergo-szabo opened this issue Jul 20, 2023 · 2 comments

Comments

@gergo-szabo
Copy link
Contributor

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)
@tassaron
Copy link
Owner

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

@gergo-szabo
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants