search

MilkBowl / VaultAPI

API Component of Vault
GNU Lesser General Public License v3.0
273 stars 109 forks source link

Move to BigDecimal #151

Open creatorfromhell opened 1 year ago

creatorfromhell commented 1 year ago

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.

Geolykt commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

creatorfromhell commented 1 year ago

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.