Closed gergo-szabo closed 1 year ago
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
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:
The useage in setInitialAbilityScore function: