Skip to content

fix: correct command tab completion #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryanbarlow97 merged 3 commits into from
Oct 15, 2026
Merged

fix: correct command tab completion #17

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e02c757
fix: complete eligible infestation commands and provinces
ryanbarlow97 Oct 15, 2026
abdf8df
fix: preserve infestation command visibility permissions
ryanbarlow97 Oct 15, 2026
a84a8f8
fix: filter here completion by province eligibility
ryanbarlow97 Oct 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 36 additions & 2 deletions src/main/java/net/tfminecraft/infestations/command/CommandManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import net.tfminecraft.infestations.infestation.Severity;
import net.tfminecraft.infestations.loader.GroupLoader;
import net.tfminecraft.infestations.utils.Provinces;
import net.tfminecraft.simplefactions.SimpleFactions;

public final class CommandManager implements CommandExecutor, TabCompleter {

Expand Down Expand Up @@ -195,17 +196,50 @@ public List<String> onTabComplete(CommandSender sender, Command command, String
return List.of();
}
if (args.length == 2 && (args[0].equalsIgnoreCase("set") || args[0].equalsIgnoreCase("clear"))) {
return prefix(List.of("here"), args[1]);
List<String> options = new ArrayList<>();
var manager = Infestations.plugin.getInfestationManager();
if (args[0].equalsIgnoreCase("clear")) {
manager.all().forEach(infestation -> options.add(String.valueOf(infestation.getProvinceId())));
} else {
var sf = SimpleFactions.getInstance();
if (sf != null && sf.getProvinceManager() != null) {
sf.getProvinceManager().getProvinces().stream()
.filter(province -> Provinces.validLand(province.getId()) && manager.get(province.getId()) == null)
.forEach(province -> options.add(String.valueOf(province.getId())));
}
}
Integer province = completionProvince(sender, "here");
if (province != null && Provinces.validLand(province)
&& (args[0].equalsIgnoreCase("clear")
? manager.get(province) != null : manager.get(province) == null)) {
options.add("here");
}
return prefix(options, args[1]);
}
if (args.length == 3 && args[0].equalsIgnoreCase("set")) {
return prefix(new ArrayList<>(GroupLoader.groupIds()), args[2]);
Integer province = completionProvince(sender, args[1]);
if (province == null || !Provinces.validLand(province)
|| Infestations.plugin.getInfestationManager().get(province) != null) return List.of();
return prefix(GroupLoader.groupIds().stream()
.filter(group -> GroupLoader.allowsTerrain(group, province)).toList(), args[2]);
}
if (args.length == 4 && args[0].equalsIgnoreCase("set")) {
return prefix(Arrays.stream(Severity.values()).map(Severity::id).collect(Collectors.toList()), args[3]);
}
return List.of();
}

private static Integer completionProvince(CommandSender sender, String token) {
if ("here".equalsIgnoreCase(token)) {
return sender instanceof Player player ? Provinces.at(player) : null;
}
try {
return Integer.valueOf(token);
} catch (NumberFormatException ex) {
return null;
}
}

