Skip to content

fix: resolve guild names with the same rule as duplicate detection - #848

Closed
darbyjack wants to merge 1 commit into
masterfrom
fix/case-insensitive-guild-lookup
Closed

darbyjack wants to merge 1 commit into
masterfrom
fix/case-insensitive-guild-lookup

Conversation

@darbyjack

Copy link
Copy Markdown
Member

The bug

/guild create decides whether a name is free by reducing it to the text a player sees and comparing without regard to case. GuildInputValidator.isNameTaken did that, so a guild called "Raiders" already occupied "raiders" and "&cRaiders" too.

The lookup every other command goes through used a different rule. GuildHandler.getGuild(String) stripped the colour codes from the stored name and compared the result with the typed name exactly:

ACFBukkitUtil.removeColors(guild.getName()).equals(name)

So the server refused to create "raiders" and then said there was no guild called "raiders". A guild whose name carries colour codes could not be found under those codes either. Every command that names a guild resolves its argument through the ACF guild context, which calls this method: /guild accept, /guild decline, /guild ally, /guild war challenge, and the /guild admin commands that take a guild name.

Reproduced before the fix, in GuildNameLookupTest, where 5 of 7 cases failed:

GuildNameLookupTest > a guild is found whatever case the name is typed in FAILED
    org.opentest4j.AssertionFailedError: expected: <me.glaremasters.guilds.guild.Guild@3cd9aa64> but was: <null>
GuildNameLookupTest > a guild is found by its plain name and by its coloured name FAILED
    org.opentest4j.AssertionFailedError: expected: <me.glaremasters.guilds.guild.Guild@73c9e8e8> but was: <null>
GuildNameLookupTest > duplicate detection and lookup agree on the same name FAILED
    org.opentest4j.AssertionFailedError: expected: not <null>
GuildNameLookupTest > the guild name check agrees with the lookup FAILED
    org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
GuildNameLookupTest > two guilds that differ only by colour are not resolved by an ambiguous name FAILED
    org.opentest4j.AssertionFailedError: expected: <null> but was: <me.glaremasters.guilds.guild.Guild@6cd3ad8a>
7 tests completed, 5 failed

The fix

GuildNameMatcher is a new utility in me.glaremasters.guilds.utils. It holds the one rule: translate the alternate colour codes, strip every colour code, compare without regard to case. GuildInputValidator.isNameTaken and GuildHandler.getGuild(String) both use it, so the two agree about which names are the same name.

getGuild(String) resolves in two steps. A guild whose visible name is exactly the typed one wins, which keeps a server that already holds "Raiders" and "raiders" behaving as it did. Failing that, a single guild that matches without regard to case is returned.

Two guilds whose visible names differ only by case or only by colour are a state duplicate detection prevents, but legacy or hand-edited data can still hold. Such a name is ambiguous. The old code took whichever of the two the map happened to yield first. The name now resolves to nothing, and the conflict is logged with the ids of the guilds involved. The command answers with the plugin's existing "That Guild does not exist!" message, since saying more would mean a new key in every language file.

GuildHandler.checkGuildNames(String) compared the raw stored name with the typed name, so it disagreed with the lookup for every guild whose name carries colour codes. It uses the same rule now.

The tab completion needed no change. It offers the visible name of each guild, and the lookup now matches that name back to the guild it came from.

Tests

  • GuildNameLookupTest, new, 7 cases: a guild stored as "Raiders" is found by "raiders" and by "RAIDERS"; a guild stored as "§cRaiders" is found by "Raiders", by "§cRaiders" and by "&cRaiders"; an unknown name resolves to nothing; duplicate detection and the lookup agree on the same name; two guilds that differ only by case or only by colour do not resolve by an ambiguous name, while each still answers to the name it carries; checkGuildNames agrees with the lookup.
  • GuildInputValidatorTest, one case added: duplicate detection and GuildNameMatcher apply the same rule to a list of spellings.
  • membership.spec.ts, one test added: an invited player accepts a guild whose name is typed in a different case.
  • admin.spec.ts, one test added: an admin command finds its guild when the name is typed in a different case.

Both end-to-end tests fail against the code as it was before this change and pass with it. The membership one never receives "You joined guild 'E2E000kCaseTest' successfully", and the admin one is answered with "That Guild does not exist!".

Results

./gradlew test testJava11 spotlessCheck --no-daemon
BUILD SUCCESSFUL

./gradlew check --no-daemon
BUILD SUCCESSFUL
Legacy API probe: 2 Material constant(s) checked against 1.8.8, 0 allow-listed.

./gradlew plugwrightTest --no-daemon --stacktrace
All tests passed!
  local  146 passed, 0 failed, 0 skipped   (7m 3s)

Compatibility

No new dependency and no new API. The change is Java code that compiles to Java 11 bytecode, and the 1.8.8 legacy probe still compiles. No Material constant was added, so nothing there needed an XSeries swap.

No config key, default or stored format changed. Guild names are stored exactly as before, and only the way a name is matched changed.

GuildHandler.getGuild(String) keeps its signature and still returns null when there is no such guild. The UUID and OfflinePlayer overloads are untouched.

Limitations

  • An ambiguous name, which means two guilds that differ only by case or colour, resolves to nothing. The server log names the guilds involved, and the way out is to rename one of them.
  • The player sees the existing "That Guild does not exist!" message in that case.
  • Invite codes are still matched exactly, as they always have been. A code is a token rather than a name, and the codes are random mixed-case strings.
  • getNameById and the invite list still show the stored name with its colour codes, which is what they showed before.

darbyjack pushed a commit that referenced this pull request Oct 22, 2026
The coverage inventory is contended between the open pull requests, and the
header and the per-section counts have to be the ones the merged file will
have whatever order they land in: 160 tests across 14 spec files, 18 in the
invite codes section, 21 in the guild lifecycle and membership sections and 8
in the admin commands section.

The rows for the tests that #848 and #851 add are in the tables here, since
neither of those pull requests touches this file, and the rows for #849's
tests were already here. Every test in the merged suite is listed once.

The membership section heading said 19 while the spec file holds 20 tests,
which is where the 21 comes from.
@darbyjack darbyjack closed this Oct 23, 2026
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