Calling Economy#hasAccount results in RuntimeException with Essentials when OfflinePlayer is missing the name

[Phoenix616]

When calling Economy#hasAccount(OfflinePlayer) when running Essentials as the economy provider can result in a RuntimeException thrown when the name is null.

This can happen due to AbstractEconomy#hasAccount(OfflinePlayer) just using the name of the OfflinePlayer object and passing that to the hasAccount(String) method. The problem is that the Bukkit API might return an OfflinePlayer object that has a null name when using Bukkit#getOfflinePlayer(UUID) in the case that the player is unknown and no name can be found.

One could argue that Essentials should properly catch the case where the name is null when checking for an account but I think that this could as well be handled in Vault directly as this is the one using OfflinePlayer#getName without checking if it's null and Essentials doesn't indicate that this is allowed, maybe the Essentials_Economy class should catch the case where the name is null? (Although that would kinda change the behaviour but I doubt anybody relies on this throwing a RuntimeException as the VaultAPI does not claim that this would happen.

Also here is a stacktrace for this happening that a user of ChestShop sent me (although this case should only be happening when a user error setting up the admin shop was made in the plugin so it's not too common): https://pastebin.com/aAADjeda

[Lightning11wins]

Maybe you should check to see if the name is null before calling Economy#hasAccount(OfflinePlayer)?

[Phoenix616]

Why would a plugin using the API have to test that themselves? The name should not be necessary as players should be identified by their UUID, not a name that could change at any time. If the method accepts an OfflinePlayer object then it has to handle all cases of how that object can be obtained with the Bukkit API. (I mean I'm not talking about a custom OfflinePlayer object that has some unconventional behaviour, this is an object directly gotten by calling Bukkit#getOfflinePlayer(UUID))

[Lightning11wins]

Ok... I see. I guess I can't say much because when I tried to use the API, it failed (or I failed, or something.)

[Phoenix616]

Seems to be happening with CraftConomy adapter too. Basically the AbstractEconomy should either check if the username is null or directly use the UUID instead. (Or the individual plugin adapter check if the name is null if the economy plugin does not support null names, using their UUID API would be a lot better though)

[Sleaker]

@Wumfie not really related at all, that's an issue with your shop plugin and how it handles interfacing with accounts itself.

What you basically described is "If I write something completely different I get different results!" not sure what you're expecting there.

[tom1422]

Wait I think we're talking about different things here

[Sleaker]

'fixed' in latest commit. This is more of a workaround for something that essentials should just make their own adapter to handle.

[Phoenix616]

Wouldn't it be better to directly get the UUID from the OfflinePlayer and use the add(UUID,BigDecimal) method instead of falling back to the name?

[Sleaker]

@Phoenix616 it most certainly would, those methods didn't exist before and updating the connector to use them would break compatibility with older versions of essentials.This is why I've mentioned over and over that Plugins now need to provide a maintained connector in their own plugins if they want the best functionality. All of the connectors that are currently housed in Vault are maintenance only and will not support additional functionality or update for API changes. If a long-running economy plugin changes it's API they are responsible for updating and maintaining a connector. This is also why Vault used to provide versioned connectors for the same economy. Rather than continuing to do that I've tried to communicate the need for Economy/Permission creators to make their own connectors.