Added Reserve Support, and tested.

[creatorfromhell]

Reserve is a new alternative to Vault, with modern functionality. Implemented the Economy API in this PR.

Main Advantages:

• CompletableFuture methods for those plugins that rely heavily on async.
• Support for multi-currency economies(not that this one, in particular, affects EssentialsX)
• Optional ExtendedAPI for even more functionality.
  • Ability to add new currencies or denominations.
  • Transaction API for third-party plugins to hook into economies that utilize the transaction API for transaction logging, and voiding.

I mainly based this implementation off a quick glance of the Essentials Economy API, if there are any methods that seem like they are not implemented in the most efficient way just let me know.

[creatorfromhell]

Screenshots of the implementation working:

• as a provider: https://prnt.sc/o2e81u
• as a non-provider: https://prnt.sc/o2e917

[caojohnny]

Not sure why you PR'd this to Essentials, the implementation for your API goes inside of your plugin, not inside a different plugin

[creatorfromhell]

The implementation goes inside each individual plugin, as Vault does with new plugins. The idea of putting each implementation inside my plugin would be like sponge putting each economy plugin inside their releases, which is far from ideal.

Essentials was added into Vault before they forced each economy provider to make their own file inside their plugins, but generally i want to keep Reserve as lightweight as possible.

[creatorfromhell]

Any updates on this?

[mdcfe]

I don't currently have time to review and merge PRs, but when I get a chance I'll look at this.

[Ichbinjoe]

Mostly nits, requested change in pom.xml I feel is mandatory since putting the repository first opens up anyone building EssX to a dependency poison attack.

[Ichbinjoe]

Ah darn it, meant to press request changes.

[Ichbinjoe]

Please see above review regarding problems.

[mdcfe]

Most of this is code style issues, but there's also a lot of duplication.

[mdcfe]

I like the look of the API overall - while EssentialsX doesn't take advantage of it, I appreciate the first-class support for async calls and multiple currencies, as well as the use of UUIDs not OfflinePlayers in method calls.

One concern I have: how are plugins supposed to create accounts for use that don't belong to players? For example, Towny has town balances, factions plugins have a balance for each faction, Citizens NPCs may have balances, and there are other countless examples of "banks" or shared accounts that aren't directly tied to a user. Do we just randomly generate a UUID and hope it doesn't clash? Do we create an account with a String and prepend some sort of namespace? Or are UUIDs and String identifiers always assumed to be associated with a player?

---

On another note, I'm concerned that simply returning false when a method fails isn't clear enough. There are a multitude of different conditions you could consider a "failure" which, as a plugin calling the economy API, you might want to handle differently.

Firstly, a transaction might fail for a simple reason, such as it would place the account balance above the maximum balance or below the minimum (eg addHoldings, removeHoldings). The plugin calling the provider may want to inform users that the transaction failed due to an account constraint.

However, there might be less obvious issues with a transaction, like a plugin trying to add to a account that doesn't exist yet. There's currently no way for the calling plugin to know exactly what went wrong, only that the transaction failed. If this was exposed, the calling plugin could respond by setting up accounts and trying again or by informing users/administrators "hey, you need to do X".

Another thing to consider is the possibility that the provider plugin could encounter an internal error while processng the transaction. Economy providers work in many different ways; if a database connection fails, the provider plugin may not be able to do anything, or perhaps the plugin is just encountering some other internal issue that prevents the call succeeding. The calling plugin may want to catch this and show a stack trace to ease the process of identifying and rectifying the issue.

