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

Move to BigDecimal #151

Open
creatorfromhell opened this issue Feb 28, 2023 · 4 comments
Open

Move to BigDecimal #151

creatorfromhell opened this issue Feb 28, 2023 · 4 comments

Comments

@creatorfromhell
Copy link
Contributor

The title is pretty self-explanatory, but I think Vault should be using BigDecimal instead of Double in API methods as the level of control over precision and rounding modes is superior in every way to doubles. You have to keep in mind this is also the defacto representation of monetary value in Java preferred by developers because of the amount of control over precision.

@creatorfromhell creatorfromhell changed the title Move to BigDecimals Move to BigDecimal Feb 28, 2023
@Geolykt
Copy link
Contributor

Geolykt commented Mar 1, 2023

I remember there being talks about this in a "vault replacement" API and I believe the outcome was that the overhead was not worth the precision improvements.

Furthermore, developers and server owners should generally balance their cashflow in a way that hyperinflation or hyperdeflation cannot happen - at least to the degree where floating point precision becomes an issue.

Of course, in the real world that cannot be guaranteed and henceforth developers are forced to account for such inaccuracies - but modeling a model over the real world doesn't work in minecraft I have learned.

@creatorfromhell
Copy link
Contributor Author

creatorfromhell commented Mar 1, 2023

I remember there being talks about this in a "vault replacement" API and I believe the outcome was that the overhead was not worth the precision improvements.

Furthermore, developers and server owners should generally balance their cashflow in a way that hyperinflation or hyperdeflation cannot happen - at least to the degree where floating point precision becomes an issue.

Of course, in the real world that cannot be guaranteed and henceforth developers are forced to account for such inaccuracies - but modeling a model over the real world doesn't work in minecraft I have learned.

I remember the conversation you're referring to and the general outcome/consensus was that it was an important resource for precision and scaling control and which got it added into the replacement.

In fact if you look at said replacement I was the one that PR'd the changeover anyways. The precision is there then if there's an economy plugin that doesn't need it, BigDecimal has wrapper methods for it.

@Geolykt
Copy link
Contributor

Geolykt commented Mar 1, 2023

Took another look at it and you're absolutely right!
That's what I get for using outdated versions of libraries

@creatorfromhell
Copy link
Contributor Author

Took another look at it and you're absolutely right! That's what I get for using outdated versions of libraries

No worries. I'm just on the side of flexibility. If we want to get into a true performance conversation then I think we need to merge into a completeablefuture direction and how most plugins operate in a blocking capacity when it shouldn't be done.

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

2 participants