private static List<String> prefix(List<String> options, String typed) {
String t = typed.toLowerCase(Locale.ROOT);
List<String> out = new ArrayList<>();
Expand Down
2 changes: 1 addition & 1 deletion src/main/resources/plugin.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ commands:
description: Infestation admin commands
usage: /infestation <reload|set|clear|list>
aliases: [infestations]
permission: infestations.admin
permission: infestations.admin;infestations.admin.reload
lure:
description: Leave a lure you have joined
usage: /lure leave
Expand Down
131 changes: 129 additions & 2 deletions src/test/java/net/tfminecraft/infestations/command/CommandManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,11 @@ private void run(CommandSender sender, String... args) {
admin(); when(console.hasPermission("infestations.admin.reload")).thenReturn(true);
assertEquals(List.of("reload", "set", "clear", "list"), complete(console, command, ""));
assertEquals(List.of("set"), complete(console, command, "S"));
assertEquals(List.of("here"), complete(console, command, "set", "H"));
assertEquals(List.of("here"), complete(console, command, "clear", ""));
assertEquals(List.of(), complete(console, command, "set", "H"));
assertEquals(List.of(), complete(console, command, "clear", ""));
groups.when(GroupLoader::groupIds).thenReturn(Set.of("rats"));
provinces.when(() -> Provinces.validLand(4)).thenReturn(true);
groups.when(() -> GroupLoader.allowsTerrain("rats", 4)).thenReturn(true);
assertEquals(List.of("rats"), complete(console, command, "set", "4", "r"));
assertEquals(Severity.values().length, complete(console, command, "set", "4", "rats", "").size());
assertEquals(List.of(), complete(console, command));
Expand All @@ -141,6 +143,131 @@ private void run(CommandSender sender, String... args) {
assertEquals(List.of("leave"), complete(console, lure, "L"));
assertEquals(List.of(), complete(console, lure, "leave", ""));
}
@Test void provinceAndGroupCandidatesRespectLiveStateAndSender() {
admin();
when(player.hasPermission("infestations.admin")).thenReturn(true);
var sf = mock(net.tfminecraft.simplefactions.SimpleFactions.class, RETURNS_DEEP_STUBS);
var land = mock(net.tfminecraft.simplefactions.map.provinces.Province.class);
var occupied = mock(net.tfminecraft.simplefactions.map.provinces.Province.class);
var water = mock(net.tfminecraft.simplefactions.map.provinces.Province.class);
when(land.getId()).thenReturn(42); when(occupied.getId()).thenReturn(43); when(water.getId()).thenReturn(44);
when(sf.getProvinceManager().getProvinces()).thenReturn(List.of(land, occupied, water));
provinces.when(() -> Provinces.validLand(42)).thenReturn(true);
provinces.when(() -> Provinces.validLand(43)).thenReturn(true);
provinces.when(() -> Provinces.at(player)).thenReturn(42);
when(manager.get(43)).thenReturn(new Infestation(43, "rats", Severity.MILD));
when(manager.all()).thenReturn(List.of(new Infestation(43, "rats", Severity.MILD)));
try (var factions = mockStatic(net.tfminecraft.simplefactions.SimpleFactions.class)) {
factions.when(net.tfminecraft.simplefactions.SimpleFactions::getInstance).thenReturn(sf);
assertEquals(List.of("42"), complete(console, command, "set", "4"));
assertEquals(List.of("43"), complete(console, command, "clear", "4"));
assertEquals(List.of("42", "here"), complete(player, command, "set", ""));
assertEquals(List.of(), complete(player, command, "clear", "H"));
provinces.when(() -> Provinces.at(player)).thenReturn(43);
assertEquals(List.of("here"), complete(player, command, "clear", "H"));
provinces.when(() -> Provinces.at(player)).thenReturn(42);
}
groups.when(GroupLoader::groupIds).thenReturn(Set.of("rats", "swamp"));
groups.when(() -> GroupLoader.allowsTerrain("rats", 42)).thenReturn(true);
assertEquals(List.of("rats"), complete(player, command, "set", "HeRe", "R"));
assertEquals(List.of("rats"), complete(console, command, "set", "42", ""));
for (String invalid : List.of("here", "nonsense", "44", "43")) {
assertTrue(complete(console, command, "set", invalid, "").isEmpty());
}
verify(console, never()).sendMessage(anyString());
verify(player, never()).sendMessage(anyString());
}

@Test void setHereCompletionRequiresUninfestedLandAndStaysQuiet() {
admin();
when(player.hasPermission("infestations.admin")).thenReturn(true);
provinces.when(() -> Provinces.at(player)).thenReturn(42);
provinces.when(() -> Provinces.validLand(42)).thenReturn(true);
provinces.when(() -> Provinces.validLand(43)).thenReturn(true);
when(manager.get(43)).thenReturn(new Infestation(43, "rats", Severity.MILD));
try (var factions = mockStatic(net.tfminecraft.simplefactions.SimpleFactions.class)) {
assertEquals(List.of("here"), complete(player, command, "set", ""));
assertEquals(List.of("here"), complete(player, command, "SeT", "H"));
assertEquals(List.of("here"), complete(player, command, "set", "HeRe"));
assertEquals(List.of(), complete(player, command, "set", "hereX"));
assertEquals(List.of(), complete(console, command, "set", ""));
assertEquals(List.of(), complete(console, command, "set", "H"));
for (int ineligible : List.of(43, 44, 999, Provinces.UNKNOWN)) {
provinces.when(() -> Provinces.at(player)).thenReturn(ineligible);
assertEquals(List.of(), complete(player, command, "set", ""), "province " + ineligible);
assertEquals(List.of(), complete(player, command, "SET", "H"), "province " + ineligible);
}
}
verify(console, never()).sendMessage(anyString());
verify(player, never()).sendMessage(anyString());
messages.verifyNoInteractions();
verify(manager, never()).set(anyInt(), anyString(), any());
verify(manager, never()).clear(anyInt(), anyBoolean());
}

@Test void clearHereCompletionRequiresAnActiveInfestationOnLandAndStaysQuiet() {
admin();
when(player.hasPermission("infestations.admin")).thenReturn(true);
provinces.when(() -> Provinces.at(player)).thenReturn(42);
provinces.when(() -> Provinces.validLand(42)).thenReturn(true);
provinces.when(() -> Provinces.validLand(43)).thenReturn(true);
Infestation infestation = new Infestation(43, "rats", Severity.MILD);
when(manager.get(43)).thenReturn(infestation);
when(manager.get(44)).thenReturn(new Infestation(44, "rats", Severity.MILD));
when(manager.all()).thenReturn(List.of(infestation));
assertEquals(List.of("43"), complete(player, command, "clear", ""));
assertEquals(List.of(), complete(player, command, "clear", "H"));
provinces.when(() -> Provinces.at(player)).thenReturn(43);
assertEquals(List.of("43", "here"), complete(player, command, "clear", ""));
assertEquals(List.of("here"), complete(player, command, "ClEaR", "H"));
assertEquals(List.of("here"), complete(player, command, "clear", "HeRe"));
assertEquals(List.of(), complete(player, command, "clear", "hereX"));
assertEquals(List.of("43"), complete(player, command, "clear", "4"));
assertEquals(List.of("43"), complete(console, command, "clear", ""));
assertEquals(List.of(), complete(console, command, "clear", "H"));
for (int ineligible : List.of(44, 999, Provinces.UNKNOWN)) {
provinces.when(() -> Provinces.at(player)).thenReturn(ineligible);
assertEquals(List.of("43"), complete(player, command, "clear", ""), "province " + ineligible);
assertEquals(List.of(), complete(player, command, "CLEAR", "H"), "province " + ineligible);
}
verify(console, never()).sendMessage(anyString());
verify(player, never()).sendMessage(anyString());
messages.verifyNoInteractions();
verify(manager, never()).set(anyInt(), anyString(), any());
verify(manager, never()).clear(anyInt(), anyBoolean());
}

@Test void registeredCommandAllowsReloadOnlySenderAndStillDeniesAdminActions() throws Exception {
when(console.hasPermission("infestations.admin.reload")).thenReturn(true);
when(Infestations.plugin.isEnabled()).thenReturn(true);
var description = new org.bukkit.plugin.PluginDescriptionFile(
getClass().getResourceAsStream("/plugin.yml"));
assertEquals("infestations.admin;infestations.admin.reload",
description.getCommands().get("infestation").get("permission"));
var constructor = PluginCommand.class.getDeclaredConstructor(String.class, org.bukkit.plugin.Plugin.class);
constructor.setAccessible(true);
PluginCommand registered = constructor.newInstance("infestation", Infestations.plugin);
registered.setPermission((String) description.getCommands().get("infestation").get("permission"));
registered.setExecutor(commands);
registered.setTabCompleter(commands);
assertFalse(registered.testPermissionSilent(player));
when(player.hasPermission("infestations.admin")).thenReturn(true);
assertTrue(registered.testPermissionSilent(player));
when(player.hasPermission("infestations.admin")).thenReturn(false);
when(player.hasPermission("infestations.admin.reload")).thenReturn(true);
assertTrue(registered.testPermissionSilent(player));
assertTrue(registered.testPermissionSilent(console));
assertEquals(List.of("reload"), registered.tabComplete(console, "infestations", new String[]{""}));
assertEquals(List.of("reload"), registered.tabComplete(console, "infestations", new String[]{"RE"}));
assertEquals(List.of(), registered.tabComplete(console, "infestations", new String[]{"set", "H"}));
assertEquals(List.of(), registered.tabComplete(console, "infestations", new String[]{"clear", "H"}));
assertTrue(registered.execute(console, "infestation", new String[]{"reload"}));
verify(Infestations.plugin).reloadAll();
assertTrue(registered.execute(console, "infestation", new String[]{"set", "42", "rats", "mild"}));
verify(console).sendMessage("no-permission");
verify(manager, never()).set(anyInt(), anyString(), any());
}

private List<String> complete(CommandSender sender, Command cmd, String... args) {
return commands.onTabComplete(sender, cmd, cmd.getName(), args);
}
Expand Down
Loading