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

Crashes with 0d #278

Open
nesges opened this issue May 30, 2023 · 3 comments
Open

Crashes with 0d #278

nesges opened this issue May 30, 2023 · 3 comments

Comments

@nesges
Copy link

nesges commented May 30, 2023

Description

dice roller crashes on 0d. roller (CLI) throws a stacktrace:

root@gond:~# roller 0d20
file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:4873
  var self = Error.call(this, message);
                   ^

peg$SyntaxError: Expected "%", "*", "**", "+", "-", "/", "^", [.], [0-9], end of input, or whitespace but "d" found.
    at new peg$SyntaxError (file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:4873:20)
    at peg$buildStructuredError (file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:5387:12)
    at peg$parse (file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:7147:11)
    at Function.parse (file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:7188:12)
    at new DiceRoll (file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:7360:40)
    at file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:7911:24
    at Array.map (<anonymous>)
    at DiceRoller.roll (file:///usr/lib/node_modules/@dice-roller/cli/node_modules/@dice-roller/rpg-dice-roller/lib/esm/bundle.js:7910:37)
    at Object.default [as handler] (file:///usr/lib/node_modules/@dice-roller/cli/lib/roll.js:28:10)
    at /usr/lib/node_modules/@dice-roller/cli/node_modules/yargs/build/index.cjs:1:8993 {
  expected: [
    {
      type: 'class',
      parts: [ [ '0', '9' ] ],
      inverted: false,
      ignoreCase: false
    },
    {
      type: 'class',
      parts: [ '.' ],
      inverted: false,
      ignoreCase: false
    },
    { type: 'other', description: 'whitespace' },
    { type: 'literal', text: '**', ignoreCase: false },
    { type: 'literal', text: '*', ignoreCase: false },
    { type: 'literal', text: '^', ignoreCase: false },
    { type: 'literal', text: '%', ignoreCase: false },
    { type: 'literal', text: '/', ignoreCase: false },
    { type: 'literal', text: '+', ignoreCase: false },
    { type: 'literal', text: '-', ignoreCase: false },
    { type: 'end' }
  ],
  found: 'd',
  location: {
    source: undefined,
    start: { offset: 1, line: 1, column: 2 },
    end: { offset: 2, line: 1, column: 3 }
  }
}

Dice Roller version

v5.3.2

Bundle type

UMD

Node version

v16.20.0

Browser name and version

No response

@GreenImp
Copy link
Collaborator

This is actually expected behaviour, because you can't roll 0 dice.
That error is the die parser failing because the syntax is invalid; The number of sides must be at least 1.

What behaviour would you expect in this situation?

@GreenImp GreenImp removed the bug label Jun 10, 2023
@nesges
Copy link
Author

nesges commented Jun 10, 2023

What behaviour would you expect in this situation?

Some kind of human readable message like "you can't roll 0 dice" would be better error handling.

I'm using rpg-dice-roller in a very simple service for a mastodon dice roller bot (https://codeberg.org/nesges/rollbot-mastodon/src/branch/main/server/server.js) and users try funny things

@GreenImp
Copy link
Collaborator

GreenImp commented Jun 12, 2023

It's nice to see someone implementing it on Mastodon! Really cool.

You make a good point about the readability. I've been having a think about it and, at the core it's difficult to make those errors really nice, because they're thrown by the grammar parser library, Peggy. And, before it's been parsed, it's not easy to reliably tell if the notation is invalid.

However, I've just been looking over the Peggy docs, and it looks like the error messages can be enhanced slightly by naming the grammar rules. Most of the rules currently aren't annotated.
I've had a quick play around, but haven't got a result I'm happy with

PRs are always appreciated.

@GreenImp GreenImp removed their assignment Jun 19, 2023
@GreenImp GreenImp changed the title [Bug]: Crashes with 0d Crashes with 0d Jul 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants