search

dice-roller / rpg-dice-roller

An advanced JS based dice roller that can roll various types of dice and modifiers, along with mathematical equations.
https://dice-roller.github.io/documentation
MIT License
243 stars 58 forks source link

Rounding of negative numbers works different from Math.round #238

Closed xremming closed 2 years ago

xremming commented 2 years ago

Describe the bug Rounding of negative numbers does not work as stated in the documentation.

round(-1.5) !== Math.round(-1.5)

You can also use an array of mathematical formulas and functions. It works with the following Javascript math functions abs, ceil, cos, exp, floor, log, max, min, pow, round, sign, sin, sqrt, tan

Noticed this while writing tests for my project that uses this library.

To Reproduce Steps to reproduce the behaviour:

  1. Roll notation round(-1.5)
  2. It returns -2
  3. Use browsers console to run Math.round(-1.5)
  4. It returns -1

Expected behaviour The values should be equal, or the documentation should specify the differing behaviour.

Environment (please complete the following information): Confirmed both on:

GreenImp commented 2 years ago

This is an interesting one. I hadn't noticed there was a difference!

Internally, we use MathJs for calculations, which has it's own round() method. The documentation there does accurately describe how it works.

However, JS Math.round() does round differently.

There's lots of discussions as to which one is correct (And several other rounding methods), and which one to use. Both how MathJs and JS handles it are very popular.

As suggested, I'll update the documentation to mention this difference. I've added a ticket for this on the documentation repo here: https://github.com/dice-roller/documentation/issues/46

xremming commented 2 years ago

Thanks! I guess MathJS does the "more correct" choice and rounds towards the even number (just like python for example) whereas JS does whatever it wants to.

GreenImp commented 2 years ago

It actually seems like MathJs is rounding half away from zero, as opposed to even numbers.

round(-3.5) === -4
round(-4.5) === -5

Whereas JS (I think) just always rounds upwards.

But yes, I think MathJs does it more how a mathematician would expect it.

Also, I have fixed this on the dev branch for the documentation, but I've been having some connection issues, so can't push up the changes to the site. I'll try again tomorrow.

GreenImp commented 2 years ago

I've got this deployed now: https://dice-roller.github.io/documentation/guide/notation/maths.html#functions