I have some CommandHelper code that uses the CHVault vault_pgroup() API. It always returns an empty array of group names when applied to an offline player, but works with online players. The same CommandHelper code works fine with bPermissions.

I modified CHVault to log the OfflinePlayer that is passed to the Vault getPlayerGroups(String,OfflinePlayer) API and I can confirm that a valid CraftOfflinePlayer with the correct UUID is used.

To me, this behaviour is counter to reasonable expectations, in that LuckPerms claims to support the Vault API, but Permission.getPlayerGroups(String,OfflinePlayer) does not actually support offline players.

I have looked around for a definitive word on LuckPerms' Vault support and I discovered this:

You cannot pull data for offline players using Vault.

in issue #201.

So my request is threefold:

1. Is there a way to coax LuckPerms to query an offline player's group membership via Vault?
2. If not, would you consider adding a configuration option to make that possible? LuckPerms seems to be a desirable plugin (it ticks all the boxes on it's Spigot page). I understand that being async is desirable, but I also know that some users would really rather things just work, even if the main thread is occasionally blocked for several milliseconds.
3. If the answer to the first two questions is 'no', can we at least get LuckPerms' documentation to reflect this limitation on Vault usage, please?

We've been discussing this in Discord a bit recently, I just hadn't got around to committing the change just yet!

Luckily, what you've suggested above is pretty much exactly what was proposed and agreed on by others.

Can you take a look at the commit description above and see if it matches what you were expecting? In other words, are you happy that the above change is the best solution?

I'm happy to hear other ideas about how it could be improved, even if it means experimenting with a totally different approach.

My primary objective is to ensure servers don't lag as a result of offline player data lookups, without server admins being aware of the consequences (and who to blame!).

We can't change the design of Vault - and we can't change the assumption of plugin authors that Vault is main thread friendly - so the challenge is providing decent implementations of these methods, using/relying on data that's not in memory, without lagging the server. I definitely think throwing an exception is better than returning a dummy / null result.

Thanks for the very prompt and positive replies to both the issues I raised. My faith in humanity is restored 😄

The commit looks pretty good. 😄 I do take some slight issue with calling them unsafe lookups. The prospect that a vault lookup would throw an exception makes the fast/async code path more unsafe from a user perspective than the sync path. The most explicitly "self-documenting" name would be something like vault-synchronous-offline-lookups (or some abbreviation), but vault-slow-lookups would capture the essence of it more than vault-unsafe-lookups, in my opinion.

Other than that, I think it should resolve quite a lot of the Vault compatibity issues I've seen reported here.

Thank you.

The reason for calling them "unsafe" is because when the option is enabled, database lookups will be performed on the main server thread, which is most certainly not a safe operation to perform.