Skip to content
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

multicurrency, uuid and legacysupport #153

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

anjoismysign
Copy link

It follows the principle of @LlmDl about using UUIDs as an option but not as a hard fork.
It also provides a wrapper that can convert legacy providers (the ones that only make use of Economy and not IdentityEconomy). This is useful in case you wish to only listen to an IdentityEconomy and not have to make two layers for different economies... Take a look at the following as an example of what I am trying to explain... anjoismysign/BlobLib@68116fb

The wrappers also have built-in methods that will know how to register to Vault. This is because I have a pull request ready for Vault so that Vault detects when a legacy economy provider attempts to hook so it also provides an IdentityEconomy (if you don't understand why this, please start reading again).

I hope you don't heat up from this pull request. Most of IdentityEconomy was taken from the initial draft of @LlmDl. I don't have intentions to cause drama or discussion, I just want to finish this period of chaos... I believe that Vault (the original) is the way to go and follow! Again, sorry for all my previous mess...

LlmDl and others added 4 commits April 4, 2021 10:32
This commit adds UUID support to Vault, allowing plugins to bypass the
OfflinePlayer methods which result in Bukkit trying to resolve a player
to associate with the OfflinePlayer (via the server playercache and if
that player doesn't exist via Mojang.)

This is incredibly useful for any plugin which wants to have an Economy
account that isn't associated with a player. This includes Towny,
Factions, Shops plugins and others.

Most importantly: having UUID methods will give these plugins an avenue
to update from using the String accountName methods deprecated since
Vault 1.4, which doesn't result in slow OfflinePlayer creation.

AbstractEconomy has been updated so that the various Economy plugins
supported internally by Vault will have support for the new methods in
the same manner as when the OfflinePlayer methods were added.

Small javadoc typos have also been fixed up (extra {'s, an additional
{@link, etc.)
These methods are meant for players, non-players and anything with a
UUID.
To match the PR I have opened at the Vault repo, which has had the
native economy plugin support removed, the VaultAPI plugin no longer
requires the AbstractEconomy class.

Removal means that this Pull Request no longer calls
Bukkit.getOfflinePlayer(uuid), making this much safer.
…rt economy providers that are already able to hook into Vault.

Added MultiEconomy which considers an economy as a currency, granting a multi-currency abstraction.
Provided wrappers that help registering/hooking all three types of layers into Vault.
@anjoismysign anjoismysign changed the title Feature/multicurrency-uuid-legacysupport multicurrency, uuid, legacysupport Apr 1, 2023
@anjoismysign anjoismysign changed the title multicurrency, uuid, legacysupport multicurrency, uuid and legacysupport Apr 1, 2023
@anjoismysign anjoismysign marked this pull request as draft April 2, 2023 09:46
@anjoismysign anjoismysign marked this pull request as ready for review April 2, 2023 10:00
@anjoismysign
Copy link
Author

anjoismysign commented Apr 2, 2023

Also, solves #145

@GeorgeV220
Copy link

I believe Vault is abandoned, so probably a new fork with a different name like Vault2 or X should be made, idk to be honest.

@Geolykt
Copy link
Contributor

Geolykt commented May 27, 2023

I believe Vault is abandoned, so probably a new fork with a different name like Vault2 or X should be made, idk to be honest.

I mean the issue is that there isn't something that would force a change. And ABI breakages are pretty much a no-go with an application that is as ancient as Vault. So superficially, nothing changes.

A fork would make little sense - it would need to be a ground-up recode. There have been plenty of them in the past - but you only need to look around you to know what happened to them.

@GeorgeV220
Copy link

yes I understand, vault for better or worse has become an "industry standard" but I believe a good fork that is community based or a new implementation would be good. I don't know why the other vault forks didn't work but NuVotifier which is a better version of Votifier succeeded and has additional useful features.

@anjoismysign
Copy link
Author

I agree. It seems that contributors lost interest in it... My proposal doesn't break current API, it only extends it. I also made a Vault2 (the JavaPlugin) which makes use of this pull request/proposal and provides new layers based on old/legacy classes.
It's 100% possible to keep developing Vault & VaultAPI with new features without needing to deprecate software that already works!

@GeorgeV220
Copy link

I agree. It seems that contributors lost interest in it... My proposal doesn't break current API, it only extends it. I also made a Vault2 (the JavaPlugin) which makes use of this pull request/proposal and provides new layers based on old/legacy classes. It's 100% possible to keep developing Vault & VaultAPI with new features without needing to deprecate software that already works!

if someone is willing to do something like this and inform the community in relevant forums (Spigot, PaperMC, BuiltByBits and etc) I think it would have some impact. The plugin has remained more or less the same for many years and as minecraft evolves it has become somewhat outdated. It still works pretty well but we need new features like multicurrency, UUID support and even CompletableFutures if possible.

@Geolykt
Copy link
Contributor

Geolykt commented May 27, 2023

We need new features like multicurrency, UUID support and even CompletableFutures if possible.

No need to reinvent the wheel! APIs such as treasury already exist. But they all don't provide backwards compat with Vault for good reason. Implementing both worlds was very painful last time I tried.

@anjoismysign
Copy link
Author

What? Didn't you read my pull request?

@Geolykt
Copy link
Contributor

Geolykt commented May 27, 2023

Your PR basically introduces several shadow economies that exist in parallel. Merging them into one singular economy is a tall order. I am very much of the opinion that it isn't possible to execute with 100% correctness due to the in retrospect unfortunate decision to attach everything to names.

Even then UUID & multi-currency support and everything else is optional. While that guarantees that ABI breakages aren't super common it also means that there is little reason to stick to the old Vault formula - API consumers can't just use the new UUID API willy-nilly - they'd need to check whether a plugin even exposes it, causing severe programming overhead. Hence most Vault "successors" only relegate compat with Vault to compatibility bridges. The idea there is: If breakages have to exist, then you can also get all other breakages in.

Converting all old databases from being name-based to UUID-based is another exercise that most wouldn't dare (remember that fake players are a thing).

@GeorgeV220
Copy link

@anjoismysign
Copy link
Author

anjoismysign commented May 28, 2023

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

Successfully merging this pull request may close these issues.

4 participants