Essentials economy trying to use name for OfflinePlayers
RoboMWM opened this issue · 3 comments
As per discussion in this ticket for another plugin, https://dev.bukkit.org/bukkit-plugins/scs/tickets/861-passing-a-null-uuid-to-economy/ I discovered that EssentialsX is trying to use getName when doing economy actions for an account. There should be no reason to do this, and as mentioned in the ticket, it breaks instances where Bukkit#getOfflinePlayer(UUID) will return an OfflinePlayer object with a null name.
[20:48:35] RoboMWM: and if the player has been inactive for a while, doing Bukkit.getOfflinePlayer(UUID) will only return an OfflinePlayer object with a UUID and a null name
[20:49:46] RoboMWM: now, if you do Bukkit.getOfflinePlayer(string) it will return an offlineplayer object with the appropriate UUID. From then on (until server restart unless that inactive player joins before then), calls to getOfflinePlayer(UUID) will return with name
Here is the relevant stack trace from the above issue: https://gist.github.com/RoboMWM/5dc11e0c4f7b46028e6fb11b2029c3aa
Essentially, Essentials should only be using the UUID to determine where to store this data - there is no need to get the name.
Necro bump perhaps, but EssentialsX isn't responsible for this.
For as long as Vault has included methods that take an OfflinePlayer, all of its economy plugin support classes have extended the AbstractEconomy class, including the EssentialsEco support. AbstractEconomy provides methods that, if not overridden, will call the equivalent deprecated method that takes a String instead using the result of getName. Since Vault's EssentialsEco support was never updated to override those methods, it causes issues with both upstream Essentials for 1.7.10 and EssentialsX.
To fix this, we could make a PR to Vault that implements OfflinePlayer-based methods, but I'm not sure how quickly it would be merged and included in a release. Instead, I think it would be preferable to ship with our own implementation of VaultAPI's Economy class (similar to what LuckPerms does for Permission and Chat), which would also allow us to consider extending EssentialsX's economy functionality in the future without being bound to Vault's implementation.
@drtshock I'm working on a replacement Economy provider that uses the newer IUser methods and retrieves users by OfflinePlayer rather than by username.
