Skip to content
This repository has been archived by the owner on Aug 16, 2021. It is now read-only.

[Feature] Multiple Currency Support #4

Open
5 tasks
jasonw4331 opened this issue Jun 8, 2020 · 24 comments
Open
5 tasks

[Feature] Multiple Currency Support #4

jasonw4331 opened this issue Jun 8, 2020 · 24 comments

Comments

@jasonw4331
Copy link

jasonw4331 commented Jun 8, 2020

The current economy plugin provided by Onebone has multi-currency support. I would like to see an implementation added to this plugin also.

  • Multi-currency fields added to current API
  • Add Exchange Rate API
  • Per-currency default values
  • Per-currency maximums
  • Some other things I can't think of right now...
@Thunder33345
Copy link

Thunder33345 commented Jun 8, 2020

Add Exchange Rate API

shouldnt this be a responsibility of another plugin?

wouldnt this be BC breaking?
because this means an addition of an currency selector process

@jasonw4331
Copy link
Author

Selector by the player or by the plugin using the API?

@SOF3
Copy link

SOF3 commented Jun 8, 2020

Exchange rates are not a necessary part of a multi-currency system.

@SOF3
Copy link

SOF3 commented Jun 8, 2020

Currency should be defined by the initiation code of a transaction. For example, in a pay operation, do we really want to allow any player to give any currency to anyone else, or do we only want it for a certain currency? What if the server opts to per-world currency, and the pay command should automatically select based on the world? Alternatively, for a shop command, should all currencies be allowed for trading?

@Thunder33345
Copy link

Selector by the player or by the plugin using the API?

PT1: API

IMO it should ALWAYS be EXPLICITLY done by API, and it will be VERY nice, if it's ALWAYS EXPLICIT(so yes bc breaking)

unlike economyapi which take the route of being BC compatible, by having some "default currency" if plugin didnt specify

IMO it's bad, it encourage a lazy code, with undefined behaviour, most times plugins would not even have a config, and ONLY support default currency (yes very useful, multi currency but most plugins didnt even bother implementing that part)


PT2: player

Players just shouldnt be able to "set which currency to use", that's up to the plugin using the API,
and if plugin want to support inline conversion, multi currency and value support, that should also be up to them

Allowing player to "use this currency for that" would create more unnecessary complications or more complicated logic to control when conversion can be used, and when it cant

I cant think of a good usecase that would justify this extra effort, i think the ability to support multiple currencies is good enough, and the rest should be up to the plugins

@SOF3
Copy link

SOF3 commented Jun 9, 2020

Alternatively, a fuzzy model where multiple currencies are allowed and the best one or feasible ones are selected by the player is also doable. The caller defines a set of rules declaring which currencies are selectable and which currencies are not selectable, and gives each currency a score. Then the player is asked to select one of them, or the most likely one is selected based on machine learning algorithms.

However, this is unlikely to be worth the cost in most cases, and is totally possible to do this selection in the API caller.

@SOF3
Copy link

SOF3 commented Jun 9, 2020

Are we sure we want maximums at all? Is this a hard limit, or is this a soft limit that can be overridden explicitly? If it can be overridden, is it better to always ask the API caller (without default behaviour) whether the soft limit needs to be checked?

@SOF3
Copy link

SOF3 commented Jun 9, 2020

One issue is how API callers can conveniently specify the currency to use. Should it be a string identifier like "dollar"/"elo"/"coin"/etc.? Or should it be an object reference?

Most importantly, how should a trivial plugin supporting economy operations know what currency to use? In particular, if there does not exist a default currency, what should be the default value in the plugin config?

@Thunder33345
Copy link

actually from an adoption standpoint, not having a "default" currency can be problematic for plugin setup

something more practical would be