Finally, the provider plugin may choose not to implement all of the features of a Reserve economy service (such as in this case with . The calling plugin may want to disable some of its features if the economy provider doesn't support all Reserve features.

These could be represented by either returning a response object that could indicate success/failure (like Vault's EconomyResponse class and ResponseType enum), or by throwing specific relevant exceptions (eg. one for balance out of bounds, one for nonexistent account, one for internal provider errors etc).

Furthermore, for optional features/capabilities/whatever you choose to call them, you could add an enum of feature flags to the API, and the provider can expose a supports(Feature) or getSupportedFeatures() method to allow for better feature detection. Attempting to call methods for unsupported features would naturally still result in an error, but now calling plugins don't need to attempt an API call to determine whether provider supports a feature or not.

I'm aware this is a big change to consider for the API, but if Reserve is intended as a replacement for Vault in the long term, it needs to be able to match and improve upon the capabilities of Vault or there simply won't be any reason for developers to support Reserve.

---

edit: clarity + Vault source code link

[creatorfromhell]

"One concern I have: how are plugins supposed to create accounts for use that don't belong to players? For example, Towny has town balances, factions plugins have a balance for each faction, Citizens NPCs may have balances, and there are other countless examples of "banks" or shared accounts that aren't directly tied to a user. Do we just randomly generate a UUID and hope it doesn't clash? Do we create an account with a String and prepend some sort of namespace? Or are UUIDs and String identifiers always assumed to be associated with a player?"

This is where the methods with a String-based identifier come in, each plugin generally has their own prefix to denote that it's not a player(i.e. town_, nation_ for towny, faction- for factions).

"On another note, I'm concerned that simply returning false when a method fails isn't clear enough. There are a multitude of different conditions you could consider a "failure" which, as a plugin calling the economy API, you might want to handle differently."

Yes, this is something I've been concerned with too, and is something I started improving upon earlier this week, and will be included in this PR when it is ready.

[mdcfe]

This is where the methods with a String-based identifier come in, each plugin generally has their own prefix to denote that it's not a player [...]

So do we assume that UUIDs should be associated with a player but Strings might not be?

---

Out of interest, is there any public documentation on what the different failure responses will look like?

[mdcfe]

Even with support of EssentialsX, I agree with @zml2008 that Reserve will struggle to gain the major adoption that Vault currently has due to the slow-moving nature of the Bukkit world.

However, I have no objections to shipping Reserve support within EssentialsX once supported. Splitting it out into a separate plugin wouldn't make sense, as the economy register system used internally within EssentialsX is designed so that EssentialsX ships its own support for external economy APIs and there isn't much point shipping only one side of the implementation in this case.

Can you confirm that EssentialsX and other plugins work together properly when using Reserve instead of Vault? Ideally they should work exactly as they would when using Vault.

[creatorfromhell]

Even with support of EssentialsX, I agree with @zml2008 that Reserve will struggle to gain the major adoption that Vault currently has due to the slow-moving nature of the Bukkit world.

Yes, I understand this, but I want to start with getting economy providers to support it first then I can focus on non-provider developers adding support.

However, I have no objections to shipping Reserve support within EssentialsX once supported. Splitting it out into a separate plugin wouldn't make sense, as the economy register system used internally within EssentialsX is designed so that EssentialsX ships its own support for external economy APIs and there isn't much point shipping only one side of the implementation in this case.

Can you confirm that EssentialsX and other plugins work together properly when using Reserve instead of Vault? Ideally they should work exactly as they would when using Vault.

Yes I tested this before and it worked with Towny(since they have Reserve support). Obviously, I'll test it again with the updates that have been made to the implementation.

[mdcfe]

Mostly minor formatting issues and a quirk with the diff.

[creatorfromhell]

Corrected the strange diff showing up for some reason, probably from the merge of 2.x into my branch, and also the requested changes. I still want to verify that all works well still with additional testing later today after work.

[mdcfe]

I've approved this pending testing.

For reference, a test build of EssentialsX with Reserve support can be downloaded here.

[Ichbinjoe]

mostly nits, this can go in without change.

[creatorfromhell]

This has been tested and works correctly.

The testing that was done:
As a non-provider:
Console: http://prntscr.com/qnrpzu
In-Game Through ChestShop 3: http://prntscr.com/qnrrq5
In Game with Essentials Sign: http://prntscr.com/qnruyi http://prntscr.com/qnrv6s

As a provider:
Switching to EssX from other eco plugin: http://prntscr.com/qns39e
Console: http://prntscr.com/qns4ht
ChestShop Transaction: http://prntscr.com/qnt0ud

[creatorfromhell]

Dead PR is dead.

[mdcfe]

The reason this was not reviewed in January is that I could not find any public versions of Reserve and The New Economy that actually worked together; all the builds I could find threw errors whenever any plugin tried to access the economy through either Reserve or Vault. I'm not prepared to merge this PR when I can't guarantee that it works at all.

I've been on a break for a while now and haven't been able to review major PRs for 2.18. Feel free to reopen this PR if you decide you want Reserve support added.