ArcanePlugins / Treasury

🏦 A powerful multi-platform library for next-level plugin integrations.
https://hangar.papermc.io/ArcanePlugins/Treasury
Other
56 stars 13 forks source link

Clarify negative balance support #197

Closed lokka30 closed 2 years ago

lokka30 commented 2 years ago

User Story

Discord user Hai_tun#8468 in the ArcanePlugins Discord Server suggested that each currency in the Economy API should be able to independently allow/deny negative balances.

Is your feature request related to a problem?

No response

Describe the solution you'd like.

Adding negative balance support to the Currency interface.

Potential methods:

name params returns
allowsNegativeBalances none boolean


If allowsNegativeBalances() returns false then it can be expected that the provider will throw an economy exception with NEGATIVE_BALANCES_NOT_SUPPORTED (iirc?) as the failure reason. Otherwise, a return of true implies that the balance can be any negative value.

Describe alternatives you've considered.

Instead of having 'supports negative balances', the economy API could natively support overdrafts, akin to what EssentialsX offers. Each currency has a getMaximumOverdraft() -> BigDecimal method.


Another alternative is to not support negative balances at all. To me, they make no sense, it would be better to have a separate bank plugin where users can take out loans which have interest and so on; no negative values are involved.

Storing negative values in a database table might be detrimental to its schema as the max possible balance value is halved by converting from unsigned -> signed values.

Additional information

No response

Jikoo commented 2 years ago

Of the two, overdrafts make more sense to me because there usually would be some hard lower limit in place. That said, variance like that is why I generally tend to just blindly attempt a withdrawal without bothering to query balance - if it works they had access to enough currency, if it doesn't they didn't.

I almost prefer just eliminating negative balances. I think the only reason not to is that some economy plugins do support them for rudimentary loans, but a more realistic system would not have the loan be lumped into the amount of currency you currently have. 2 currencies (or a separate loan plugin that gives you money at varying interest rates) makes a lot more sense to me.

Re: DB schema: as an API, we don't really care how data storage functions so long as we're not making it prohibitively difficult. If the implementer wants to squeeze that extra bit in, they should likely use a different data type or add an additional separate bit for sign.

A248 commented 2 years ago

I believe negative balances are a justifiable feature of some economy providers. In my view -- well, as someone who likes mathematics -- I see nothing wrong with that the balance is negative. One could conceive of economic systems where players purchase first, then repay debt later.

I much prefer to give this discretion to the economy plugin. Often, other plugins, for example a baltop plugin, display player balances by querying the economy API. It seems sensible for the economy plugin to return a negative balance, which is then displayed in client plugins. In these cases, the negative balance is the player's balance, according to the economy provider.

lokka30 commented 2 years ago

I believe negative balances are a justifiable feature of some economy providers. In my view -- well, as someone who likes mathematics -- I see nothing wrong with that the balance is negative. One could conceive of economic systems where players purchase first, then repay debt later.

I much prefer to give this discretion to the economy plugin. Often, other plugins, for example a baltop plugin, display player balances by querying the economy API. It seems sensible for the economy plugin to return a negative balance, which is then displayed in client plugins. In these cases, the negative balance is the player's balance, according to the economy provider.

Do you believe it would be better to use negative balances instead of overdrafts? It would definitely simplify things for the API.

MrNemo64 commented 2 years ago

I like the idea of having negative balances support, and I like the idea of overdrafts. Maybe we could have 2 methods, the withdraw that doesn't allow you to go below 0 and the overdraft one that allows you to go below 0 (if the provider allows you to do so) But this could be confusing

lokka30 commented 2 years ago

I like the idea of having negative balances support, and I like the idea of overdrafts. Maybe we could have 2 methods, the withdraw that doesn't allow you to go below 0 and the overdraft one that allows you to go below 0 (if the provider allows you to do so) But this could be confusing

a boolean 'preventOverdrafts'?

Jikoo commented 2 years ago

I believe negative balances are a justifiable feature of some economy providers. In my view -- well, as someone who likes mathematics -- I see nothing wrong with that the balance is negative. One could conceive of economic systems where players purchase first, then repay debt later. I much prefer to give this discretion to the economy plugin. Often, other plugins, for example a baltop plugin, display player balances by querying the economy API. It seems sensible for the economy plugin to return a negative balance, which is then displayed in client plugins. In these cases, the negative balance is the player's balance, according to the economy provider.

