-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Add Reserve support to latest base. #4555
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Isn't there hella lot of recursive infinity loops in |
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. |
This should be ready for review if you want to review it now. Screenshot of using /sell hand with Reserve and an eco plugin: Screenshots of EssentialsX with reserve and no eco plugin: |
|
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:
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. |
For me personally, does this sound like the project one of two things (or both):
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. |
Fair enough, I will look over these comments and return after addressing them, thanks for your time. |
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 |
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. |
|
@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. |
This is my updated PR from #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:
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