Essentials
Essentials copied to clipboard
Add Reserve support to latest base.
This is my updated PR from https://github.com/EssentialsX/Essentials/pull/2601.
As stated in the original:
Reserve is a new alternative to Vault, with modern functionality. Implemented the Economy API in this PR.
Main Advantages:
- BigDecimal usage instead of Double for more precise amounts.
- 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 off the vault layers, if there are any methods that seem like they are not implemented in the most efficient way just let me know.
For more information about contributing to EssentialsX, see CONTRIBUTING.md: https://github.com/EssentialsX/Essentials/blob/2.x/CONTRIBUTING.md
Isn't there hella lot of recursive infinity loops in ReserveEconomyProvider
? Like isAccessor(String identifier, UUID accessor)
. Or this is just an early draft?
Isn't there hella lot of recursive infinity loops in
ReserveEconomyProvider
? LikeisAccessor(String identifier, UUID accessor)
. Or this is just an early draft?
This is an early draft but I'll definitely have a look at that next, probably just overlooked it when I was quickly implementing everything.
Isn't there hella lot of recursive infinity loops in
ReserveEconomyProvider
? LikeisAccessor(String identifier, UUID accessor)
. Or this is just an early draft?
This should be ready for review if you want to review it now. ~~I'll have screenshots of it in action soon.~~ Attached is the build with this included that I've been using for testing. EssentialsX-2.19.1-dev+14-bd00fa1.zip
Screenshot of using /sell hand with Reserve and an eco plugin: http://prntscr.com/1wfcsk6
Screenshots of EssentialsX with reserve and no eco plugin: http://prntscr.com/1wfdvvr http://prntscr.com/1wfed97
EssentialsX is GPLv3, but linking against Reserve would impose the terms of the AGPLv3 on all EssentialsX users. These terms would in turn make EssentialsX entirely unsuitable for servers who wish to maintain a private fork, or anyone who writes a plugin that depends on EssentialsX. We therefore cannot merge this PR in its current state.
However, the reason this hasn't had a response sooner is that there are an overwhelming number of issues with Reserve's code, API design and the Reserve project as a whole, and in order to leave a fair response, I've had to condense and summarise the most important of these issues:
- Reserve appears to be developed by a one-person team, with little to no input from others. The only public discussions with stakeholders about Reserve's economy API design that I can find are the discussions on PR #2601. The near-total lack of community engagement is very concerning for a project that aims to establish itself across the entire Bukkit ecosystem and replace a pre-existing and well-supported library.
- Reserve doesn't currently have a clear scope. The README claims it only intends to cover economy and eventually permissions, the commit history seems to suggest that permissions and chat were implemented then removed, and the issues tab lists several other plans for "APIs", some of which look more like utility classes than interface standards. It's hard to tell from a glance what Reserve is right now, let alone what it intends to be in the future.
- Reserve appears to have some sort of Vault interop, but this is completely undocumented. It's not at all clear whether users should expect Reserve to interact with an existing Vault install, and was the entire reason I was unable to test #2601 when it was originally opened.
- Reserve has a number of optional capabilites that economy providers may or may not implement, but the only means for economy providers to indicate to consumers what capabilities they support is wildly inconsistent, doesn't seem to have been thought through and doesn't even seem to exist for half of Reserve's optional capabilities? (And no, trial and error doesn't count.)
- Reserve's versioning scheme is not documented, and so plugin developers cannot safely assume any cross-compatibility between even consecutive versions of Reserve. If we assume Reserve is using Semantic Versioning, a universal standard for library versioning, then the current release of Reserve doesn't promise any API stability.
- From a glance at Reserve's codebase, there are a lot of code quality issues and red flags everywhere:
- Reserve compiles against an ancient, unsupported pre-release version of the Bukkit API. There is no reason to do this. Don't do this.
- The code style is heavily inconsistent, mixing different spacing, capitalisation and spellings of words.
- The plugin and API are distributed in the same package, meaning any plugin that depends on Reserve is required to pull in the entire plugin including internal classes.
- As mentioned before, the AGPLv3 is viral, and imposes its terms on software that links against yours, much like the GPLv3. Reserve cannot expect to gain mass adoption when it imposes more strict licensing terms on implementers and consumers than the Bukkit API itself.
Finally, Reserve's command code includes the facility to implement backdoor commands. There is no place for this facility in any public plugin whatsoever - any sensible developer would allow users to enable debug tools with reasonable warnings, permissions and/or config options, or simply not distribute modified debug builds in public. The existence of this code in public release versions of Reserve should be enough to bar Reserve and any software related to TheNewEconomy from any self-respecting server.
None of this is to say we aren't open to supporting an alternative to Vault in EssentialsX. Vault is far from perfect, being both difficult to use for anyone who wasn't around during its original conception and poorly retrofitted to function on modern Minecraft. However, Reserve falls short of being the alternative that the community needs. I would much prefer a community-led project that is designed from the ground up, without restrictive licensing and design choices. I will leave it to you whether you want to keep this PR open or close it, but as I stated earlier, we cannot support Reserve in its current state without causing licensing headaches for all of our downstreams.
- The code style is heavily inconsistent, mixing different spacing, capitalisation and spellings of words.
For me personally, does this sound like the project one of two things (or both):
- The dev constantly switched its own code style, giving this project some feeling of insecurity and instability.
- If the code is actually from other contributors would this suggest that the project has no enforced coding style which makes it quite unprofessional.
- The code comes from various other sources - probably Vault? - which would be questionable, perhaps license-violating (not giving credit to original authors) and/or would show the main dev to not really care about finding better, more optimized solutions.
This is just my 2 cents on this and I don't want to suggest that the dev is bad or has any kind of evil intend.
On a different note does Reserve's README mention a bintray download as an option which due to Bintray now being read-only, means that the release of Reserve is stuck on whatever was the latest release until the dev decides to go with a new distribution place like MavenCentral or a self-hosted alternative. This makes the usage of the API more restricted and in case devs want to support older versions, makes it more difficult to maintain proper dependencies for your own projects.
EssentialsX is GPLv3, but linking against Reserve would impose the terms of the AGPLv3 on all EssentialsX users. These terms would in turn make EssentialsX entirely unsuitable for servers who wish to maintain a private fork, or anyone who writes a plugin that depends on EssentialsX. We therefore cannot merge this PR in its current state.
However, the reason this hasn't had a response sooner is that there are an overwhelming number of issues with Reserve's code, API design and the Reserve project as a whole, and in order to leave a fair response, I've had to condense and summarise the most important of these issues:
Reserve appears to be developed by a one-person team, with little to no input from others. The only public discussions with stakeholders about Reserve's economy API design that I can find are the discussions on PR Added Reserve Support, and tested. #2601. The near-total lack of community engagement is very concerning for a project that aims to establish itself across the entire Bukkit ecosystem and replace a pre-existing and well-supported library.
Reserve doesn't currently have a clear scope. The README claims it only intends to cover economy and eventually permissions, the commit history seems to suggest that permissions and chat were implemented then removed, and the issues tab lists several other plans for "APIs", some of which look more like utility classes than interface standards. It's hard to tell from a glance what Reserve is right now, let alone what it intends to be in the future.
Reserve appears to have some sort of Vault interop, but this is completely undocumented. It's not at all clear whether users should expect Reserve to interact with an existing Vault install, and was the entire reason I was unable to test Added Reserve Support, and tested. #2601 when it was originally opened.
Reserve has a number of optional capabilites that economy providers may or may not implement, but the only means for economy providers to indicate to consumers what capabilities they support is wildly inconsistent, doesn't seem to have been thought through and doesn't even seem to exist for half of Reserve's optional capabilities? (And no, trial and error doesn't count.)
Reserve's versioning scheme is not documented, and so plugin developers cannot safely assume any cross-compatibility between even consecutive versions of Reserve. If we assume Reserve is using Semantic Versioning, a universal standard for library versioning, then the current release of Reserve doesn't promise any API stability.
From a glance at Reserve's codebase, there are a lot of code quality issues and red flags everywhere:
- Reserve compiles against an ancient, unsupported pre-release version of the Bukkit API. There is no reason to do this. Don't do this.
- The code style is heavily inconsistent, mixing different spacing, capitalisation and spellings of words.
- The plugin and API are distributed in the same package, meaning any plugin that depends on Reserve is required to pull in the entire plugin including internal classes.
As mentioned before, the AGPLv3 is viral, and imposes its terms on software that links against yours, much like the GPLv3. Reserve cannot expect to gain mass adoption when it imposes more strict licensing terms on implementers and consumers than the Bukkit API itself.
Finally, Reserve's command code includes the facility to implement backdoor commands. There is no place for this facility in any public plugin whatsoever - any sensible developer would allow users to enable debug tools with reasonable warnings, permissions and/or config options, or simply not distribute modified debug builds in public. The existence of this code in public release versions of Reserve should be enough to bar Reserve and any software related to TheNewEconomy from any self-respecting server.
None of this is to say we aren't open to supporting an alternative to Vault in EssentialsX. Vault is far from perfect, being both difficult to use for anyone who wasn't around during its original conception and poorly retrofitted to function on modern Minecraft. However, Reserve falls short of being the alternative that the community needs. I would much prefer a community-led project that is designed from the ground up, without restrictive licensing and design choices. I will leave it to you whether you want to keep this PR open or close it, but as I stated earlier, we cannot support Reserve in its current state without causing licensing headaches for all of our downstreams.
Fair enough, I will look over these comments and return after addressing them, thanks for your time.
- The code style is heavily inconsistent, mixing different spacing, capitalisation and spellings of words.
For me personally, does this sound like the project one of two things (or both):
The dev constantly switched its own code style, giving this project some feeling of insecurity and instability.
- If the code is actually from other contributors would this suggest that the project has no enforced coding style which makes it quite unprofessional.
The code comes from various other sources - probably Vault? - which would be questionable, perhaps license-violating (not giving credit to original authors) and/or would show the main dev to not really care about finding better, more optimized solutions.
This is just my 2 cents on this and I don't want to suggest that the dev is bad or has any kind of evil intend.
On a different note does Reserve's README mention a bintray download as an option which due to Bintray now being read-only, means that the release of Reserve is stuck on whatever was the latest release until the dev decides to go with a new distribution place like MavenCentral or a self-hosted alternative. This makes the usage of the API more restricted and in case devs want to support older versions, makes it more difficult to maintain proper dependencies for your own projects.
The code style difference is from a set of library files that at some point lost their credit header, which will be added back into the files. That's an oversight on my point. The download issue is just a simple update, reserve was moved to codemc
EssentialsX is GPLv3, but linking against Reserve would impose the terms of the AGPLv3 on all EssentialsX users. These terms would in turn make EssentialsX entirely unsuitable for servers who wish to maintain a private fork, or anyone who writes a plugin that depends on EssentialsX. We therefore cannot merge this PR in its current state.
However, the reason this hasn't had a response sooner is that there are an overwhelming number of issues with Reserve's code, API design and the Reserve project as a whole, and in order to leave a fair response, I've had to condense and summarise the most important of these issues:
Reserve appears to be developed by a one-person team, with little to no input from others. The only public discussions with stakeholders about Reserve's economy API design that I can find are the discussions on PR Added Reserve Support, and tested. #2601. The near-total lack of community engagement is very concerning for a project that aims to establish itself across the entire Bukkit ecosystem and replace a pre-existing and well-supported library.
Reserve doesn't currently have a clear scope. The README claims it only intends to cover economy and eventually permissions, the commit history seems to suggest that permissions and chat were implemented then removed, and the issues tab lists several other plans for "APIs", some of which look more like utility classes than interface standards. It's hard to tell from a glance what Reserve is right now, let alone what it intends to be in the future.
Reserve appears to have some sort of Vault interop, but this is completely undocumented. It's not at all clear whether users should expect Reserve to interact with an existing Vault install, and was the entire reason I was unable to test Added Reserve Support, and tested. #2601 when it was originally opened.
Reserve has a number of optional capabilites that economy providers may or may not implement, but the only means for economy providers to indicate to consumers what capabilities they support is wildly inconsistent, doesn't seem to have been thought through and doesn't even seem to exist for half of Reserve's optional capabilities? (And no, trial and error doesn't count.)
Reserve's versioning scheme is not documented, and so plugin developers cannot safely assume any cross-compatibility between even consecutive versions of Reserve. If we assume Reserve is using Semantic Versioning, a universal standard for library versioning, then the current release of Reserve doesn't promise any API stability.
From a glance at Reserve's codebase, there are a lot of code quality issues and red flags everywhere:
- Reserve compiles against an ancient, unsupported pre-release version of the Bukkit API. There is no reason to do this. Don't do this.
- The code style is heavily inconsistent, mixing different spacing, capitalisation and spellings of words.
- The plugin and API are distributed in the same package, meaning any plugin that depends on Reserve is required to pull in the entire plugin including internal classes.
As mentioned before, the AGPLv3 is viral, and imposes its terms on software that links against yours, much like the GPLv3. Reserve cannot expect to gain mass adoption when it imposes more strict licensing terms on implementers and consumers than the Bukkit API itself.
Finally, Reserve's command code includes the facility to implement backdoor commands. There is no place for this facility in any public plugin whatsoever - any sensible developer would allow users to enable debug tools with reasonable warnings, permissions and/or config options, or simply not distribute modified debug builds in public. The existence of this code in public release versions of Reserve should be enough to bar Reserve and any software related to TheNewEconomy from any self-respecting server.
None of this is to say we aren't open to supporting an alternative to Vault in EssentialsX. Vault is far from perfect, being both difficult to use for anyone who wasn't around during its original conception and poorly retrofitted to function on modern Minecraft. However, Reserve falls short of being the alternative that the community needs. I would much prefer a community-led project that is designed from the ground up, without restrictive licensing and design choices. I will leave it to you whether you want to keep this PR open or close it, but as I stated earlier, we cannot support Reserve in its current state without causing licensing headaches for all of our downstreams.
I've reopened this and will be responding accordingly.
When it comes to the single person aspect, I've contacted numerous economy plugin developers as well as developers that utilize vault for the API solely, which I've used to develop the API of Reserve. A lot of this input came from the Towny developer and developers which hook into TNE directly(hence the reason towny and chestshop already has reserve support built in).
The clear scope issue is completely my fault and the readme and documentation needs revamped entirely, I understand that's a major issue and will have to tackle that accordingly.
The vault interlop feature is something that needs included into that revamped documentation.
The optional capabilities are no longer in Reserve as they were removed a couple weeks ago due to the confusion that would ensue and the fact that it was too restrictive on provider implementations.
Versioning scheme is something that I'll have to add into that revamped documentation.
Code quality concerns: there was a PR that addressed this and updated to the latest dependencies. When it comes to distributing it should never need to be included into a jar ever since those optional API elements were removed. As for licensing concerns, Reserve will be switching to the GPLv3 license to make things easier for all plugin compatibility.
@creatorfromhell for Reserve (since it's an API) it's more suited the LGPLv3 license.
Closing as per comments above, if we add support for another economy standard it will be one that is being developed by a server software.