Skip to content

Conversation

@stephenwade
Copy link
Contributor

@stephenwade stephenwade commented Apr 2, 2020

Fixes #1.

  • s;roll 1d8|4d6 Roll multiple dice at once
  • s;roll 2d20k1 Roll multiple dice, keep highest n
  • s;roll 2d20kl1 Roll multiple dice, keep lowest n
  • s;roll d20 Infer the dice count to be 1 if omitted

Also fixes exploding dice: the current code only explodes once even if the max value is rolled twice. The proposed code will keep exploding repeatedly if possible.

- previous code would only explode once, even if it rolled the highest
  possible number twice in a row
- this code will repeatedly explode if it keeps rolling the highest
  possible number
@stephenwade
Copy link
Contributor Author

Screenshot of Steve responding to 'roll d20' and 'roll d20|d6'

@jwford jwford added command Requires adding/modifying a command semcommit: feat new feature for users semver: minor Constitutes a minor version update labels Apr 2, 2020
@stephenwade stephenwade changed the title [WIP] s;roll improvements s;roll improvements Apr 3, 2020
@stephenwade
Copy link
Contributor Author

Screenshot of Steve responding to 'roll 2d20k1' and 'roll 5d6kl3'

@jwford jwford requested review from BoedJ and jwford April 3, 2020 00:37
Copy link
Member

@jwford jwford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add the suggested examples to cover the keeping highest/keeping lowest stuff? Looks great otherwise

Co-Authored-By: Jonathan Ford <jonathanwford156@gmail.com>
@stephenwade
Copy link
Contributor Author

whoops! should be good now

Copy link
Member

@jwford jwford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

@jwford jwford self-requested a review April 3, 2020 13:19
Co-Authored-By: Jonathan Ford <jonathanwford156@gmail.com>
Copy link
Contributor

@BoedJ BoedJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest moving the explanation for the new complex stuff to extendedHelp, and making helpUsage easier for the average person.
(I know very little about dice, so do feel free to correct anything in this suggestion! Just need something along these lines for the help command. Everything else looks good.)

Co-Authored-By: BoedJ <boedj7@gmail.com>
@stephenwade
Copy link
Contributor Author

Not sure why it still says blocked on changes requested when I already did them. But it should be good to go now!

@BoedJ
Copy link
Contributor

BoedJ commented Apr 3, 2020

Thank you!

@jwford jwford merged commit 2ecbde8 into stevebot-project:master Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command Requires adding/modifying a command semcommit: feat new feature for users semver: minor Constitutes a minor version update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dice Roller Features

3 participants