Or a gamemode where you start in debt and have to work it off. I'm convinced, yeah. A floating minimum balance line is definitely better than not allowing negative balances. The pro-positive-only argument would be that if the actual minimum is -X, that is effectively the same as starting the user at +X with a minimum of 0 (which is currently supported), only differing for display purposes. All that would do is force display plugins to support arbitrary math, which seems like a huge pain when the economy could just handle that.

Do you believe it would be better to use negative balances instead of overdrafts? It would definitely simplify things for the API.

Imo overdrafts are just negative balances with a declared minimum. Mostly useful for things like "do as many of X as possible" where the transaction does not have a fixed quantity because you need to know the total maximum currency you can withdraw before the transaction fails. For example, a command that charges you per block of gold created might want to create as many gold blocks as possible if an amount is not specified. Doing a single transaction per block would be prohibitively slow (and annoying for transaction tracking!), but creating a stack would fail when the user has enough to create only 63. Getting the current and minimum balance and doing some math to determine the number creatable would be correct, but as we don't have a concept of minimum balance it's only possible to do this accurately if negative amounts are not supported.

I like the idea of having negative balances support, and I like the idea of overdrafts. Maybe we could have 2 methods, the withdraw that doesn't allow you to go below 0 and the overdraft one that allows you to go below 0 (if the provider allows you to do so) But this could be confusing

I feel like that would create a lot of mess in usages, would prefer to let the economy handle how overdrafts are managed. Theoretically (assuming the usage is not on the main thread) the economy could prompt the user for input before allowing an overdraft or something of that nature.

I think we might actually consider just allowing a minimum balance to be defined per-currency (and maybe per-account? Allows for VIPs accruing more debt or something, economy plugin can decide). It solves the problem of "are negative balances supported" because if they're not, that minimum is 0. If they are, it's whatever negative number they allow. Logically it doesn't really matter what the balance is at any given moment for anything but display and sorting purposes, only what the user does and does not have access to for funds.

A248 commented 2 years ago

I think the API should not declare a per-currency minimum balance. The economy plugin may handle whether to reject any transaction which violates minimum balance requirements. This allows for maximum flexibility and dynamic behavior. We want client plugins to handle negative balances transparently, without assuming any other information.

Perhaps the API might define a per-account minimum balance. I'm worried this might limit flexibility in some other way I am unaware of; however, I cannot think of any concrete examples.

lokka30 commented 2 years ago

I think the API should not declare a per-currency minimum balance. The economy plugin may handle whether to reject any transaction which violates minimum balance requirements. This allows for maximum flexibility and dynamic behavior. We want client plugins to handle negative balances transparently, without assuming any other information.

Great point.

Perhaps the API might define a per-account minimum balance. I'm worried this might limit flexibility in some other way I am unaware of; however, I cannot think of any concrete examples.

By your previous comment on per-currency minimum balances being undesirable for dynamic behavior, I believe that would apply for per-account minimum balances too.

lokka30 commented 2 years ago

Stating my current opinion: negative balances are allowed, no limitations, plugins just need to make sure that a withdrawal is successful before they try to do things as the economy provider will say whether it was accepted or not.

MrIvanPlays commented 2 years ago

imo overdrafts are bloat. i don't really care whether we have negative balances or not as long as everything's landed onto the plugin implementing the api, with the exception of having some way to check whether they're supported or not.

lokka30 commented 2 years ago

imo overdrafts are bloat. i don't really care whether we have negative balances or not as long as everything's landed onto the plugin implementing the api, with the exception of having some way to check whether they're supported or not.

so that's exactly what we've currently got: API doesn't care about negative balances, providers handle it by using this economy failure reason. πŸ‘πŸ»

MrIvanPlays commented 2 years ago

imo overdrafts are bloat. i don't really care whether we have negative balances or not as long as everything's landed onto the plugin implementing the api, with the exception of having some way to check whether they're supported or not.

so that's exactly what we've currently got: API doesn't care about negative balances, providers handle it by using this economy failure reason. πŸ‘πŸ»

so-be-it then. leave it as-is