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

Percentile Advantage/Disadvantage not working as expected #216

Open
rnadams2 opened this issue Mar 9, 2023 · 2 comments
Open

Percentile Advantage/Disadvantage not working as expected #216

rnadams2 opened this issue Mar 9, 2023 · 2 comments
Assignees

Comments

@rnadams2
Copy link

rnadams2 commented Mar 9, 2023

Describe the bug
The "+d%" and "-d%" commands are interpreting the 10s die incorrectly. It seems to be subtracting 1 from the resulting dice (see screenshot below). It also seems to be treating a result of 10 as a 10 instead of a 0, though this may be the same as the first problem (turning a 10 into a 9).

To Reproduce
Simply type the command.

Expected behavior
The 10s dice should be interpreted as rolled and with a result of 10 being equal to a leading 0.

Screenshots

bandicam 2023-03-09 12-08-37-555

In the first instance, the result should be 07, not 37. Note however that it appears to be reading the 4 from the two 10s dice as a 3.

In the second instance, the result should be 81, not 71.

@Teguki
Copy link

Teguki commented Mar 13, 2023

This is happening because Dice Maiden doesn't have a 0-indexed d10 (a d100) for d% rolls.

It rolled a 1-indexed d10, and rolled the "10th face", which is 90 on a d100, and the "4th face", which is 30 on a d100.

Using an edge case here would overcomplicate the advantage and disadvantage part of the roll, making the code harder to maintain. Without 0-indexed dice, subtracting 10 from the final result is the more elegant solution. Adding 0-indexed dice to Dice Maiden would negate the need for any adjustments, so if the "Roll: [...]" part being harder to read bothers you, I suggest you support the addition of 0-indexed dice by adding this as another use of such dice here: #214

@B-Rex
Copy link
Contributor

B-Rex commented Jun 25, 2024

This is functionality is working as expected. As Teguki pointed out the bot is accounting for a regular set of percentile dice having a zero face. That why -1 is applied to the tens die to get a result (0-9)*10 + (1-10) gives us a range of 01-100 which is the expected range for a percentile set. Without the manipulation on the tens die the range would be 11-110.

I think however that the output could be improved by using modulo on the tens die rather than a -1. That should give the functionality of a 0 from the 10 result. @Humblemonk if you assign this to me I'll have a crack at that as the current output will probably mean that the same bug gets logged again in the future.

@Humblemonk Humblemonk assigned B-Rex and unassigned Humblemonk Jun 25, 2024
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

4 participants