Currency System Rework. Addresses #74.

[creatorfromhell]

Addresses #74 #74

[MrNemo64]

I don't like the idea of removing the CurrencyManager and moving the currency methods to the EconomyProvider, we also lose the hability to parse any string into a curremcy and an amount

[MrIvanPlays]

The whole idea of the current implementation is for ease of currency creation not just by the economy provider, but by plugins using the API.

Forcing a default formatting system onto economy plugins.

Nope. Currently the Currency class accepts a BiFunction<Double, Locale, String> for formatting a currency.

Forcing the conversion standards inside Treasury onto economy plugins.

We'd be better off changing the conversion coefficient to a BiFunction<Double, Currency, Double> for independent conversion rather than nuking everything and redoing it in the way we wanted to avoid.

Forcing the currency manager inside Treasury onto economy plugins

Bukkit is also forcing the plugin manager onto plugins. Bukkit is also forcing you to create plugin.yml so your plugin runs. Bukkit is also forcing you to implement CommandExecutor in order to create a command and it is also forcing you to define it in the plugin.yml. Bukkit is forcing you to implement Listener so you can register event handlers.

Forcing currency UUIDs that are used nowhere inside the API onto economy plugins.

Just because they are not used in the treasury plugin does not mean they won't be used at all. Currency UUIDs are a way to identify a currency, and a way to store more data with less memory in a set, list, etc.

[creatorfromhell]

The whole idea of the current implementation is for ease of currency creation not just by the economy provider, but by plugins using the API.

The new system is still easy to create new currencies outside the provider. You just call .registerCurrency.

Forcing the conversion standards inside Treasury onto economy plugins.

We'd be better off changing the conversion coefficient to a BiFunction<Double, Currency, Double> for independent conversion rather than nuking everything and redoing it in the way we wanted to avoid.

Again it's not an API's place to dictate how something should be handled. While I agree with the fact there should be a method for conversion, it shouldn't be dictated how it's achieved, or the system that should be used.

Forcing the currency manager inside Treasury onto economy plugins

Bukkit is also forcing the plugin manager onto plugins. Bukkit is also forcing you to create plugin.yml so your plugin runs. Bukkit is also forcing you to implement CommandExecutor in order to create a command and it is also forcing you to define it in the plugin.yml. Bukkit is forcing you to implement Listener so you can register event handlers.

That kinda hit the marker a little with comparisons. This would be essentially like registering a permission node to a player without doing so in a permissions plugin. It's an extreme overreach.

Forcing currency UUIDs that are used nowhere inside the API onto economy plugins.

Just because they are not used in the treasury plugin does not mean they won't be used at all. Currency UUIDs are a way to identify a currency, and a way to store more data with less memory in a set, list, etc.

Yes but it should be up to implementations.

[MrIvanPlays]

it shouldn't be dictated how it's achieved, or the system that should be used.

Looks like you don't really understand how github reviewing works. I'm suggesting a way to implement it which for me looks more viable and better with the current implementation. If you have a better method please show it. I don't like the thing that I need to implement 1000s of interfaces in order to make an economy provider with 1 currency work.

This would be essentially like registering a permission node to a player without doing so in a permissions plugin. It's an extreme overreach.

I don't understand your point here.

Yes but it should be up to implementations.

If we're gonna talk "should be up to implementations", then currencies should not have been part of Treasury. Bank permissions and bank accounts should not have been part of Treasury. Heck, even transaction history should not have been part of Treasury.

Same as twitch saying "requiring an oauth key for changing the topic or title of a streamer's stream should be up to the implementation of our API in the language you're gonna develop a bot/oauth app". You know what would be the outcome if they actually do something like this, don't you?

[creatorfromhell]

it shouldn't be dictated how it's achieved, or the system that should be used.

Looks like you don't really understand how github reviewing works. I'm suggesting a way to implement it which for me looks more viable and better with the current implementation. If you have a better method please show it. I don't like the thing that I need to implement 1000s of interfaces in order to make an economy provider with 1 currency work.

That's fairly abrupt for a modest conversion I thought was happening. Apparently not. If we're going onto this route, I'm presuming you have no experience in the economy plugin realm, where I do have experience.

This would be essentially like registering a permission node to a player without doing so in a permissions plugin. It's an extreme overreach.

I don't understand your point here.

It's the same premise, the current manager registered and handles currencies without the economy implementation even knowing. Would the same be done in permissions? I would hope not.

Yes but it should be up to implementations.

If we're gonna talk "should be up to implementations", then currencies should not have been part of Treasury. Bank permissions and bank accounts should not have been part of Treasury. Heck, even transaction history should not have been part of Treasury.

This is very reductio ad absurdum. Currencies and transactions are presumed to be in an economy plugin(same as multiworld). I think you're forgetting that majority of economy plugins are well established at this point and if you're implementing things that ought not be implemented by an API do you know what the outcome is? Vault works good enough.

Same as twitch saying "requiring an oauth key for changing the topic or title of a streamer's stream should be up to the implementation of our API in the language you're gonna develop a bot/oauth app". You know what would be the outcome if they actually do something like this, don't you?

Comparisons to apples and oranges are fun.

[MrIvanPlays]

I'm presuming you have no experience in the economy plugin realm

I've done stuff in the past with economy plugins and I don't wanna go with "implement this and that and that too oh and that too otherwise it won't compile and that other thing and that small thing and that other other thing and yes this also requires implementing that".

The point of Treasury is to offer as much as possible without bloating the API as simple as possible and simple not just for plugins using the API, but for providers too.

the currentcurrency manager registered and handles currencies without the economy implementation even knowing.

Why should it know? In my opinion it shouldn't care what a plugin does with a currency until they actually use it for a transaction.

This is very reductio ad absurdum.

You wanted "should be up to implementations" things and I listed them.

[creatorfromhell]

I'm presuming you have no experience in the economy plugin realm

I've done stuff in the past with economy plugins and I don't wanna go with "implement this and that and that too oh and that too otherwise it won't compile and that other thing and that small thing and that other other thing and yes this also requires implementing that".

The point of Treasury is to offer as much as possible without bloating the API as simple as possible and simple not just for plugins using the API, but for providers too.
This is the point, the whole current currency system is bloatware. It's an entire implementation of a currency manager that shouldn't be inside an API, aka bloat.

the currentcurrency manager registered and handles currencies without the economy implementation even knowing.

I still meant current manager here. Thanks for the grammar fix though.

Why should it know? In my opinion it shouldn't care what a plugin does with a currency until they actually use it for a transaction.

This is very reductio ad absurdum.

You wanted "should be up to implementations" things and I listed them.

This argument is getting nowhere fast, I think the best method is to wait on @Jikoo or @lokka30 as you'd rather just be abrupt and correct the grammar of people responding than have a decent conversation.

[MrIvanPlays]

This is the point, the whole current currency system is bloatware. It's an entire implementation of a currency manager that shouldn't be inside an API, aka bloat.

Why shouldn't it be?

The PluginManager in BungeeCord is a final class, implemented in the API, and in this thinking, it should not be implemented in the API but in BungeeCord's internals. Why complicate stuff?

This argument is getting nowhere fast, I think the best method is to wait on Jikoo or lokka30 as you'd rather just be abrupt and correct the grammar of people responding than have a decent conversation.

Just wanted to let you know you sound very unprofessional and childish by saying this.

[creatorfromhell]

This is the point, the whole current currency system is bloatware. It's an entire implementation of a currency manager that shouldn't be inside an API, aka bloat.

Why shouldn't it be?

The PluginManager in BungeeCord is a final class, implemented in the API, and in this thinking, it should not be implemented in the API but in BungeeCord's internals. Why complicate stuff?

Because it's bloatware.

This argument is getting nowhere fast, I think the best method is to wait on Jikoo or lokka30 as you'd rather just be abrupt and correct the grammar of people responding than have a decent conversation.

Just wanted to let you know you sound very unprofessional and childish by saying this.

Not really, I'm dealing with someone who goes on unprofessional drunk rants in other topics then decides it's a good idea to correct grammar in a quoted response. That behavior has no business being in a civilized conversion. Not to mention this response included. If this is the way you conduct yourself in everyday life I'm sorry. (I refer to comments here #72).

[MrNemo64]

Here I'm on @creatorfromhell's side. An API is ideally a collection of interfaces that an aplication gives to the consumer to implement.
I think we did a good job with the CurrencyManager but having the inplementation in the API defeats the purpouse of the API

Bukkit is also forcing the plugin manager onto plugins. Bukkit is also forcing you to create plugin.yml so your plugin runs. Bukkit is also forcing you to implement CommandExecutor in order to create a command and it is also forcing you to define it in the plugin.yml. Bukkit is forcing you to implement Listener so you can register event handlers.

