Skip to content

Fix multiple bugs in chat plugin integrations and Vault - #948

Open
Smorki wants to merge 1 commit into
MilkBowl:masterfrom
Smorki:fix-multiple-bugs
Open

Fix multiple bugs in chat plugin integrations and Vault#948
Smorki wants to merge 1 commit into
MilkBowl:masterfrom
Smorki:fix-multiple-bugs

Conversation

@Smorki

@Smorki Smorki commented Aug 11, 2026

Copy link
Copy Markdown

Description

This PR fixes four small but real bugs I noticed while reading through the codebase.

1. Chat_PermissionsEx — null check on wrong variable

Four methods (getGroupPrefix, setGroupPrefix, getGroupSuffix, setGroupSuffix) check if (group != null) where group is the String parameter (never null in practice) instead of pGroup, the resolved PermissionGroup (which CAN be null if the group name doesn't exist). This means when a non-existent group name is passed, the null check silently passes and the next line NPEs on pGroup.

Fix: Check pGroup != null instead of group != null.

2. Chat_OverPermissionssetGroupSuffix writes to "prefix" key

Copy-paste bug: setGroupSuffix calls setGroupInfoString(world, group, "prefix", suffix), overwriting the group's prefix with the suffix value instead of setting the suffix.

Fix: "prefix""suffix".

3. Vault.java — missing null check in findCustomData

findCustomData null-checks the RegisteredServiceProvider for Economy and Chat before calling .getProvider(), but does not null-check Permission. If no permission provider is registered (edge case, but possible), this would NPE.

Fix: Apply the same null-check pattern used for Economy and Chat, defaulting to "No Permission" when unavailable.

4. Vault.java — typo "Converson" → "Conversion"

Admin-facing typo in the /vault-convert command output.

Notes

  • No public API changes.
  • All fixes are behavior-preserving for the intended code path.
  • The Chat_PermissionsEx and Chat_OverPermissions bugs affect users of those permission plugins when calling group-related methods.
  • The Vault.java null-check brings consistency with the existing Economy/Chat handling pattern in the same method.

@Smorki

Smorki commented Aug 23, 2026

Copy link
Copy Markdown
Author

can anyone review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant