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

BUG: Manage Currency -> Convert All Currency gives incorrect result due to rounding errors #5017

Open
costonb opened this issue Jan 19, 2025 · 0 comments

Comments

@costonb
Copy link

costonb commented Jan 19, 2025

Description:
Due to the way currency is converted with the base being in gold and anything worth less than that being fractional when you convert currency it can give incorrect results due to rounding errors in the convertCurrency() function in module/applications/currency-manager.mjs

Repro steps:

  1. Create a character
  2. Go to inventory tab of character sheet
  3. Give character 1ep, 1sp, 1cp
  4. Click Manage Currency
  5. Go to Convert tab of Manage Currency modal
  6. Click Convert All Currency button

Result:
Character now has 1ep, 1sp, 0cp

Expected result:
Character should still have 1ep, 1sp, 1cp

Additional debug info:
This happens due to a rounding conversion with Javascript. Your method starts with a basis of 0.61 (0.5 from ep + 0.1 from sp + 0.01 from cp) when the method calculates that there should be 1 ep, it subtracts 0.5 from 0.61, however due to how floating point math works in JS it gets the result wrong and thinks the result is 0.10999999999999999 instead of 0.11, this means when it calculates the final copper it ends up with the calculation being Math.floor(0.00999999999999999 * 100) which is 0, so the final currency is incorrect.

Analysis:
The easiest and probably best solution would be to make cp your base currency instead of gp. This would mean that everything is always an integer so there is no floating point arithmetic. This would just involve updating the conversion parameter for each currency of CONFIG.DND5E.currencies, however I don't know if there is anything else in the codebase that would be affected by that change that would be a potential issue.
Another option is to adjust your calculation so it doesn't have those errors. I know there are JS libraries that can do floating point math without error, however it looks like you do not currently use any libraries in your codebase, so I don't know how open you are to that approach.

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

1 participant