config: currency: default(or currency's UUID)
api: plugin->getCurrency(UUID|Default)
where default is a reserved keyword, which in the economy's config: default: (UUID)
if there was no default value, it will be a hassle for developers to try to prompt user for a value
making something very travial otherwise, overly complicated

API design wise, it should be something like CurrencyObject->action(), rather then Plugin->action(currencyid) which is what economy api did, and what allow them to BC support all the way back into PO stone age

for identifiers, there should be multiples:
UUID, would be for internal and configuration
Name, would be for display(like list all my money), and targeting via commands, though undefined behavour is bound to happen if there's 2 with the same name
Display format, would be for actually displaying the currency(ex $%d)

@Thunder33345
Copy link

Alternatively, a fuzzy model where multiple currencies are allowed and the best one or feasible ones are selected by the player is also doable.

while i understand this post is made in humorous attempt, even so, this would be complicated, simply because different rates for each currency(if both currencies are the same rates, why would it be necessary to have 2 of these)

Are we sure we want maximums at all? Is this a hard limit, or is this a soft limit that can be overridden explicitly? If it can be overridden, is it better to always ask the API caller (without default behaviour) whether the soft limit needs to be checked?

i agree in that, we had no minimum value, why do we need a maximum value?

@SOF3
Copy link

SOF3 commented Jun 10, 2020

Actually both the amount of money and the currency should not have defaults. The value of one unit of any currency is not well-defined anyway, and plugins simply should not try to make a default.
They should consider forcing the user to specify both the amount and currency.

@SOF3
Copy link

SOF3 commented Jun 10, 2020

Alternatively, a fuzzy model where multiple currencies are allowed and the best one or feasible ones are selected by the player is also doable.

while i understand this post is made in humorous attempt, even so, this would be complicated, simply because different rates for each currency(if both currencies are the same rates, why would it be necessary to have 2 of these)

Are we sure we want maximums at all? Is this a hard limit, or is this a soft limit that can be overridden explicitly? If it can be overridden, is it better to always ask the API caller (without default behaviour) whether the soft limit needs to be checked?

i agree in that, we had no minimum value, why do we need a maximum value?

0 is a de facto minimum. But are we sure we don't want to allow users to have negative balances (overdrafting)?

@SOF3
Copy link

SOF3 commented Jun 10, 2020

for identifiers, there should be multiples:
UUID, would be for internal and configuration
Name, would be for display(like list all my money), and targeting via commands, though undefined behavour is bound to happen if there's 2 with the same name
Display format, would be for actually displaying the currency(ex $%d)

this looks overcomplicated. An identifier and a display format should suffice. The UUID has to be exposed through user interface (commands) anyway, so making an extra display name is pointless.

@SOF3
Copy link

SOF3 commented Jun 10, 2020

API design wise, it should be something like CurrencyObject->action(), rather then Plugin->action(currencyid) which is what economy api did, and what allow them to BC support all the way back into PO stone age

Currency-oriented sounds like a bad idea. What is the advantage of this?

@SOF3
Copy link

SOF3 commented Jun 10, 2020

https://forums.pmmp.io/threads/plugin-installation-wizards.885/ This RFC suggests a library to interactively query user for config values on startup. Perhaps developing a virion to do this would be helpful.

Ideally, ParoxityEcon can provide an API to let user select currencies conveniently.

@Thunder33345
Copy link

Currency-oriented sounds like a bad idea. What is the advantage of this?

fair enough, just thought of it off my head, what would be a better idea then?

@SOF3
Copy link

SOF3 commented Jun 11, 2020

Currency-oriented sounds like a bad idea. What is the advantage of this?

fair enough, just thought of it off my head, what would be a better idea then?

I don't see anything wrong with $eco->addMoney() etc.

Nevertheless I still think atomicity is better. A single API to perform a transaction is better than asking developers to call two functions separately.

@SOF3 SOF3 mentioned this issue Jun 11, 2020
5 tasks
@SOF3
Copy link

SOF3 commented Jun 11, 2020

Another issue: where are currencies defined?
Should they really be defined in the config, or would it be better if they are defined via commands and stored on the database directly?

In #7 I raised the concern on configuration synchronization.
This issue is best mitigated by... not adding config.
Is there really a reason for creating a config file instead of creating a console/web/command/form interface to let user edit configuration?
In particular, for currencies, do they really need to be defined in the config?
Can't we just add currencies by command?
In this case we also prevent the problem of unsound database relations.

Alternatively, we could synchronize the currency options with the database currencies table,
but this will end up with the problem of... well... a new set of questions.

  • If a currency exists in database but not in config, should it get removed?
  • If multiple servers defined currency display format differently, what should be the resolution?

I would not agree that it is a good idea to have a shared configuration defined in multiple config files.

@Aericio Aericio changed the title [Feature Request] Multiple Currency Support [Feature] Multiple Currency Support Jun 12, 2020
@Thunder33345
Copy link

this reminds me, what happen if another plugin wanted to register a custom currency with a custom provider?
since there's now multi currency support, it would make sense that is possible

@SOF3
Copy link

SOF3 commented Jun 12, 2020

@Thunder33345 can you define precisely "custom providers"? What is the point of introducing externally managed currencies into the system?

@Thunder33345
Copy link

say what happens if something i wanted isnt supported?
easiest way would be to extend it, rather then to nag the maintainer about it and argue back and forth why it should (or not) be implemented

@SOF3
Copy link

SOF3 commented Jun 17, 2020

@Thunder33345 what exactly do you want? I don't see the reason why you would want to store currency outside ParoxityEcon. If you need to do so, how does it even have anything to do with ParoxityEcon?

@Thunder33345
Copy link

probably more of "if it's possible why not"
(maybe this is more like abusing econ as vault now)

@SOF3
Copy link

SOF3 commented Jun 19, 2020

probably more of "if it's possible why not"
(maybe this is more like abusing econ as vault now)

If there is no reasonable use of this feature, you are just increasing API complexity and making it harder to ensure backward compatibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants