Add UUID support to Vault.

[LlmDl]

Edit: Everything from below and more is found in the 2nd incarnation of this pr!

OP follows:

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.)

---

I am the lead dev of Towny, where we are still using the deprecated String methods instead of the OfflinePlayer methods.

Over the years updating the Vault support has become more of a priority for me, I see new economy plugins coming out that don't bother implementing the deprecated Vault methods, leading to their plugin not being able to handle the Town and Nation bank accounts in Towny, until one of their users makes an issue ticket for the economy plugin author to address.

Somewhat recently I did make an attempt to update Towny to use the OfflinePlayer methods, which had very poor results because of what happens when you try to make an OfflinePlayer in Bukkit that doesn't equate to a player UUID. The stalled PR is here if you're curious/bored. The results looked like this:

• We either see lag whenever an account is accessed after each restart or,
• We set the OfflinePlayer on loading each Town and Nation which results in a massive slowdown during the startup.
  • 2 towns and 1 nation went from loading in 29ms to 400+ms.

I have read through the various closed UUID tickets (#88, #38, #37) and I do acknowledge that the upstream providers is an issue, but I have heard that the Essentials team is looking at redoing their Vault implementation or even actively working on it already, giving them the ability to use UUIDs instead of OfflinePlayers would be useful to them as well.

Giving upstream and downstream plugins the option to use UUIDs is the right choice, most upstream plugins are storing their player accounts by a UUID already, downstream plugins are using UUIDs as well. Hopefully this PR can be a jump-off point and we can see a new version of the VaultAPI made available.

[Sleaker]

Do you think it's also appropriate to strip out the deprecated methods? This commit already breaks existing built-in implementations in Vault proper so those will all need to be updated or removed (most likely removed).

Secondarily if this work is being done, are you able to update the Permission side too?

Edit: If I'm remembering things correctly too, We've never implemented UUID support directly because at some point that means Vault calls Bukkit.getOfflinePlayer(uuid) - which Vault never ever wants to do. - the only way Vault can properly expose an API for this is if it doesn't shim those methods, or instead throws unsupported op exceptions if they're left unimplemented.

[LlmDl]

I actually would argue that the String economy calls shouldn't have been deprecated. There never was a suitable replacement for accounts that don't belong to a player.

I'm not sure which parts in this commit breaks existing installs, aside from any plugin that would use version 1.8 of the VaultAPI would require the economy plugin to also have 1.8 or newer so the new methods are present (I would consider this normal progress and not a fault. If a plugin doesn't update it doesn't update, Bukkit is 10+ years old at this point and we cannot let legacy plugins hold modern development back,)

I would have to look into the permissions side of things. I've never actually looked into it past the stuff surrounding Chat. Since I've never found an issue with things I suspect the Permissions side of things is not critical like the Economy support.

[lokka30]

I think we need entirely new methods for non-player transactions, and then the deprecated methods should be removed.
I think using UUID instead of OfflinePlayer is a good idea, although I still believe these methods should only be used for player transactions, hence my first statement.

[LlmDl]

Since most non-player objects are going to (or should have) UUIDs I don't agree that only player transactions should use the UUID methods. I am all-ears on what you think non-player transactions should look like though @lokka30, as that's the pain point for Towny-using server admins trying to find a good economy plugin in the wide selection available.

[lokka30]

I'm currently developing a successor to my old economy plugin, aiming for full compatibility across the board. I would be very glad to chat with you directly to ensure Towny fully is compatible with it. Then your users won't have to look far. 😃
Please assist me in clearing up my confusion here, I am very inclined to do what's best for server owners here.

How does the UUID system work for non-players by the way? Is a random UUID generated for non-player transactions? If so, what happens if such UUID was generated, later on a player joins with the same UUID?
Unless these things are sorted out, I think it is best to make a clear division in the API between player and non-player balances.

I have this thought to add non-player methods to the Vault API:

• deprecate the current OfflinePlayer methods, instead add the UUID methods as you suggested
• repurpose the current String playerName methods
  • remove their deprecation notice - these methods have been deprecated for a long time afaik and should be removed or modified now
  • these methods will now be used for non player transactions
  • the playerName variable is renamed to id or nonPlayerName or something better
  • note - still remove the deprecated bank methods.

Another thought I have is, how come do plugins use Vault for non-player balances? Is this so it can be accessed by other plugins? If so, what's against creating its own database for the plugin containing the non-player balances, and then adding an API to access the database for the other plugins?

[LlmDl]

I can only speak for Towny (the other big player in non-player economy accounts is Factions, but there's probably a lot more.) which has UUIDs for the Town and Nation objects.

We're currently stuck using the String vault methods, due to the reasons laid out in the original post. It's awfully close to what you're second point is talking about, this is what Towny and Factions have been doing since before the String methods were deprecated. What it doesn't help with is when a town or nation renames itself, which Towny takes care of in Vault. We want an economy method that accepts the UUID of the Town or Nation, a variable that will not ever change.

Some economy plugin take that name from the String methods, make a UUID from the name, then make an OfflinePlayer, then go to Vault with it (CMIEconomy does this.) This is very costly from a time-standpoint, as explained in the original post.

Since UUIDs are UUIDs I don't think there's any reason that a plugin cannot make a UUID account for their own objects (Town, Nation, Faction, etc.) Limiting UUID-made accounts to only player UUIDs who've logged into the server would just end up repeating what exists with OfflinePlayers.

[lokka30]

I can only speak for Towny (the other big player in non-player economy accounts is Factions, but there's probably a lot more.) which has UUIDs for the Town and Nation objects.

We're currently stuck using the String vault methods, due to the reasons laid out in the original post. It's awfully close to what you're second point is talking about, this is what Towny and Factions have been doing since before the String methods were deprecated. What it doesn't help with is when a town or nation renames itself, which Towny takes care of in Vault. We want an economy method that accepts the UUID of the Town or Nation, a variable that will not ever change.

Some economy plugin take that name from the String methods, make a UUID from the name, then make an OfflinePlayer, then go to Vault with it (CMIEconomy does this.) This is very costly from a time-standpoint, as explained in the original post.

Since UUIDs are UUIDs I don't think there's any reason that a plugin cannot make a UUID account for their own objects (Town, Nation, Faction, etc.) Limiting UUID-made accounts to only player UUIDs who've logged into the server would just end up repeating what exists with OfflinePlayers.

I see. How is it ensured that no UUID will overlap with any real player's UUID?
I also now see why you are suggesting the UUID system and I completely agree so long the above is addressed.

[LlmDl]

I see. How is it ensured that no UUID will overlap with any real player's UUID?

It's pretty unlikely we'd ever yoink a player UUID.

[lokka30]

I see. How is it ensured that no UUID will overlap with any real player's UUID?

It's pretty unlikely we'd ever yoink a player UUID.

Looks good to me haha. I do have some thoughts on your PR though, I'll submit comments now.

[lokka30]

Is this meant to be player-only? Same goes with methods beneath this.

[LlmDl]

So the way I remember this class working, is that most economy plugins will end up replacing all of these methods, the economy plugins that will actually run these methods are the ones which are still in included in the Vault source:

Of these plugins the only one I see being used is EssentialsEconomy, which afaik, has already internalized their Vault implementation in their 2.19 version. The economy plugin I maintain (ico5) was dropped from Vault long-ago. Gringott's is the only other one that I see people wanting to use (but it is not maintained any longer.)

Before this PR is ever accepted something has to be done to avoid calling Bukkit.getOfflinePlayer(), as this should never be used for cost/time/lag reasons. One way to go about this might be to drop all Economy plugins from Vault.

[Sleaker]

Yah for this PR to get accepted I basically would need to remove all the implementations from Vault. At least we are on the same page as far as that's concerned.

[LlmDl]

I think its about time. Further down you can see my comment where I went and dug up when those plugins were last updated. Did you want some help/a pull request made up for Vault?

Further, when they are removed that means the entire AbstractEconomy class can be removed from VaultAPI, correct?

[LlmDl]

@lokka30 thanks for the review. This PR hasn't had much attention to it and another set of eyes on it is a very cool thing to wake up to in the morning.

[Geolykt]

NOTE: as noted by #112 (comment) this comment has a few issues

In my economy plugin I just listen for illegal playernames ([^a-zA-Z0-9_]), if it is illegal then it is safe to assume that the player does not exist (which is then treated specially in the name of "security"). Of course this is not offline-mode compatible but I do not intent on supporting obviously grayzone clients (and on the topic of bedrock -> java bridges: they are cursed enough already). That being said this doesn't fix anything at all, but is a good way to reduce easily detectable issues.

[LlmDl]

In my economy plugin I just listen for illegal playernames ([^a-zA-Z0-9_]), if it is illegal then it is safe to assume that the player does not exist (which is then treated specially in the name of "security"). Of course this is not offline-mode compatible but I do not intent on supporting obviously grayzone clients (and on the topic of bedrock -> java bridges: they are cursed enough already). That being said this doesn't fix anything at all, but is a good way to reduce easily detectable issues.

In Towny our default economy account prefixes are town- and nation-, specifically including the - character so that players cannot log in with an economy account's name. Of course we allow admins to choose the prefix (and EssEco always converted it to town_ and nation_ which made it a security issue,) so we also have a listener that blocks players from logging in if they have a username starting with a eco account prefix.

Wouldn't be an issue if it were all stored by UUIDs anyways.

[lokka30]

Really like this PR now. Oh, and one other thing I am concerned about, is that economy plugins may want to know if a transaction is for a player or not. For example, my upcoming plugin will feature /baltop, and it would be neat to know (without further checks e.g. comparing uuids against a 'known player uuids' database) if a transaction is for a player or not, so then I can add an extra column isPlayer which will make baltop sorting easier ;)
No biggie if this is not resolved though since of course I can do the known player uuids database comparison and that'll patch it.

[LlmDl]

Really like this PR now. Oh, and one other thing I am concerned about, is that economy plugins may want to know if a transaction is for a player or not. For example, my upcoming plugin will feature /baltop, and it would be neat to know (without further checks e.g. comparing uuids against a 'known player uuids' database) if a transaction is for a player or not, so then I can add an extra column isPlayer which will make baltop sorting easier ;) No biggie if this is not resolved though since of course I can do the known player uuids database comparison and that'll patch it.

Something like this could be very useful for Vault in the convert command. Currently it relies on the server returning an OfflinePlayer, something which won't happen for fake-player accounts.

Also useful: A way to make an economy plugin return all of its accounts, again for conversion.

[lokka30]

Really like this PR now. Oh, and one other thing I am concerned about, is that economy plugins may want to know if a transaction is for a player or not. For example, my upcoming plugin will feature /baltop, and it would be neat to know (without further checks e.g. comparing uuids against a 'known player uuids' database) if a transaction is for a player or not, so then I can add an extra column isPlayer which will make baltop sorting easier ;) No biggie if this is not resolved though since of course I can do the known player uuids database comparison and that'll patch it.

Something like this could be very useful for Vault in the convert command. Currently it relies on the server returning an OfflinePlayer, something which won't happen for fake-player accounts.

Also useful: A way to make an economy plugin return all of its accounts, again for conversion.

In your PR, I think this method:

boolean createAccount(UUID uuid);

should become:

boolean createAccount(UUID uuid, boolean isPlayerAccount);

Also useful: A way to make an economy plugin return all of its accounts, again for conversion.

I suggest this method:

List<UUID> getAccounts();

Although I'm unsure if there should be ways to get player or non-player accounts specifically:

List<UUID> getPlayerAccounts();
List<UUID> getNonPlayerAccounts();

since there is a method already to get a list of bank accounts ;)

[LlmDl]

I think the first step is getting this accepted as it is a pretty big change, although I am not opposed to it.

@Sleaker what do you figure? What are you thoughts about it so far and would you support discerning player accounts from non-player accounts?

[LlmDl]

I did another look into my comment earlier:

Before this PR is ever accepted something has to be done to avoid calling Bukkit.getOfflinePlayer(UUID), as this should never be used for cost/time/lag reasons. One way to go about this might be to drop all Economy plugins from Vault.

This is only being used in AbstractEconomy, which from my understanding is only ever called in the actual Vault plugin, and only when one of the 20 included economy plugins are used.

I have a feeling that AbstractEconomy was only made to act as a band-aid for older economy plugins which did not have the OfflinePlayer methods, making it so that the economy plugin would fall back gracefully to using the String methods using the offlineplayer.getName().

The way I've changed the AbstractEconomy file this time around mirrors the same thing. It is going to attempt to get the OfflinePlayer from the UUID and in the case that it is a player who has played before it will succeed, if it is a non-player account it will likely fail. Again, this is only when either an outdated EssentialsEconomy (pre-2.19) or Gringotts (in the early stages of being abandoned,) is used. All of the other 18 plugins in Vault are completely inactive/abandoned:

[lokka30]

@Sleaker, any update on this PR? :)

[Jikoo]

In my economy plugin I just listen for illegal playernames ([^a-zA-Z0-9_]), if it is illegal then it is safe to assume that the player does not exist (which is then treated specially in the name of "security"). Of course this is not offline-mode compatible but I do not intent on supporting obviously grayzone clients (and on the topic of bedrock -> java bridges: they are cursed enough already). That being said this doesn't fix anything at all, but is a good way to reduce easily detectable issues.

@Geolykt So you're aware, there's actually at least one real Java edition account that has a period in its name: https://namemc.com/profile/..Shadow...1
Supporting periods has the side benefit of silencing most Geyser users' complaints, I'm told that's Geyser's new default prefix for Bedrock clients.

In your PR, I think this method:

boolean createAccount(UUID uuid);

should become:

boolean createAccount(UUID uuid, boolean isPlayerAccount);

I think that UUIDs for non-player accounts are the way to go, but I disagree about the method signature change. All non-player accounts effectively are bank accounts, yes? If createAccount is purely for player accounts I think it would probably be more helpful to deprecate the unclear methods and replace them with createPlayer(UUID), playerBalance(UUID), etc. to match bank methods.

I do think it would be nice to have a method to get all player accounts to match how it's possible to get all bank accounts.

Regarding new methods:
I would also like to see defaults implemented so that new modern implementations do not bother with now-deprecated methods. For example, accessing a bank with a String instead of a UUID:

@Deprecated
default @NotNull EconomyResponse bankBalance(@Nullable String name) {
  if (name == null) {
    name = "null";
  }

  UUID accountId = UUID.nameUUIDFromBytes(("BankAccount:" + name).getBytes(StandardCharsets.UTF_8));

  return bankBalance(accountId);
}

@NotNull EconomyResponse bankBalance(@NotNull UUID accountId);

The legacy method is still available to plugins and still overridden in existing economy implementations, but a modern implementation doesn't even have to consider its existence.

An aside: Not sure if it's a stylistic choice or what, but all interface methods are always public. The public modifier is unnecessary.

[LlmDl]

I think that UUIDs for non-player accounts are the way to go, but I disagree about the method signature change. All non-player accounts effectively are bank accounts, yes? If createAccount is purely for player accounts I think it would probably be more helpful to deprecate the unclear methods and replace them with createPlayer(UUID), playerBalance(UUID), etc. to match bank methods.

The concept of non-player accounts always being bank accounts was probably the plan 11 years ago, but the reality is that many economy plugins do not have support for the bank methods. I also don't want to turn this Pull Request into something gargantuan, take-it-all-or-leave it scenario for Sleaker. OfflinePlayer methods can be deprecated, this PR finalized, and pushed out to the masses as Vault 1.8 after some of the new API is added. Plugins can then begin to switch over to the UUID methods and in a year or so Vault 2.0 can be brought out with the deprecated methods removed. Plugins that cannot adapt in a year are probably not maintained and shouldn't factor in to the advancement of Vault.

I do think it would be nice to have a method to get all player accounts to match how it's possible to get all bank accounts.

If this PR is accepted I'll be broadening the API. Until that time I don't see the point in fleshing out the API further unless we know its going to be merged.

Regarding new methods: I would also like to see defaults implemented so that new modern implementations do not bother with now-deprecated methods. For example, accessing a bank with a String instead of a UUID:

@Deprecated
default @NotNull EconomyResponse bankBalance(@Nullable String name) {
  if (name == null) {
    name = "null";
  }

  UUID accountId = UUID.nameUUIDFromBytes(("BankAccount:" + name).getBytes(StandardCharsets.UTF_8));

  return bankBalance(accountId);
}

@NotNull EconomyResponse bankBalance(@NotNull UUID accountId);

The legacy method is still available to plugins and still overridden in existing economy implementations, but a modern implementation doesn't even have to consider its existence.
This could be helpful, but the idea is that with UUID accounts we can finally drop support for String and OfflinePlayer methods.

[Jikoo]

The concept of non-player accounts always being bank accounts was probably the plan 11 years ago, but the reality is that many economy plugins do not have support for the bank methods. I also don't want to turn this Pull Request into something gargantuan, take-it-all-or-leave it scenario for Sleaker. OfflinePlayer methods can be deprecated, this PR finalized, and pushed out to the masses as Vault 1.8 after some of the new API is added. Plugins can then begin to switch over to the UUID methods and in a year or so Vault 2.0 can be brought out with the deprecated methods removed. Plugins that cannot adapt in a year are probably not maintained and shouldn't factor in to the advancement of Vault.

That's fair, it's a lot easier to review a smaller PR.

Honestly surprised to hear that few economies support banks - the only one I can really see arguing it doesn't work is that one (might've been GoldIsMoney?) that exclusively used physical items in players' inventories for currency. Then again, I'm hardly out shopping around for economy plugins, I'm sure you see way more interaction with them for town balances and such.
Would definitely like to see an overhaul of banks, but you're right that it's probably better left for a future update and separate PR.

[LlmDl]

Yeah the default for all of the new economy plugins that pop up on a constant basis is typically to skip banks, ignore the string methods and assume that every account will be a player account.

I've even seen multiple plugins have the createAccount() methods return false and then use a onplayerjoin listener to create the account in their plugin. Towny really didn't play well with those.

[Geolykt]

@Geolykt So you're aware, there's actually at least one real Java edition account that has a period in its name: https://namemc.com/profile/..Shadow...1
Supporting periods has the side benefit of silencing most Geyser users' complaints, I'm told that's Geyser's new default prefix for Bedrock clients.

Thanks, I guess I could add that symbol to my regex. Of course, there are far more nowadays illegal symbols out there but at some point you will have to draw the line

[lokka30]

Yeah the default for all of the new economy plugins that pop up on a constant basis is typically to skip banks, ignore the string methods and assume that every account will be a player account.

I've even seen multiple plugins have the createAccount() methods return false and then use a onplayerjoin listener to create the account in their plugin. Towny really didn't play well with those.

Could we add a method to the PR to address this - boolean createsPlayerAccountsOnJoin()? What do you guys think about this?

[LlmDl]

Yeah the default for all of the new economy plugins that pop up on a constant basis is typically to skip banks, ignore the string methods and assume that every account will be a player account.
I've even seen multiple plugins have the createAccount() methods return false and then use a onplayerjoin listener to create the account in their plugin. Towny really didn't play well with those.

Could we add a method to the PR to address this - boolean createsPlayerAccountsOnJoin()? What do you guys think about this?

That would require a listener that just points to the createAccount() method. Not really in the scope of the API. Most economy plugins are going to make one internally and its not going to be a shop plugin or Towny calling on vault to make an account.

[lokka30]

Yeah the default for all of the new economy plugins that pop up on a constant basis is typically to skip banks, ignore the string methods and assume that every account will be a player account.
I've even seen multiple plugins have the createAccount() methods return false and then use a onplayerjoin listener to create the account in their plugin. Towny really didn't play well with those.

Could we add a method to the PR to address this - boolean createsPlayerAccountsOnJoin()? What do you guys think about this?

That would require a listener that just points to the createAccount() method. Not really in the scope of the API. Most economy plugins are going to make one internally and its not going to be a shop plugin or Towny calling on vault to make an account.

I'm unsure if you understood my suggestion correclty: if I have misunderstood you then I apologize.

The method I suggested simply tells plugins whether an economy provider creates accounts when players join the server or not. If the provider makes it return true then any plugin can safely assume the provider uses a PlayerJoinEvent listener to create player accounts automatically.

Regardless I am unsure if it would be better just to make it standard that economy providers should create player accounts on join. I don't see why any plugin wouldn't want an account for each player.

[Jikoo]

Yeah the default for all of the new economy plugins that pop up on a constant basis is typically to skip banks, ignore the string methods and assume that every account will be a player account.
I've even seen multiple plugins have the createAccount() methods return false and then use a onplayerjoin listener to create the account in their plugin. Towny really didn't play well with those.

Could we add a method to the PR to address this - boolean createsPlayerAccountsOnJoin()? What do you guys think about this?

That would require a listener that just points to the createAccount() method. Not really in the scope of the API. Most economy plugins are going to make one internally and its not going to be a shop plugin or Towny calling on vault to make an account.

I'm unsure if you understood my suggestion correclty: if I have misunderstood you then I apologize.

The method I suggested simply tells plugins whether an economy provider creates accounts when players join the server or not. If the provider makes it return true then any plugin can safely assume the provider uses a PlayerJoinEvent listener to create player accounts automatically.

Regardless I am unsure if it would be better just to make it standard that economy providers should create player accounts on join. I don't see why any plugin wouldn't want an account for each player.

I believe the point is that the economy providers that do this are horribly incorrectly implementing the interface. They clearly have the capacity to create accounts and do so, but they do not implement the correct method for doing so. I actually stumbled on one by accident last night (this discussion raised my curiosity enough to check out the source of the first economy plugin I saw in someone's signature) and boy was it a trip. The entire Vault implementation was an absolute mess. This is a modern plugin, written under 2 years ago and actively maintained, with no support for name changes and no support for account creation. The implementation used names instead of UUIDs and all creation methods just returned false. A listener created the accounts on first join using methods not exposed through Vault. I couldn't believe it. Stunningly bad.

It's not really the API's job to police implementations for misuse, although technically Vault proper could attempt fake account creation and then disallow providers who do not actually support it. It's just that 1) that's outside of the scope of this PR and 2) if Vault starts doing that, the spaghetti writers are likely just going to start doing worse and worse things to circumvent it instead of implementing the methods. "Oh, Vault disables my Economy if I return false during account creation, better just return true and do nothing!"

This PR's scope is purely to add methods that access player accounts via UUID. Anything else (the bank change I suggested, etc.) is only going to make it more difficult to get it pulled by adding complexity.

[LlmDl]

Yeah the major take-away from this PR is that Vault's use of OfflinePlayers is probably totally fine when it comes to Chat and Permissions because they will (usually) deal with players who are online. But when it comes to Economy, an OfflinePlayer look up ends up being an extra step (and in the case of non-player accounts a very laggy one,) compared to using UUIDs. UUIDs being something which every player has, any desirable Object could have, and which every Economy database should probably be keyed by.

The method I suggested simply tells plugins whether an economy provider creates accounts when players join the server or not. If the provider makes it return true then any plugin can safely assume the provider uses a PlayerJoinEvent listener to create player accounts automatically.

I understand you now. I'm still not sure why an economy-using-plugin like a shop would have to know if the economy plugin makes an account on join. Most economy plugins probably do, the main thing is they should also be able to be told to make an account via the VaultAPI createAccount() methods (Towny making town and nation bank accounts for instance.)

Regardless I am unsure if it would be better just to make it standard that economy providers should create player accounts on join. I don't see why any plugin wouldn't want an account for each player.

There's probably a few plugins that require the player to have a permission node to use currency/have their account. You just never know.

[Sleaker]

I'm still concerned that these changes just aren't compatible at all with existing implementations. So this basically forces everyone to re-adopt a new set of functionality. If this is the case then I would prefer if all of the API got redone similar to how apache commons handled their API changes. net.milkbowl.vault2. This might be a better option now that I've had a change to think through everything. That allows the old interface to be completely marked as @Deprecated and allow for a period of time where plugins adopt the new API.

Thoughts on this?

[cerealcable]

I'm still concerned that these changes just aren't compatible at all with existing implementations. So this basically forces everyone to re-adopt a new set of functionality. If this is the case then I would prefer if all of the API got redone similar to how apache commons handled their API changes. net.milkbowl.vault2. This might be a better option now that I've had a change to think through everything. That allows the old interface to be completely marked as @Deprecated and allow for a period of time where plugins adopt the new API.

Thoughts on this?

I second this direction especially if we can publish both API versions within the same release. Ideally there's a large period of time where apps can coexist using a mix of new and old versions. There sure have been a lot of major breaking changes that have hurt projects significantly.

This would also give flexibility to provide more significant changes that match the current needs now that the usages and needs have greatly changed.

One suggestion (just considering it right now), we namespace the API specific to it's function, such as net.milkbowl.vault.economy perhaps?

[LlmDl]

I can get behind that I think. I'll see what I can work up next time I've got some free time.

[cerealcable]

@LlmDl looking at your other PR in Vault for removing the economies lead me back to this discussion about deprecating the old API and making the new Vault2 implementation, is this something you're still interested in pursuing?

[LlmDl]

@cerealcable it's something I picked up again this afternoon.

There's a couple of shortfalls that aren't yet documented in the PR so far, which I've started work on. The changes @Sleaker wanted to see happen first will also be included and are actually quite free-ing. I hope to have something to show the team soon.

[LlmDl]

Closed in lieu of #138.