LuckPerms

LuckPerms

41.4k Downloads

Major plugin compatibility issues

MackenzieMolloy opened this issue ยท 13 comments

commented

I'm getting this error spamming my console when using various plugins.
https://pastebin.com/E92YzFha
Using the latest luckperms (5.0.52)

commented

vk is setting a nullworld as guessed yesterday. Thats okay in vault by design. Seems to be a bug in lp.
https://github.com/MilkBowl/VaultAPI/blob/master/src/main/java/net/milkbowl/vault/chat/Chat.java#L107

commented

Since @vk2gpz is probably too busy making his 1000th plugin, I'll add the things he said to @MackenzieMolloy on Discord:

then you can ask them to make sure that "null" for "world" parameter is supported for setPlayerPrefix, method. convention is that if "null" is specified for "world", the prefix should be applied to the player regardless of the world the player is in.

null is valid option there.

other permission plugins will apply prefix regardless of world if null was specified.

they really need to fix it to support null in world
to comply with Vault's convention

no, i have no interest in talking to those who just ignore the specification of Vault.
i do not have any explicit support for LP.
they're the one claiming that LP works with Vault... which clearly does not since they do not follow the specification of Vault.

it's fine for them to claim that "LP is not like other perm plugins"
but he needs to learn that if he claims that LP is supported by Vault, they need to follow Vault's specification.

that's why i'm telling you, you need to ask LP to fix itself to comply with Vault's specification if they're hooking onto Vault.

I'm not sure exactly what needs to be done but we need input from @lucko on this one.

commented

Is this confirmed in the official Javadoc of Vault?

commented

Yes andre, see the link I provided soem comments earlier.

Guess needs a small change here, unless my thinking is too easy:
https://github.com/lucko/LuckPerms/blob/master/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/LuckPermsVaultChat.java#L226

commented

Yes andre, see the link I provided soem comments earlier.

Guess needs a small change here, unless my thinking is too easy:
https://github.com/lucko/LuckPerms/blob/master/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/LuckPermsVaultChat.java#L226

LuckPerms seems to already handle it as in the two below examples:
https://github.com/lucko/LuckPerms/blob/242993763c22b2d20e181422628d8ba01a6311f4/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/LuckPermsVaultChat.java#L236

https://github.com/lucko/LuckPerms/blob/242993763c22b2d20e181422628d8ba01a6311f4/bukkit/src/main/java/me/lucko/luckperms/bukkit/vault/LuckPermsVaultChat.java#L276

So maybe it's Vault's fault? Perhaps it doesn't send null but perhaps "null" (or literally NULL as shown in the Javadoc comment of the method)

commented

vk2gpz said:
NPE is coming out from

    if ((key.equalsIgnoreCase(DefaultContextKeys.SERVER_KEY) || key.equalsIgnoreCase(DefaultContextKeys.WORLD_KEY)) && value.equalsIgnoreCase("global")) {

I'll be honest - I have no idea I am talking about

commented

That comment didn't meant you, all good. (It was about a comment that has been deleted)
Please update to latest, which is 5.0.52 atm if you havent already and post error logs that were produced with that exact lp version

commented

That is on the latest version, the developer of MyPrefix (the plugin that caused those errors to emitted) has updated his plugin and these are the error i get now;
https://pastebin.com/Vd9bx3J4

commented

Fixed in the above commit.

no, i have no interest in talking to those who just ignore the specification of Vault.
i do not have any explicit support for LP.
they're the one claiming that LP works with Vault... which clearly does not since they do not follow the specification of Vault.
it's fine for them to claim that "LP is not like other perm plugins"
but he needs to learn that if he claims that LP is supported by Vault, they need to follow Vault's specification.

@vk2gpz It was just a bug? A bit wild to just immediately jump to the conclusion that I'm purposely "ignoring the specification", or conclude that a misimplementation of the Vault API is a result of an unwillingless to "learn" to be compatible on my part, lol.

I'm always happy to look into issues and fix bugs, but at least give me 24 hours to respond before jumping to "oh it must just be because they're incompetent and/or think they're too special to play by the rules" - hopefully you can see how absurd that is.

Turns out it was a very simple fix! @MackenzieMolloy please could you confirm that the issue is fixed in the latest version. (v5.0.53, download from here https://luckperms.net/)

commented

Also, to make things clear: To my knowledge is vault not supporting LP (directly), but more LP is supporting Vault... It's a minor, yet important detail if you ask me.
And if Vault actually does support LP: Cool.

commented

The latest commit has fixed my issue on the test server - let me try it in production.

commented

The latest comment has also fixed my issue in production - Thanks for the latest commit.

commented

Great, thanks :)