Bukkit is telling you how to interact with their server. They specify a set of rules (the interfaces) that guarantee that your code will run on every Bukkit server.

We, as the designers of an API that we hope everyone will use, are only entitled to define the set of rules that providers and consumers have to follow and hopelully fulfill everyones needs. We don't tell them how they have to follow them, just what they have to follow.

[lokka30]

Awesome - very satisfied with the state of this PR. Thank you very much

[creatorfromhell]

Awesome - very satisfied with the state of this PR. Thank you very much

Cheers. You're welcome.

[lokka30]

@Jikoo - whenever you are comfortable, please take a look at this PR :)

[Jikoo]

Looks good to me. I like currencies not having UUIDs - as much as they may be unique, the unique ID should be easily user-referenced for configuration/commands. Making users determine UUIDs seems like an unnecessary pain.

[lokka30]

Thanks Jikoo!

@MrIvanPlays what is your opinion on this PR? Has it changed at all, or does it remain the same?

[MrIvanPlays]

It has not changed.

[lokka30]

I think it would be best if you could list each issue you have with this PR so we can pinpoint them and respond to them.

Doing so, we must have a civil discussion - avoid the personal attacks like last time.

[MrNemo64]

We have a method to parse an amount of a given currency but we are missing a method that given a string returns the currency and the amount.

[MrIvanPlays]

@lokka30 i already pointed them out.

[lokka30]

@lokka30 i already pointed them out.

I'll happily read your opinion on this if it's a bullet point list of the issues you have with this PR. I don't want to chase through the conversation, I am just looking for an overall status.

[MrIvanPlays]

The whole idea of the current implementation is for ease of currency creation not just by the economy provider, but by plugins using the API.

Forcing a default formatting system onto economy plugins.

Nope. Currently the Currency class accepts a BiFunction<Double, Locale, String> for formatting a currency.

Forcing the conversion standards inside Treasury onto economy plugins.

We'd be better off changing the conversion coefficient to a BiFunction<Double, Currency, Double> for independent conversion rather than nuking everything and redoing it in the way we wanted to avoid.

Forcing the currency manager inside Treasury onto economy plugins

Bukkit is also forcing the plugin manager onto plugins. Bukkit is also forcing you to create plugin.yml so your plugin runs. Bukkit is also forcing you to implement CommandExecutor in order to create a command and it is also forcing you to define it in the plugin.yml. Bukkit is forcing you to implement Listener so you can register event handlers.

Forcing currency UUIDs that are used nowhere inside the API onto economy plugins.

Just because they are not used in the treasury plugin does not mean they won't be used at all. Currency UUIDs are a way to identify a currency, and a way to store more data with less memory in a set, list, etc.

@lokka30

[lokka30]

Not a quote from a conversation :) I've already read that. What exactly is wrong with this PR?

[creatorfromhell]

We have a method to parse an amount of a given currency but we are missing a method that given a string returns the currency and the amount.

Won't let me respond to the conversation, but I see where you're coming from about the decimal character. The only thing is, if we base it off of real-life situations, decimals are tied to currencies not individuals.

[MrNemo64]

We have a method to parse an amount of a given currency but we are missing a method that given a string returns the currency and the amount.

Won't let me respond to the conversation, but I see where you're coming from about the decimal character. The only thing is, if we base it off of real-life situations, decimals are tied to currencies not individuals.

I'm not saying to base it off of real-life situations but to let the provider decide what character is used for each player. May be the same for everyone may not be. The format method would be the one saying it.

[lokka30]

We have a method to parse an amount of a given currency but we are missing a method that given a string returns the currency and the amount.

Won't let me respond to the conversation, but I see where you're coming from about the decimal character. The only thing is, if we base it off of real-life situations, decimals are tied to currencies not individuals.

A server which houses users of different languages would probably like to make each currency be able to adapt its formatting according to an individual player's language :)

[creatorfromhell]

A server which houses users of different languages would probably like to make each currency be able to adapt its formatting according to an individual player's language :)

Sure, which I agree should be added, but what about default cases? I'm sure they'd like to have an option for a non "." default?

[lokka30]

A server which houses users of different languages would probably like to make each currency be able to adapt its formatting according to an individual player's language :)

Sure, which I agree should be added, but what about default cases? I'm sure they'd like to have an option for a non "." default?

Oh, I don't prefer either system over the other, I would rather have both.