From 8d505196006599dc509e94da046d53f36492960a Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 24 Oct 2026 22:50:46 +0200 Subject: [PATCH 1/6] feat: staff admin commands for factions and guilds Adds /faction admin and /guild admin (simplefactions.admin) so staff can fix realms whose leader is away: info, who, rename, renamesettlement, movecapital + confirm, setcolour, setbanner, set culture/religion/ rulingsystem/rulertitle and kick for factions; info, rename, setcolour, setbanner and kick for guilds. They skip the leader check and any price and keep every other rule. Changes are logged as [StaffAdmin]. Co-Authored-By: Claude Opus 5.5 --- .../admin/StaffAdminCommand.java | 282 +++++++++++ .../admin/StaffAdminService.java | 329 +++++++++++++ .../economy/TreasuryTopology.java | 8 +- .../managers/CommandManager.java | 6 +- .../simplefactions/utils/TabCompletion.java | 7 +- .../admin/StaffAdminCommandTest.java | 330 +++++++++++++ .../admin/StaffAdminServiceTest.java | 448 ++++++++++++++++++ .../economy/TreasuryTopologyTest.java | 9 + .../managers/CommandManagerCoverageTest.java | 1 + .../managers/TabCompletionCoverageTest.java | 1 + 10 files changed, 1415 insertions(+), 6 deletions(-) create mode 100644 src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java create mode 100644 src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java new file mode 100644 index 00000000..29ab567a --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java @@ -0,0 +1,282 @@ +package net.tfminecraft.simplefactions.admin; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.function.LongSupplier; + +import org.bukkit.ChatColor; +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; + +import net.tfminecraft.simplefactions.Cache; +import net.tfminecraft.simplefactions.SimpleFactions; +import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.migration.QuarantineCommands; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.rest.RestServer; +import net.tfminecraft.simplefactions.settlement.Settlement; +import net.tfminecraft.simplefactions.utils.Permissions; + +/** + * {@code /faction admin ...} and {@code /guild admin ...}: staff-only fixes for + * realms and guilds whose leader is away or stuck. Everything that needs no + * location or held item also works from the console. Changes are logged. + */ +public final class StaffAdminCommand { + public static final String SUBCOMMAND = "admin"; + public static final List FACTION_SUBCOMMANDS = List.of("info", "who", "rename", "renamesettlement", + "movecapital", "confirm", "setcolour", "setbanner", "set", "kick", "quarantine"); + public static final List GUILD_SUBCOMMANDS = List.of("info", "rename", "setcolour", "setbanner", "kick"); + static final String[] FACTION_USAGE = { + "§6/faction admin info §7- leader, capital, settlements, guilds and bank", + "§6/faction admin who §7- which faction and guild a character is in", + "§6/faction admin rename §7- change the faction's name", + "§6/faction admin renamesettlement §7- rename a settlement", + "§6/faction admin movecapital [name...] §7- move the capital to the province you stand in", + "§6/faction admin confirm §7- apply the capital move you just previewed", + "§6/faction admin setcolour §7- change the map colour", + "§6/faction admin setbanner §7- use the banner in your hand", + "§6/faction admin set ", + "§6/faction admin kick §7- remove a member from the faction", + "§6/faction admin quarantine list|assign §7- unmatched character records", + }; + static final String[] GUILD_USAGE = { + "§6/guild admin info §7- leader, members, seat and bank", + "§6/guild admin rename §7- change the guild's name", + "§6/guild admin setcolour §7- change the map colour", + "§6/guild admin setbanner §7- use the banner in your hand", + "§6/guild admin kick §7- send a member back to the faction's main guild", + }; + static final String DENIED = "§a[SimpleFactions]§c You do not have access to this command"; + static final String IN_GAME = "§cOnly a player in game can do that"; + static final long CONFIRM_MILLIS = 60_000; + + private record Pending(StaffAdminService.CapitalMove move, long expires) {} + + private static final Map pending = new HashMap<>(); + static LongSupplier clock = System::currentTimeMillis; + + private StaffAdminCommand() {} + + /** Also covers {@code /faction admin quarantine}, which is passed to its own handler. */ + public static boolean matches(String command, String[] args) { + return (command.equalsIgnoreCase("faction") || command.equalsIgnoreCase("guild")) + && args.length >= 1 && args[0].equalsIgnoreCase(SUBCOMMAND); + } + + public static boolean handle(CommandSender sender, String command, String[] args) { + if (!Permissions.isAdmin(sender)) { + sender.sendMessage(DENIED); + return true; + } + boolean guild = command.equalsIgnoreCase("guild"); + if (args.length < 2) { + sender.sendMessage(guild ? GUILD_USAGE : FACTION_USAGE); + return true; + } + String sub = args[1].toLowerCase(); + if (guild) handleGuild(sender, sub, args); + else handleFaction(sender, sub, args); + return true; + } + + private static String rest(String[] args, int from) { + return String.join(" ", Arrays.copyOfRange(args, from, args.length)); + } + + private static String usage(String[] usage, List subs, String sub) { + return usage[subs.indexOf(sub)]; + } + + private static void send(CommandSender sender, List lines) { + for (String line : lines) sender.sendMessage(line); + } + + /** Sends the result and records the change in the server log. */ + private static void apply(CommandSender sender, String command, String[] args, List lines) { + send(sender, lines); + SimpleFactions.getInstance().getLogger().info("[StaffAdmin] " + sender.getName() + " ran /" + command + " " + rest(args, 0) + + ": " + ChatColor.stripColor(String.join(" | ", lines))); + } + + private static Faction faction(CommandSender sender, String id) { + Faction faction = FactionManager.getByString(id); + if (faction == null) sender.sendMessage("§cNo faction by the id " + id); + return faction; + } + + private static Guild guild(CommandSender sender, String id) { + Guild guild = FactionManager.getGuildByString(id); + if (guild == null) sender.sendMessage("§cNo guild by the id " + id); + return guild; + } + + private static Player player(CommandSender sender) { + if (sender instanceof Player player) return player; + sender.sendMessage(IN_GAME); + return null; + } + + /** Arguments a subcommand needs, counting {@code admin} and the subcommand itself. */ + static int minArgs(String sub) { + return switch (sub) { + case "confirm" -> 2; + case "info", "who", "movecapital", "setbanner" -> 3; + case "rename", "setcolour", "kick" -> 4; + default -> 5; + }; + } + + /** Whether the arguments fit {@code sub}; these three take nothing after their target. */ + static boolean fits(String sub, int length) { + boolean exact = sub.equals("confirm") || sub.equals("info") || sub.equals("setbanner"); + return exact ? length == minArgs(sub) : length >= minArgs(sub); + } + + private static void handleFaction(CommandSender sender, String sub, String[] args) { + if (sub.equals("quarantine")) { + QuarantineCommands.handle(sender, args); + return; + } + if (!FACTION_SUBCOMMANDS.contains(sub)) { + sender.sendMessage(FACTION_USAGE); + return; + } + if (!fits(sub, args.length)) { + sender.sendMessage(usage(FACTION_USAGE, FACTION_SUBCOMMANDS, sub)); + return; + } + if (sub.equals("who")) { + send(sender, StaffAdminService.who(rest(args, 2))); + return; + } + if (sub.equals("confirm")) { + confirm(sender, args); + return; + } + if ((sub.equals("movecapital") || sub.equals("renamesettlement")) && !Cache.requireProvinces(sender)) return; + Faction faction = faction(sender, args[2]); + if (faction == null) return; + switch (sub) { + case "info" -> send(sender, StaffAdminService.info(faction)); + case "rename" -> apply(sender, "faction", args, StaffAdminService.rename(faction, rest(args, 3))); + case "renamesettlement" -> apply(sender, "faction", args, StaffAdminService.renameSettlement(faction, args[3], rest(args, 4))); + case "movecapital" -> previewCapital(sender, faction, args); + case "setcolour" -> apply(sender, "faction", args, StaffAdminService.setColour(faction, String.join("", Arrays.copyOfRange(args, 3, args.length)))); + case "setbanner" -> { + Player player = player(sender); + if (player != null) apply(sender, "faction", args, StaffAdminService.setBanner(faction, player.getInventory().getItemInMainHand())); + } + case "set" -> apply(sender, "faction", args, StaffAdminService.set(faction, args[3], rest(args, 4))); + default -> apply(sender, "faction", args, StaffAdminService.kick(faction, rest(args, 3))); + } + } + + private static void previewCapital(CommandSender sender, Faction faction, String[] args) { + Player player = player(sender); + if (player == null) return; + String name = args.length > 3 ? rest(args, 3) : null; + List lines = new ArrayList<>(); + StaffAdminService.CapitalMove move = StaffAdminService.previewCapital(player, faction, RestServer.getProvince(player), name, lines); + send(sender, lines); + if (move == null) pending.remove(player.getUniqueId()); + else pending.put(player.getUniqueId(), new Pending(move, clock.getAsLong() + CONFIRM_MILLIS)); + } + + private static void confirm(CommandSender sender, String[] args) { + Player player = player(sender); + if (player == null) return; + Pending waiting = pending.remove(player.getUniqueId()); + if (waiting == null || waiting.expires() < clock.getAsLong()) { + sender.sendMessage("§cNothing to confirm. Run §e/faction admin movecapital §cagain."); + return; + } + if (!Cache.requireProvinces(sender)) return; + apply(sender, "faction", args, StaffAdminService.moveCapital(player, waiting.move(), RestServer.getProvince(player))); + } + + private static void handleGuild(CommandSender sender, String sub, String[] args) { + if (!GUILD_SUBCOMMANDS.contains(sub)) { + sender.sendMessage(GUILD_USAGE); + return; + } + if (!fits(sub, args.length)) { + sender.sendMessage(usage(GUILD_USAGE, GUILD_SUBCOMMANDS, sub)); + return; + } + Guild guild = guild(sender, args[2]); + if (guild == null) return; + switch (sub) { + case "info" -> send(sender, StaffAdminService.guildInfo(guild)); + case "rename" -> apply(sender, "guild", args, StaffAdminService.guildRename(guild, rest(args, 3))); + case "setcolour" -> apply(sender, "guild", args, StaffAdminService.guildSetColour(guild, String.join("", Arrays.copyOfRange(args, 3, args.length)))); + case "setbanner" -> { + Player player = player(sender); + if (player != null) apply(sender, "guild", args, StaffAdminService.guildSetBanner(guild, player.getInventory().getItemInMainHand())); + } + default -> apply(sender, "guild", args, StaffAdminService.guildKick(guild, rest(args, 3))); + } + } + + // ---- Tab completion ---- + + public static List complete(CommandSender sender, String command, String[] args) { + if (!Permissions.isAdmin(sender) || !matches(command, args) || args.length < 2) return List.of(); + boolean guild = command.equalsIgnoreCase("guild"); + if (args.length == 2) return filtered(new ArrayList<>(guild ? GUILD_SUBCOMMANDS : FACTION_SUBCOMMANDS), args[1]); + String sub = args[1].toLowerCase(); + if (!guild && sub.equals("quarantine")) return QuarantineCommands.complete(sender, args); + List subs = guild ? GUILD_SUBCOMMANDS : FACTION_SUBCOMMANDS; + if (!subs.contains(sub) || sub.equals("confirm") || sub.equals("who")) return List.of(); + if (args.length == 3) { + List ids = new ArrayList<>(); + if (guild) { + for (Guild each : FactionManager.getAllGuilds()) ids.add(each.getId()); + } else { + for (Faction each : FactionManager.getCopy()) ids.add(each.getId()); + } + return filtered(ids, args[2]); + } + String[] target = Arrays.copyOfRange(args, 2, args.length); + if (guild) { + Guild each = FactionManager.getGuildByString(args[2]); + if (each == null) return List.of(); + return switch (sub) { + case "setcolour" -> args.length == 4 ? filtered(new ArrayList<>(List.of(String.valueOf(each.getRGB()))), args[3]) : List.of(); + case "kick" -> Members.completions(Members.display(each.getMembers()), target); + default -> List.of(); + }; + } + Faction faction = FactionManager.getByString(args[2]); + if (faction == null) return List.of(); + return switch (sub) { + case "renamesettlement" -> { + if (args.length != 4) yield List.of(); + List ids = new ArrayList<>(List.of("capital")); + for (Settlement settlement : faction.getSettlementHandler().getAll()) ids.add(settlement.getId()); + yield filtered(ids, args[3]); + } + case "setcolour" -> args.length == 4 ? filtered(new ArrayList<>(List.of(String.valueOf(faction.getRGB()))), args[3]) : List.of(); + case "set" -> args.length == 4 ? filtered(new ArrayList<>(StaffAdminService.FIELDS), args[3]) : List.of(); + case "kick" -> Members.completions(Members.display(faction.getMembers()), target); + default -> List.of(); + }; + } + + private static List filtered(List values, String prefix) { + String lower = prefix.toLowerCase(); + values.removeIf(value -> value == null || !value.toLowerCase().startsWith(lower)); + return values; + } + + /** Test hook: forgets every previewed capital move. */ + static void clearPending() { + pending.clear(); + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java new file mode 100644 index 00000000..44271ecf --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java @@ -0,0 +1,329 @@ +package net.tfminecraft.simplefactions.admin; + +import java.util.ArrayList; +import java.util.List; + +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; + +import net.tfminecraft.simplefactions.economy.TreasuryActions; +import net.tfminecraft.simplefactions.economy.TreasuryTopology; +import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.objects.Bank; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.settlement.Settlement; +import net.tfminecraft.simplefactions.settlement.handler.CapitalResult; +import net.tfminecraft.simplefactions.settlement.handler.SettlementHandler; +import net.tfminecraft.simplefactions.utils.Formatter; +import net.tfminecraft.simplefactions.utils.RandomRGB; +import net.tfminecraft.tlibs.objects.api.subapi.StringFormatter; + +/** + * Staff versions of the leader-only faction and guild commands. Each skips the + * leader check and any price, and nothing else: the same naming, colour and + * membership rules apply. Each returns the lines to show the staff member. + */ +public final class StaffAdminService { + /** Faction fields staff set with {@code /faction admin set}. */ + public static final List FIELDS = List.of("culture", "religion", "rulingsystem", "rulertitle"); + + private StaffAdminService() {} + + /** A move checked by {@link #previewCapital}, applied by {@link #moveCapital} once staff confirm. */ + public record CapitalMove(String factionId, int fromCapital, int province, String name) {} + + /** Display name as the player commands store it: {@code _} for spaces, default colour, hex codes. */ + static String displayName(String input) { + return StringFormatter.formatHex(Formatter.formatName(input)); + } + + static boolean validName(String input) { + return !Formatter.formatId(input).isBlank(); + } + + /** Why {@code rgb} cannot be used, or null. {@code current} is the colour it replaces. */ + static String colourProblem(String rgb, String current) { + return switch (FactionManager.validateRGB(rgb)) { + case 1 -> "§cInvalid format. Use: R,G,B (e.g. 255,0,0)"; + case 2 -> "§cRGB values must be numbers (e.g. 128,128,128)"; + case 3 -> "§cEach RGB value must be between 0 and 255"; + case 4 -> "§c0,0,0 is reserved for unclaimed land; pick another colour"; + default -> !RandomRGB.sameRGB(rgb, current) && !RandomRGB.isFree(rgb) ? "§cThat colour is already used" : null; + }; + } + + static boolean isBanner(ItemStack item) { + return item != null && item.getType().toString().contains("BANNER"); + } + + // ---- Faction ---- + + public static List info(Faction faction) { + List lines = new ArrayList<>(); + lines.add("§6=== §f" + faction.getName() + " §7(" + faction.getId() + ") §6==="); + lines.add("§7Leader: §f" + character(faction.getLeader())); + lines.add("§7Members: §f" + faction.getMembers().size() + " §7Provinces: §f" + faction.getProvinces().size() + + " §7Colour: §f" + faction.getRGB()); + lines.add("§7Culture: §f" + orNone(faction.getCulture()) + " §7Religion: §f" + orNone(faction.getReligion()) + + " §7Ruling system: §f" + orNone(faction.getGovernmentString()) + + " §7Ruler title: §f" + orNone(faction.getRulerTitle())); + Settlement capital = faction.hasCapital() ? faction.getSettlementHandler().getByProvince(faction.getCapital()) : null; + lines.add("§7Capital: §f" + (!faction.hasCapital() ? "none" + : "province " + faction.getCapital() + (capital == null ? "" : ", " + settlement(capital))) + + " §7Capital changes made: §f" + faction.getCapitalMoves()); + for (Settlement other : faction.getSettlementHandler().getAll()) { + if (other != capital) lines.add("§7Settlement: §f" + settlement(other) + " §7in province §f" + other.getCenterProvince()); + } + for (Guild guild : faction.getGuildHandler().getGuilds()) { + if (guild.isBase()) continue; + lines.add("§7Guild: §f" + guild.getName() + " §7(" + guild.getId() + ") led by §f" + character(guild.getLeader()) + + " §7with §f" + guild.getMembers().size() + " §7members"); + } + Bank bank = faction.getBank(); + lines.add("§7Bank: §f" + (bank == null ? "none" : Formatter.formatMoney(bank.getWealth()) + "d")); + return lines; + } + + private static String character(String key) { + return key == null || key.isEmpty() ? "nobody" : Members.display(key) + " §7(" + key + ")§f"; + } + + private static String orNone(String value) { + return value == null || value.isBlank() ? "none" : value; + } + + private static String settlement(Settlement settlement) { + return settlement.getName() + " §7(" + settlement.getId() + ")§f"; + } + + /** Where a character belongs, searched by name or character ID across every realm. */ + public static List who(String input) { + List lines = new ArrayList<>(); + for (Faction faction : FactionManager.getCopy()) { + String key = Members.find(faction.getMembers(), input); + if (key == null) continue; + StringBuilder line = new StringBuilder("§f" + character(key) + " §7is in §f" + faction.getName() + + " §7(" + faction.getId() + ")"); + if (key.equals(faction.getLeader())) line.append(" §7and leads it"); + for (Guild guild : faction.getGuildHandler().getGuilds()) { + if (guild.isBase() || !guild.isMember(key)) continue; + line.append("§7, guild §f").append(guild.getName()).append(" §7(").append(guild.getId()).append(")"); + if (guild.isLeader(key)) line.append(" §7as its leader"); + } + lines.add(line.toString()); + } + if (lines.isEmpty()) lines.add("§cNo faction member matches " + input); + return lines; + } + + public static List rename(Faction faction, String input) { + if (!validName(input)) return List.of("§cInvalid faction name"); + String old = faction.getName(); + faction.setName(displayName(input)); + FactionManager.getMap().enqueue("nation", faction.getRGB()); + return List.of("§aRenamed faction §f" + old + " §ato §f" + faction.getName()); + } + + /** {@code which} is a settlement id, or {@code capital} for the faction capital. */ + public static List renameSettlement(Faction faction, String which, String input) { + SettlementHandler settlements = faction.getSettlementHandler(); + Settlement settlement = null; + if (which.equalsIgnoreCase("capital")) { + if (faction.hasCapital()) settlement = settlements.getByProvince(faction.getCapital()); + } else { + // Ids keep the founder's capitals, so staff need not match them. + String id = Formatter.formatId(which); + for (Settlement each : settlements.getAll()) { + if (each.getId().equalsIgnoreCase(id)) settlement = each; + } + } + if (settlement == null) { + return List.of("§c" + faction.getName() + " §chas no settlement " + which + + "§c. §e/faction admin info " + faction.getId() + " §clists them."); + } + CapitalResult result = settlements.rename(settlement.getCenterProvince(), input, false); + return List.of(result.getMessage()); + } + + /** + * Checks moving {@code faction}'s capital to {@code province}, where the staff + * member stands. Returns the move to confirm, or null with the refusal in {@code lines}. + */ + public static CapitalMove previewCapital(Player staff, Faction faction, int province, String name, List lines) { + if (province == -2) { + lines.add("§a[SimpleFactions] §cError! could not resolve province"); + return null; + } + if (!faction.ownsProvince(province)) { + lines.add("§c" + faction.getName() + " §cdoes not own province " + province + + "§c. Give it to them with §e/faction forceclaim " + faction.getId() + " §cfirst."); + return null; + } + if (faction.hasCapital() && faction.getCapital() == province) { + lines.add("§cProvince " + province + " §cis already the capital of " + faction.getName() + + "§c. Rename it with §e/faction admin renamesettlement " + faction.getId() + " capital "); + return null; + } + SettlementHandler settlements = faction.getSettlementHandler(); + Settlement existing = settlements.getByProvince(province); + // Also refuses a missing or invalid name when a new settlement would be founded. + CapitalResult check = settlements.validateFactionCapital(staff, province, name); + if (!check.isSuccess()) { + lines.add(check.getMessage()); + return null; + } + lines.add("§eMove the capital of " + faction.getName() + " §efrom " + + (faction.hasCapital() ? "province " + faction.getCapital() : "nowhere") + " §eto province " + province + + (existing != null ? " §e(" + existing.getName() + "§e)" : " §e(new settlement " + displayName(name) + "§e)")); + List lost = faction.hasCapital() + ? faction.getProvinceHandler().previewProvincesLostIfCapitalMoved(province) : List.of(); + if (!lost.isEmpty()) lines.add("§c" + faction.getName() + " §cwould lose provinces cut off from the new capital: " + lost); + lines.add("§7It is free and does not use up their free capital change. Run §e/faction admin confirm §7to apply it."); + return new CapitalMove(faction.getId(), faction.getCapital(), province, existing != null ? null : name); + } + + /** Applies a confirmed move if nothing changed since the preview. */ + public static List moveCapital(Player staff, CapitalMove move, int standing) { + Faction faction = FactionManager.getByString(move.factionId()); + if (faction == null) return List.of("§cThat faction no longer exists. Nothing changed."); + if (faction.getCapital() != move.fromCapital()) return List.of("§cTheir capital changed since the preview. Nothing changed."); + if (standing != move.province()) return List.of("§cStand in province " + move.province() + " §cto confirm. Nothing changed."); + if (!faction.hasCapital()) { + // A first capital is founded in place, as for a new faction. + CapitalResult result = faction.getSettlementHandler().applyFactionCapital(staff, move.province(), move.name()); + if (!result.isSuccess()) return List.of(result.getMessage()); + faction.setCapital(move.province()); + faction.getProvinceHandler().revalidateClaims(); + return List.of(result.getMessage(), "§aSet the capital of " + faction.getName() + " §ato province " + move.province()); + } + List before = new ArrayList<>(faction.getProvinces()); + if (!TreasuryTopology.capital(staff, faction, move.province(), move.name(), false, 0, TreasuryActions.newId(), false)) { + return List.of("§cThe capital move failed. Nothing changed."); + } + before.removeAll(faction.getProvinces()); + List lines = new ArrayList<>(); + lines.add("§aMoved the capital of " + faction.getName() + " §ato province " + move.province()); + if (!before.isEmpty()) lines.add("§e" + faction.getName() + " §elost provinces cut off from their realm: " + before); + return lines; + } + + public static List setColour(Faction faction, String rgb) { + String old = faction.getRGB(); + String problem = colourProblem(rgb, old); + if (problem != null) return List.of(problem); + FactionManager.getMap().enqueue("nation", old); + faction.setRGB(rgb); + FactionManager.getMap().enqueue("nation", faction.getRGB()); + return List.of("§aColour of " + faction.getName() + " §aset to §f" + rgb); + } + + public static List setBanner(Faction faction, ItemStack held) { + if (!isBanner(held)) return List.of("§cHold the new banner in your main hand"); + faction.setBanner(held); + return List.of("§aBanner of " + faction.getName() + " §achanged"); + } + + public static List set(Faction faction, String field, String input) { + String value = input.replace("_", " "); + switch (field.toLowerCase()) { + case "culture" -> faction.setCulture(value); + case "religion" -> faction.setReligion(value); + case "rulingsystem" -> faction.setGovernment(value); + case "rulertitle" -> faction.setRulerTitle(value); + default -> { + return List.of("§cUnknown field " + field + "§c. Use one of: " + String.join(", ", FIELDS)); + } + } + return List.of("§aSet " + field.toLowerCase() + " of " + faction.getName() + " §ato §f" + value); + } + + /** Removes a character from the faction and any guild in it; leaders must be replaced first. */ + public static List kick(Faction faction, String input) { + String key = Members.find(faction.getMembers(), input); + if (key == null) return List.of("§cNo member of " + faction.getName() + " §cmatches " + input); + if (key.equals(faction.getLeader())) { + return List.of("§c" + Members.display(key) + " §cleads " + faction.getName() + + "§c. Replace them with §e/faction forceleader §cfirst."); + } + for (Guild guild : faction.getGuildHandler().getGuilds()) { + if (!guild.isBase() && guild.isLeader(key)) { + return List.of("§c" + Members.display(key) + " §cleads the guild " + guild.getName() + + "§c. Replace them with §e/guild forceleader §cor delete the guild first."); + } + } + faction.forceRemoveMember(key); + faction.updatePrestige(); + Members.online(key).ifPresent(kicked -> kicked.sendMessage("§cStaff removed you from " + faction.getName())); + return List.of("§aRemoved " + Members.display(key) + " §afrom " + faction.getName()); + } + + // ---- Guild ---- + + static String baseGuild(Guild guild, String instead) { + return guild.isBase() ? "§c" + guild.getId() + " §cis the main guild of " + guild.getFaction().getName() + + "§c; use §e/faction admin " + instead + " " + guild.getFaction().getId() + " §cinstead" : null; + } + + public static List guildInfo(Guild guild) { + List lines = new ArrayList<>(); + Faction faction = guild.getFaction(); + lines.add("§6=== §f" + guild.getName() + " §7(" + guild.getId() + ") §6==="); + lines.add("§7Faction: §f" + faction.getName() + " §7(" + faction.getId() + ")" + (guild.isBase() ? " §7- its main guild" : "")); + lines.add("§7Leader: §f" + character(guild.getLeader())); + List names = new ArrayList<>(); + for (String member : guild.getMembers()) names.add(Members.display(member)); + lines.add("§7Members (" + names.size() + "): §f" + String.join(", ", names)); + Settlement seat = guild.hasCapital() ? faction.getSettlementHandler().getByProvince(guild.getCapital()) : null; + lines.add("§7Seat: §f" + (!guild.hasCapital() ? "none" + : "province " + guild.getCapital() + (seat == null ? "" : ", " + settlement(seat))) + " §7Colour: §f" + guild.getRGB()); + Bank bank = guild.getBank(); + lines.add("§7Bank: §f" + (bank == null ? "none" : Formatter.formatMoney(bank.getWealth()) + "d")); + return lines; + } + + public static List guildRename(Guild guild, String input) { + String base = baseGuild(guild, "rename"); + if (base != null) return List.of(base); + if (!validName(input)) return List.of("§cInvalid guild name"); + String old = guild.getName(); + guild.setName(displayName(input)); + return List.of("§aRenamed guild §f" + old + " §ato §f" + guild.getName()); + } + + public static List guildSetColour(Guild guild, String rgb) { + String base = baseGuild(guild, "setcolour"); + if (base != null) return List.of(base); + String problem = colourProblem(rgb, guild.getRGB()); + if (problem != null) return List.of(problem); + guild.setRGB(rgb); + return List.of("§aColour of " + guild.getName() + " §aset to §f" + rgb); + } + + public static List guildSetBanner(Guild guild, ItemStack held) { + String base = baseGuild(guild, "setbanner"); + if (base != null) return List.of(base); + if (!isBanner(held)) return List.of("§cHold the new banner in your main hand"); + guild.setBanner(held); + return List.of("§aBanner of " + guild.getName() + " §achanged"); + } + + /** Sends a member back to the faction's main guild, as a guild leader's kick does. */ + public static List guildKick(Guild guild, String input) { + String base = baseGuild(guild, "kick"); + if (base != null) return List.of(base); + String key = Members.find(guild.getMembers(), input); + if (key == null) return List.of("§cNo member of " + guild.getName() + " §cmatches " + input); + if (guild.isLeader(key)) { + return List.of("§c" + Members.display(key) + " §cleads " + guild.getName() + + "§c. Replace them with §e/guild forceleader §cfirst."); + } + guild.kick(key); + guild.getFaction().addMember(key); + Members.online(key).ifPresent(kicked -> kicked.sendMessage("§cStaff removed you from " + guild.getName() + + "§c. You are back in the main guild of " + guild.getFaction().getName())); + return List.of("§aMoved " + Members.display(key) + " §afrom " + guild.getName() + " §ato the main guild of " + + guild.getFaction().getName()); + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/economy/TreasuryTopology.java b/src/main/java/net/tfminecraft/simplefactions/economy/TreasuryTopology.java index 89431bd8..4fa6bcb2 100644 --- a/src/main/java/net/tfminecraft/simplefactions/economy/TreasuryTopology.java +++ b/src/main/java/net/tfminecraft/simplefactions/economy/TreasuryTopology.java @@ -25,6 +25,12 @@ private TreasuryTopology() {} public static boolean capital(Player actor, Faction faction, int province, String name, boolean rename, double cost, String operationId) { + return capital(actor, faction, province, name, rename, cost, operationId, true); + } + + /** {@code countMove} false leaves the faction's free capital change unspent, for staff moves. */ + public static boolean capital(Player actor, Faction faction, int province, String name, + boolean rename, double cost, String operationId, boolean countMove) { Boolean prior = TreasuryActions.resume(operationId, actor); if (prior != null) return prior; var result = rename ? faction.getSettlementHandler().rename(province, name, true) @@ -41,7 +47,7 @@ public static boolean capital(Player actor, Faction faction, int province, Strin depart(after, state, old); revalidate(after, faction); } - state.addProperty("capital moves", faction.getCapitalMoves() + 1); + state.addProperty("capital moves", faction.getCapitalMoves() + (countMove ? 1 : 0)); return commit(actor, SettlementGuilds.capital(faction), cost, operationId, "Capital change", before, after); } diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java index ba486f40..d60a8277 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java @@ -1,5 +1,6 @@ package net.tfminecraft.simplefactions.managers; +import net.tfminecraft.simplefactions.admin.StaffAdminCommand; import net.tfminecraft.simplefactions.map.AdminLandService; import net.tfminecraft.simplefactions.identity.Members; import net.tfminecraft.simplefactions.economy.TreasuryActions; @@ -85,8 +86,9 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, return net.tfminecraft.simplefactions.guild.network.TradeGraphCommand.handle(sender, args); if (cmd.getName().equalsIgnoreCase(cmd1) && args.length >= 1 && args[0].equalsIgnoreCase("reloadespionage")) return EspionageCommands.reload(sender, args); - if(cmd.getName().equalsIgnoreCase(cmd1) && net.tfminecraft.simplefactions.migration.QuarantineCommands.matches(args)) { - return net.tfminecraft.simplefactions.migration.QuarantineCommands.handle(sender, args); + // Staff fixes (and the quarantine) also work from the console. + if(StaffAdminCommand.matches(cmd.getName(), args)) { + return StaffAdminCommand.handle(sender, cmd.getName(), args); } if(cmd.getName().equalsIgnoreCase(cmd1) && args.length >= 1 && args[0].equalsIgnoreCase("addguildbank")) { return addGuildBank(sender, args); diff --git a/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java b/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java index 16bc2eb4..c79e877e 100644 --- a/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java +++ b/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java @@ -1,7 +1,7 @@ package net.tfminecraft.simplefactions.utils; import net.tfminecraft.simplefactions.identity.Members; -import net.tfminecraft.simplefactions.migration.QuarantineCommands; +import net.tfminecraft.simplefactions.admin.StaffAdminCommand; import net.tfminecraft.simplefactions.espionage.EspionageCommands; @@ -209,8 +209,8 @@ public List onTabComplete (CommandSender sender, Command cmd, String lab } private List complete(CommandSender sender, Command cmd, String[] args) { - if(cmd.getName().equalsIgnoreCase("faction") && args.length >= 2 && args[0].equalsIgnoreCase("admin")) { - return QuarantineCommands.complete(sender, args); + if(args.length >= 2 && StaffAdminCommand.matches(cmd.getName(), args)) { + return StaffAdminCommand.complete(sender, cmd.getName(), args); } if(cmd.getName().equalsIgnoreCase("faction") && args.length < 2 && !(sender instanceof Player)) { List completions = new ArrayList<>(); @@ -287,6 +287,7 @@ else if(cmd.getName().equalsIgnoreCase("guild") && args.length >= 0 && args.leng if(!LeaderlessGuilds.appointable(Members.key(p)).isEmpty()) completions.add("appoint"); if(LeaderlessGuilds.offeredTo(Members.key(p)) != null) completions.add("lead"); if(Permissions.isAdmin(sender)) { + completions.add(StaffAdminCommand.SUBCOMMAND); completions.add("dummify"); completions.add("dummyleader"); completions.add("forceleader"); diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java new file mode 100644 index 00000000..bcffa2fc --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java @@ -0,0 +1,330 @@ +package net.tfminecraft.simplefactions.admin; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.UUID; +import java.util.logging.Logger; + +import org.bukkit.command.CommandSender; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import net.tfminecraft.simplefactions.Cache; +import net.tfminecraft.simplefactions.SimpleFactions; +import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.migration.QuarantineCommands; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.rest.RestServer; +import net.tfminecraft.simplefactions.settlement.Settlement; +import net.tfminecraft.simplefactions.settlement.handler.SettlementHandler; +import net.tfminecraft.simplefactions.utils.Permissions; + +class StaffAdminCommandTest { + private MockedStatic service; + private MockedStatic factions; + private MockedStatic plugin; + private MockedStatic rest; + private MockedStatic quarantine; + private MockedStatic members; + private boolean provinces; + private CommandSender console; + private Player staff; + private ItemStack held; + private Faction faction; + private Guild guild; + private Logger logger; + private long now; + + @BeforeEach + void setUp() { + provinces = Cache.provincesEnabled; + Cache.provincesEnabled = true; + service = mockStatic(StaffAdminService.class, + call -> call.getMethod().getReturnType() == List.class ? List.of("ok") : null); + factions = mockStatic(FactionManager.class); + plugin = mockStatic(SimpleFactions.class); + rest = mockStatic(RestServer.class); + quarantine = mockStatic(QuarantineCommands.class); + members = mockStatic(Members.class, CALLS_REAL_METHODS); + members.when(() -> Members.display(anyString())).thenAnswer(call -> call.getArgument(0)); + SimpleFactions instance = mock(SimpleFactions.class); + logger = mock(Logger.class); + when(instance.getLogger()).thenReturn(logger); + plugin.when(SimpleFactions::getInstance).thenReturn(instance); + console = mock(CommandSender.class); + when(console.getName()).thenReturn("CONSOLE"); + when(console.hasPermission(Permissions.Permission_Admin)).thenReturn(true); + staff = mock(Player.class); + when(staff.getName()).thenReturn("Fran"); + when(staff.getUniqueId()).thenReturn(UUID.randomUUID()); + when(staff.hasPermission(Permissions.Permission_Admin)).thenReturn(true); + PlayerInventory inventory = mock(PlayerInventory.class); + held = mock(ItemStack.class); + when(inventory.getItemInMainHand()).thenReturn(held); + when(staff.getInventory()).thenReturn(inventory); + faction = mock(Faction.class); + when(faction.getId()).thenReturn("sunsora"); + when(faction.getRGB()).thenReturn("1,2,3"); + when(faction.getMembers()).thenReturn(List.of("Bob Smith", "Carl")); + guild = mock(Guild.class); + when(guild.getId()).thenReturn("smiths"); + when(guild.getRGB()).thenReturn("4,5,6"); + when(guild.getMembers()).thenReturn(List.of("Bob Smith")); + factions.when(() -> FactionManager.getByString("sunsora")).thenReturn(faction); + factions.when(() -> FactionManager.getGuildByString("smiths")).thenReturn(guild); + factions.when(FactionManager::getCopy).thenReturn(List.of(faction)); + factions.when(FactionManager::getAllGuilds).thenReturn(List.of(guild)); + now = 1_000; + StaffAdminCommand.clock = () -> now; + StaffAdminCommand.clearPending(); + } + + @AfterEach + void tearDown() { + StaffAdminCommand.clock = System::currentTimeMillis; + StaffAdminCommand.clearPending(); + members.close(); + quarantine.close(); + rest.close(); + plugin.close(); + factions.close(); + service.close(); + Cache.provincesEnabled = provinces; + } + + private boolean run(CommandSender sender, String command, String... args) { + return StaffAdminCommand.handle(sender, command, args); + } + + @Test + void matchesOnlyTheAdminSubcommandOfFactionAndGuild() { + assertTrue(StaffAdminCommand.matches("faction", new String[] {"ADMIN"})); + assertTrue(StaffAdminCommand.matches("Guild", new String[] {"admin", "info"})); + assertFalse(StaffAdminCommand.matches("company", new String[] {"admin"})); + assertFalse(StaffAdminCommand.matches("faction", new String[] {})); + assertFalse(StaffAdminCommand.matches("faction", new String[] {"title"})); + } + + @Test + void argumentCountsFitEachSubcommand() { + assertTrue(StaffAdminCommand.fits("confirm", 2)); + assertFalse(StaffAdminCommand.fits("confirm", 3)); + assertTrue(StaffAdminCommand.fits("info", 3)); + assertFalse(StaffAdminCommand.fits("info", 4)); + assertTrue(StaffAdminCommand.fits("movecapital", 5)); + assertFalse(StaffAdminCommand.fits("kick", 3)); + assertTrue(StaffAdminCommand.fits("set", 6)); + assertFalse(StaffAdminCommand.fits("renamesettlement", 4)); + } + + @Test + void nonStaffAreRefusedAndBareCommandsShowUsage() { + CommandSender player = mock(CommandSender.class); + assertTrue(run(player, "faction", "admin", "info", "sunsora")); + verify(player).sendMessage(StaffAdminCommand.DENIED); + service.verifyNoInteractions(); + run(console, "faction", "admin"); + verify(console).sendMessage(StaffAdminCommand.FACTION_USAGE); + run(console, "guild", "admin"); + verify(console).sendMessage(StaffAdminCommand.GUILD_USAGE); + run(console, "faction", "admin", "nonsense"); + verify(console, org.mockito.Mockito.times(2)).sendMessage(StaffAdminCommand.FACTION_USAGE); + run(console, "guild", "admin", "nonsense"); + verify(console, org.mockito.Mockito.times(2)).sendMessage(StaffAdminCommand.GUILD_USAGE); + } + + @Test + void wrongArgumentsShowThatSubcommandsUsage() { + run(console, "faction", "admin", "rename", "sunsora"); + verify(console).sendMessage(StaffAdminCommand.FACTION_USAGE[2]); + run(console, "faction", "admin", "info", "sunsora", "extra"); + verify(console).sendMessage(StaffAdminCommand.FACTION_USAGE[0]); + run(console, "guild", "admin", "kick", "smiths"); + verify(console).sendMessage(StaffAdminCommand.GUILD_USAGE[4]); + run(console, "faction", "admin", "info", "missing"); + verify(console).sendMessage("§cNo faction by the id missing"); + run(console, "guild", "admin", "info", "missing"); + verify(console).sendMessage("§cNo guild by the id missing"); + service.verifyNoInteractions(); + } + + @Test + void quarantineKeepsItsOwnHandler() { + String[] args = {"admin", "quarantine", "list"}; + assertTrue(StaffAdminCommand.handle(console, "faction", args)); + quarantine.verify(() -> QuarantineCommands.handle(console, args)); + } + + @Test + void factionCommandsRouteToTheServiceAndLogChanges() { + run(console, "faction", "admin", "info", "sunsora"); + service.verify(() -> StaffAdminService.info(faction)); + run(console, "faction", "admin", "who", "Bob", "Smith"); + service.verify(() -> StaffAdminService.who("Bob Smith")); + run(console, "faction", "admin", "rename", "sunsora", "Sunsora", "Empire"); + service.verify(() -> StaffAdminService.rename(faction, "Sunsora Empire")); + run(console, "faction", "admin", "renamesettlement", "sunsora", "capital", "Sunsora", "Castle"); + service.verify(() -> StaffAdminService.renameSettlement(faction, "capital", "Sunsora Castle")); + run(console, "faction", "admin", "setcolour", "sunsora", "1,", "2,", "3"); + service.verify(() -> StaffAdminService.setColour(faction, "1,2,3")); + run(console, "faction", "admin", "set", "sunsora", "culture", "High", "Elven"); + service.verify(() -> StaffAdminService.set(faction, "culture", "High Elven")); + run(console, "faction", "admin", "kick", "sunsora", "Bob", "Smith"); + service.verify(() -> StaffAdminService.kick(faction, "Bob Smith")); + verify(console, org.mockito.Mockito.times(7)).sendMessage("ok"); + verify(logger).info("[StaffAdmin] CONSOLE ran /faction admin kick sunsora Bob Smith: ok"); + verify(logger, never()).info("[StaffAdmin] CONSOLE ran /faction admin info sunsora: ok"); + } + + @Test + void bannersAndCapitalMovesNeedAPlayerInGame() { + run(console, "faction", "admin", "setbanner", "sunsora"); + run(console, "guild", "admin", "setbanner", "smiths"); + run(console, "faction", "admin", "movecapital", "sunsora"); + run(console, "faction", "admin", "confirm"); + verify(console, org.mockito.Mockito.times(4)).sendMessage(StaffAdminCommand.IN_GAME); + run(staff, "faction", "admin", "setbanner", "sunsora"); + service.verify(() -> StaffAdminService.setBanner(faction, held)); + run(staff, "guild", "admin", "setbanner", "smiths"); + service.verify(() -> StaffAdminService.guildSetBanner(guild, held)); + } + + @Test + void settlementCommandsNeedProvinces() { + Cache.provincesEnabled = false; + run(console, "faction", "admin", "renamesettlement", "sunsora", "capital", "X"); + run(staff, "faction", "admin", "movecapital", "sunsora"); + verify(console).sendMessage(Cache.PROVINCES_DISABLED_MESSAGE); + verify(staff).sendMessage(Cache.PROVINCES_DISABLED_MESSAGE); + service.verifyNoInteractions(); + } + + @Test + void capitalMovesArePreviewedThenConfirmedOnce() { + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", 1, 5, "Sunsora_Castle"); + rest.when(() -> RestServer.getProvince(staff)).thenReturn(5); + service.when(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), eq("Sunsora Castle"), any())) + .thenAnswer(call -> { + call.>getArgument(4).add("preview"); + return move; + }); + run(staff, "faction", "admin", "movecapital", "sunsora", "Sunsora", "Castle"); + verify(staff).sendMessage("preview"); + service.when(() -> StaffAdminService.moveCapital(staff, move, 5)).thenReturn(List.of("§aMoved")); + run(staff, "faction", "admin", "confirm"); + verify(staff).sendMessage("§aMoved"); + verify(logger).info("[StaffAdmin] Fran ran /faction admin confirm: Moved"); + run(staff, "faction", "admin", "confirm"); + verify(staff).sendMessage("§cNothing to confirm. Run §e/faction admin movecapital §cagain."); + service.verify(() -> StaffAdminService.moveCapital(staff, move, 5)); + } + + @Test + void aRefusedOrStalePreviewCannotBeConfirmed() { + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", 1, 5, null); + rest.when(() -> RestServer.getProvince(staff)).thenReturn(5); + service.when(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), any(), any())).thenReturn(move); + run(staff, "faction", "admin", "movecapital", "sunsora"); + service.verify(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), eq(null), any())); + // A later refused preview drops the earlier one. + service.when(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), any(), any())).thenReturn(null); + run(staff, "faction", "admin", "movecapital", "sunsora"); + run(staff, "faction", "admin", "confirm"); + service.when(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), any(), any())).thenReturn(move); + run(staff, "faction", "admin", "movecapital", "sunsora"); + now += StaffAdminCommand.CONFIRM_MILLIS + 1; + run(staff, "faction", "admin", "confirm"); + verify(staff, org.mockito.Mockito.times(2)).sendMessage("§cNothing to confirm. Run §e/faction admin movecapital §cagain."); + // Provinces turned off between preview and confirm. + now = 1_000; + run(staff, "faction", "admin", "movecapital", "sunsora"); + Cache.provincesEnabled = false; + run(staff, "faction", "admin", "confirm"); + service.verify(() -> StaffAdminService.moveCapital(any(), any(), anyInt()), never()); + } + + @Test + void guildCommandsRouteToTheService() { + run(console, "guild", "admin", "info", "smiths"); + service.verify(() -> StaffAdminService.guildInfo(guild)); + run(console, "guild", "admin", "rename", "smiths", "Iron", "Smiths"); + service.verify(() -> StaffAdminService.guildRename(guild, "Iron Smiths")); + run(console, "guild", "admin", "setcolour", "smiths", "7,8,9"); + service.verify(() -> StaffAdminService.guildSetColour(guild, "7,8,9")); + run(console, "guild", "admin", "kick", "smiths", "Bob", "Smith"); + service.verify(() -> StaffAdminService.guildKick(guild, "Bob Smith")); + } + + @Test + void completionIsForStaffOnly() { + CommandSender player = mock(CommandSender.class); + assertEquals(List.of(), StaffAdminCommand.complete(player, "faction", new String[] {"admin", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"title", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin"})); + } + + @Test + void completionOffersSubcommandsTargetsAndArguments() { + assertEquals(List.of("rename", "renamesettlement"), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "ren"})); + assertEquals(List.of("kick"), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "K"})); + quarantine.when(() -> QuarantineCommands.complete(eq(console), any())).thenReturn(List.of("list")); + assertEquals(List.of("list"), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "quarantine", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "bogus", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "confirm", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "who", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "quarantine", ""})); + assertEquals(List.of("sunsora"), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "info", "S"})); + assertEquals(List.of("smiths"), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "info", ""})); + } + + @Test + void guildCompletionUsesTheChosenGuild() { + assertEquals(List.of(), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "kick", "missing", ""})); + assertEquals(List.of("4,5,6"), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "setcolour", "smiths", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "setcolour", "smiths", "4,5,6", ""})); + assertEquals(List.of("Smith"), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "kick", "smiths", "Bob", "S"})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "guild", new String[] {"admin", "rename", "smiths", ""})); + } + + @Test + void factionCompletionUsesTheChosenFaction() { + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "kick", "missing", ""})); + SettlementHandler settlements = mock(SettlementHandler.class); + Settlement tower = mock(Settlement.class); + when(tower.getId()).thenReturn("Research_Tower"); + when(settlements.getAll()).thenReturn(List.of(tower)); + when(faction.getSettlementHandler()).thenReturn(settlements); + assertEquals(List.of("capital", "Research_Tower"), + StaffAdminCommand.complete(console, "faction", new String[] {"admin", "renamesettlement", "sunsora", ""})); + assertEquals(List.of("Research_Tower"), + StaffAdminCommand.complete(console, "faction", new String[] {"admin", "renamesettlement", "sunsora", "re"})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "renamesettlement", "sunsora", "x", ""})); + assertEquals(List.of("1,2,3"), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "setcolour", "sunsora", ""})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "setcolour", "sunsora", "1", ""})); + assertEquals(List.of("rulingsystem", "rulertitle"), + StaffAdminCommand.complete(console, "faction", new String[] {"admin", "set", "sunsora", "rul"})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "set", "sunsora", "culture", ""})); + assertEquals(List.of("Carl"), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "kick", "sunsora", "c"})); + assertEquals(List.of(), StaffAdminCommand.complete(console, "faction", new String[] {"admin", "info", "sunsora", ""})); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java new file mode 100644 index 00000000..042b300b --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java @@ -0,0 +1,448 @@ +package net.tfminecraft.simplefactions.admin; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; + +import org.bukkit.Material; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +import net.tfminecraft.simplefactions.economy.TreasuryTopology; +import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.map.MapSystem; +import net.tfminecraft.simplefactions.objects.Bank; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.objects.handler.GuildHandler; +import net.tfminecraft.simplefactions.objects.handler.ProvinceHandler; +import net.tfminecraft.simplefactions.settlement.Settlement; +import net.tfminecraft.simplefactions.settlement.handler.CapitalResult; +import net.tfminecraft.simplefactions.settlement.handler.SettlementHandler; +import net.tfminecraft.simplefactions.utils.RandomRGB; + +class StaffAdminServiceTest { + private MockedStatic members; + private MockedStatic factions; + private MockedStatic colours; + private Faction faction; + private GuildHandler guilds; + private Guild base; + private Guild smiths; + private SettlementHandler settlements; + private ProvinceHandler provinces; + private MapSystem map; + private Player staff; + + @BeforeEach + void setUp() { + members = mockStatic(Members.class, CALLS_REAL_METHODS); + factions = mockStatic(FactionManager.class, CALLS_REAL_METHODS); + colours = mockStatic(RandomRGB.class); + members.when(() -> Members.display(anyString())).thenAnswer(call -> call.getArgument(0)); + members.when(() -> Members.online(anyString())).thenReturn(Optional.empty()); + faction = mock(Faction.class); + when(faction.getId()).thenReturn("sunsora"); + when(faction.getName()).thenReturn("Sunsora"); + when(faction.getLeader()).thenReturn("Verena"); + when(faction.getRGB()).thenReturn("10,20,30"); + when(faction.getMembers()).thenReturn(List.of("Verena", "Bob", "Carl")); + guilds = mock(GuildHandler.class); + when(faction.getGuildHandler()).thenReturn(guilds); + base = mock(Guild.class); + when(base.isBase()).thenReturn(true); + when(base.getId()).thenReturn("sunsora"); + when(base.getFaction()).thenReturn(faction); + smiths = mock(Guild.class); + when(smiths.getId()).thenReturn("smiths"); + when(smiths.getName()).thenReturn("Smiths"); + when(smiths.getFaction()).thenReturn(faction); + when(smiths.getLeader()).thenReturn("Carl"); + when(smiths.isLeader("Carl")).thenReturn(true); + when(smiths.getMembers()).thenReturn(List.of("Carl", "Bob")); + when(smiths.isMember("Carl")).thenReturn(true); + when(smiths.isMember("Bob")).thenReturn(true); + when(guilds.getGuilds()).thenReturn(List.of(base, smiths)); + settlements = mock(SettlementHandler.class); + when(faction.getSettlementHandler()).thenReturn(settlements); + provinces = mock(ProvinceHandler.class); + when(faction.getProvinceHandler()).thenReturn(provinces); + map = mock(MapSystem.class); + factions.when(FactionManager::getMap).thenReturn(map); + factions.when(() -> FactionManager.getByString("sunsora")).thenReturn(faction); + factions.when(FactionManager::getCopy).thenReturn(List.of(faction)); + colours.when(() -> RandomRGB.isFree(anyString())).thenReturn(true); + staff = mock(Player.class); + } + + @AfterEach + void tearDown() { + colours.close(); + factions.close(); + members.close(); + } + + private static Settlement settlement(String id, String name, int province) { + Settlement settlement = mock(Settlement.class); + when(settlement.getId()).thenReturn(id); + when(settlement.getName()).thenReturn(name); + when(settlement.getCenterProvince()).thenReturn(province); + return settlement; + } + + private static ItemStack item(Material type) { + ItemStack item = mock(ItemStack.class); + when(item.getType()).thenReturn(type); + return item; + } + + @Test + void namesFollowThePlayerCommandsFormat() { + assertTrue(StaffAdminService.displayName("Sunsora_Castle").endsWith("Sunsora Castle")); + assertTrue(StaffAdminService.validName("Sunsora Castle")); + assertEquals(false, StaffAdminService.validName("!!!")); + } + + @Test + void coloursAreCheckedLikeThePlayerCommands() { + assertEquals("§cInvalid format. Use: R,G,B (e.g. 255,0,0)", StaffAdminService.colourProblem("1,2", "1,1,1")); + assertEquals("§cRGB values must be numbers (e.g. 128,128,128)", StaffAdminService.colourProblem("a,b,c", "1,1,1")); + assertEquals("§cEach RGB value must be between 0 and 255", StaffAdminService.colourProblem("1,2,300", "1,1,1")); + assertEquals("§c0,0,0 is reserved for unclaimed land; pick another colour", StaffAdminService.colourProblem("0,0,0", "1,1,1")); + colours.when(() -> RandomRGB.sameRGB("5,5,5", "1,1,1")).thenReturn(false); + colours.when(() -> RandomRGB.isFree("5,5,5")).thenReturn(false); + assertEquals("§cThat colour is already used", StaffAdminService.colourProblem("5,5,5", "1,1,1")); + colours.when(() -> RandomRGB.sameRGB("5,5,5", "5,5,5")).thenReturn(true); + assertNull(StaffAdminService.colourProblem("5,5,5", "5,5,5")); + colours.when(() -> RandomRGB.isFree("6,6,6")).thenReturn(true); + assertNull(StaffAdminService.colourProblem("6,6,6", "1,1,1")); + } + + @Test + void onlyBannersCount() { + assertEquals(false, StaffAdminService.isBanner(null)); + assertEquals(false, StaffAdminService.isBanner(item(Material.STONE))); + assertTrue(StaffAdminService.isBanner(item(Material.RED_BANNER))); + } + + @Test + void infoShowsCapitalSettlementsGuildsAndBank() { + Settlement capital = settlement("research_tower", "Research Tower", 395); + Settlement port = settlement("port", "Port", 393); + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(395); + when(faction.getCapitalMoves()).thenReturn(2); + when(faction.getProvinces()).thenReturn(List.of(395, 393)); + when(faction.getCulture()).thenReturn("Elven"); + when(settlements.getByProvince(395)).thenReturn(capital); + when(settlements.getAll()).thenReturn(List.of(capital, port)); + Bank bank = mock(Bank.class); + when(bank.getWealth()).thenReturn(12.5); + when(faction.getBank()).thenReturn(bank); + List lines = StaffAdminService.info(faction); + assertEquals("§6=== §fSunsora §7(sunsora) §6===", lines.get(0)); + assertEquals("§7Leader: §fVerena §7(Verena)§f", lines.get(1)); + assertEquals("§7Members: §f3 §7Provinces: §f2 §7Colour: §f10,20,30", lines.get(2)); + assertEquals("§7Culture: §fElven §7Religion: §fnone §7Ruling system: §fnone §7Ruler title: §fnone", lines.get(3)); + assertEquals("§7Capital: §fprovince 395, Research Tower §7(research_tower)§f §7Capital changes made: §f2", lines.get(4)); + assertEquals("§7Settlement: §fPort §7(port)§f §7in province §f393", lines.get(5)); + assertEquals("§7Guild: §fSmiths §7(smiths) led by §fCarl §7(Carl)§f §7with §f2 §7members", lines.get(6)); + assertEquals("§7Bank: §f12.50d", lines.get(7)); + } + + @Test + void infoCopesWithNoCapitalNoLeaderAndNoBank() { + when(faction.getLeader()).thenReturn(""); + when(faction.getReligion()).thenReturn(" "); + when(faction.getProvinces()).thenReturn(List.of()); + when(settlements.getAll()).thenReturn(List.of()); + List lines = StaffAdminService.info(faction); + assertEquals("§7Leader: §fnobody", lines.get(1)); + assertEquals("§7Capital: §fnone §7Capital changes made: §f0", lines.get(4)); + assertEquals("§7Bank: §fnone", lines.get(lines.size() - 1)); + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(7); + when(faction.getLeader()).thenReturn(null); + lines = StaffAdminService.info(faction); + assertEquals("§7Leader: §fnobody", lines.get(1)); + assertEquals("§7Capital: §fprovince 7 §7Capital changes made: §f0", lines.get(4)); + } + + @Test + void whoFindsTheRealmAndGuildOfACharacter() { + members.when(() -> Members.find(List.of("Verena", "Bob", "Carl"), "carl")).thenReturn("Carl"); + assertEquals(List.of("§fCarl §7(Carl)§f §7is in §fSunsora §7(sunsora)§7, guild §fSmiths §7(smiths) §7as its leader"), + StaffAdminService.who("carl")); + members.when(() -> Members.find(List.of("Verena", "Bob", "Carl"), "bob")).thenReturn("Bob"); + assertEquals(List.of("§fBob §7(Bob)§f §7is in §fSunsora §7(sunsora)§7, guild §fSmiths §7(smiths)"), + StaffAdminService.who("bob")); + members.when(() -> Members.find(List.of("Verena", "Bob", "Carl"), "verena")).thenReturn("Verena"); + assertEquals(List.of("§fVerena §7(Verena)§f §7is in §fSunsora §7(sunsora) §7and leads it"), + StaffAdminService.who("verena")); + members.when(() -> Members.find(List.of("Verena", "Bob", "Carl"), "nobody")).thenReturn(null); + assertEquals(List.of("§cNo faction member matches nobody"), StaffAdminService.who("nobody")); + } + + @Test + void renameKeepsTheIdAndRedrawsTheMap() { + assertEquals(List.of("§cInvalid faction name"), StaffAdminService.rename(faction, "???")); + verify(faction, never()).setName(anyString()); + when(faction.getName()).thenReturn("Sunsora", "New Sunsora"); + assertEquals(List.of("§aRenamed faction §fSunsora §ato §fNew Sunsora"), StaffAdminService.rename(faction, "New_Sunsora")); + verify(faction).setName(StaffAdminService.displayName("New_Sunsora")); + verify(map).enqueue("nation", "10,20,30"); + } + + @Test + void renameSettlementFindsTheCapitalOrAnId() { + assertEquals(List.of("§cSunsora §chas no settlement capital§c. §e/faction admin info sunsora §clists them."), + StaffAdminService.renameSettlement(faction, "capital", "Sunsora Castle")); + Settlement capital = settlement("research_tower", "Research Tower", 395); + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(395); + when(settlements.getByProvince(395)).thenReturn(capital); + when(settlements.rename(395, "Sunsora Castle", false)).thenReturn(CapitalResult.ok("§aRenamed")); + assertEquals(List.of("§aRenamed"), StaffAdminService.renameSettlement(faction, "CAPITAL", "Sunsora Castle")); + Settlement port = settlement("Port", "Port", 393); + when(settlements.getAll()).thenReturn(List.of(port, capital)); + assertEquals(List.of("§aRenamed"), StaffAdminService.renameSettlement(faction, "Research_Tower", "Sunsora Castle")); + verify(settlements, never()).rename(393, "Sunsora Castle", false); + when(settlements.getAll()).thenReturn(List.of(capital)); + assertEquals(List.of("§cSunsora §chas no settlement port§c. §e/faction admin info sunsora §clists them."), + StaffAdminService.renameSettlement(faction, "port", "Harbour")); + } + + @Test + void capitalPreviewRefusesBadProvinces() { + List lines = new ArrayList<>(); + assertNull(StaffAdminService.previewCapital(staff, faction, -2, null, lines)); + assertEquals(List.of("§a[SimpleFactions] §cError! could not resolve province"), lines); + lines.clear(); + assertNull(StaffAdminService.previewCapital(staff, faction, 9, null, lines)); + assertEquals(List.of("§cSunsora §cdoes not own province 9§c. Give it to them with §e/faction forceclaim sunsora §cfirst."), lines); + lines.clear(); + when(faction.ownsProvince(395)).thenReturn(true); + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(395); + assertNull(StaffAdminService.previewCapital(staff, faction, 395, null, lines)); + assertEquals(List.of("§cProvince 395 §cis already the capital of Sunsora§c. Rename it with §e/faction admin renamesettlement sunsora capital "), lines); + lines.clear(); + when(faction.ownsProvince(393)).thenReturn(true); + when(settlements.validateFactionCapital(staff, 393, null)).thenReturn(CapitalResult.fail("§cName required")); + assertNull(StaffAdminService.previewCapital(staff, faction, 393, null, lines)); + assertEquals(List.of("§cName required"), lines); + } + + @Test + void capitalPreviewDescribesTheMoveAndAnyLostLand() { + List lines = new ArrayList<>(); + when(faction.ownsProvince(393)).thenReturn(true); + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(395); + when(settlements.validateFactionCapital(staff, 393, "Sunsora_Castle")).thenReturn(CapitalResult.ok("ok")); + when(provinces.previewProvincesLostIfCapitalMoved(393)).thenReturn(List.of(12)); + StaffAdminService.CapitalMove move = StaffAdminService.previewCapital(staff, faction, 393, "Sunsora_Castle", lines); + assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 393, "Sunsora_Castle"), move); + assertEquals("§eMove the capital of Sunsora §efrom province 395 §eto province 393 §e(new settlement " + + StaffAdminService.displayName("Sunsora_Castle") + "§e)", lines.get(0)); + assertEquals("§cSunsora §cwould lose provinces cut off from the new capital: [12]", lines.get(1)); + assertEquals(3, lines.size()); + + lines.clear(); + Settlement port = settlement("port", "Port", 393); + when(settlements.getByProvince(393)).thenReturn(port); + when(settlements.validateFactionCapital(staff, 393, null)).thenReturn(CapitalResult.ok("ok")); + when(provinces.previewProvincesLostIfCapitalMoved(393)).thenReturn(List.of()); + assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 393, null), + StaffAdminService.previewCapital(staff, faction, 393, null, lines)); + assertEquals("§eMove the capital of Sunsora §efrom province 395 §eto province 393 §e(Port§e)", lines.get(0)); + assertEquals(2, lines.size()); + + lines.clear(); + when(faction.hasCapital()).thenReturn(false); + when(faction.getCapital()).thenReturn(-1); + when(settlements.validateFactionCapital(staff, 393, "Ignored")).thenReturn(CapitalResult.ok("ok")); + assertEquals(new StaffAdminService.CapitalMove("sunsora", -1, 393, null), + StaffAdminService.previewCapital(staff, faction, 393, "Ignored", lines)); + assertEquals("§eMove the capital of Sunsora §efrom nowhere §eto province 393 §e(Port§e)", lines.get(0)); + } + + @Test + void capitalMoveChecksNothingChangedSincePreview() { + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("gone", 395, 393, null); + assertEquals(List.of("§cThat faction no longer exists. Nothing changed."), StaffAdminService.moveCapital(staff, move, 393)); + move = new StaffAdminService.CapitalMove("sunsora", 395, 393, null); + when(faction.getCapital()).thenReturn(1); + assertEquals(List.of("§cTheir capital changed since the preview. Nothing changed."), StaffAdminService.moveCapital(staff, move, 393)); + when(faction.getCapital()).thenReturn(395); + assertEquals(List.of("§cStand in province 393 §cto confirm. Nothing changed."), StaffAdminService.moveCapital(staff, move, 1)); + } + + @Test + void aFirstCapitalIsFoundedInPlace() { + when(faction.getCapital()).thenReturn(-1); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", -1, 393, "Sunsora_Castle"); + when(settlements.applyFactionCapital(staff, 393, "Sunsora_Castle")).thenReturn(CapitalResult.fail("§cNo")); + assertEquals(List.of("§cNo"), StaffAdminService.moveCapital(staff, move, 393)); + verify(faction, never()).setCapital(393); + when(settlements.applyFactionCapital(staff, 393, "Sunsora_Castle")).thenReturn(CapitalResult.ok("§aFounded")); + assertEquals(List.of("§aFounded", "§aSet the capital of Sunsora §ato province 393"), StaffAdminService.moveCapital(staff, move, 393)); + verify(faction).setCapital(393); + verify(provinces).revalidateClaims(); + } + + @Test + void anExistingCapitalMovesFreeThroughTheTreasuryWithoutCountingAMove() { + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(395); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", 395, 393, null); + try (MockedStatic topology = mockStatic(TreasuryTopology.class)) { + topology.when(() -> TreasuryTopology.capital(eq(staff), eq(faction), eq(393), isNull(), eq(false), eq(0.0), + anyString(), eq(false))).thenReturn(false); + when(faction.getProvinces()).thenReturn(List.of(395, 393, 12)); + assertEquals(List.of("§cThe capital move failed. Nothing changed."), StaffAdminService.moveCapital(staff, move, 393)); + topology.when(() -> TreasuryTopology.capital(eq(staff), eq(faction), eq(393), isNull(), eq(false), eq(0.0), + anyString(), eq(false))).thenReturn(true); + when(faction.getProvinces()).thenReturn(List.of(395, 393, 12), List.of(395, 393)); + assertEquals(List.of("§aMoved the capital of Sunsora §ato province 393", + "§eSunsora §elost provinces cut off from their realm: [12]"), StaffAdminService.moveCapital(staff, move, 393)); + when(faction.getProvinces()).thenReturn(List.of(395, 393)); + assertEquals(List.of("§aMoved the capital of Sunsora §ato province 393"), StaffAdminService.moveCapital(staff, move, 393)); + } + } + + @Test + void colourBannerAndDetailsChange() { + assertEquals(List.of("§cInvalid format. Use: R,G,B (e.g. 255,0,0)"), StaffAdminService.setColour(faction, "x")); + verify(faction, never()).setRGB(anyString()); + assertEquals(List.of("§aColour of Sunsora §aset to §f1,2,3"), StaffAdminService.setColour(faction, "1,2,3")); + verify(faction).setRGB("1,2,3"); + verify(map, org.mockito.Mockito.times(2)).enqueue("nation", "10,20,30"); + + assertEquals(List.of("§cHold the new banner in your main hand"), StaffAdminService.setBanner(faction, item(Material.STONE))); + ItemStack banner = item(Material.BLUE_BANNER); + assertEquals(List.of("§aBanner of Sunsora §achanged"), StaffAdminService.setBanner(faction, banner)); + verify(faction).setBanner(banner); + + assertEquals(List.of("§aSet culture of Sunsora §ato §fHigh Elven"), StaffAdminService.set(faction, "Culture", "High_Elven")); + verify(faction).setCulture("High Elven"); + StaffAdminService.set(faction, "religion", "Sun"); + verify(faction).setReligion("Sun"); + StaffAdminService.set(faction, "rulingsystem", "Monarchy"); + verify(faction).setGovernment("Monarchy"); + StaffAdminService.set(faction, "rulertitle", "Queen"); + verify(faction).setRulerTitle("Queen"); + assertEquals(List.of("§cUnknown field motto§c. Use one of: culture, religion, rulingsystem, rulertitle"), + StaffAdminService.set(faction, "motto", "x")); + } + + @Test + void kickRemovesMembersButNotLeaders() { + members.when(() -> Members.find(faction.getMembers(), "x")).thenReturn(null); + assertEquals(List.of("§cNo member of Sunsora §cmatches x"), StaffAdminService.kick(faction, "x")); + members.when(() -> Members.find(faction.getMembers(), "verena")).thenReturn("Verena"); + assertEquals(List.of("§cVerena §cleads Sunsora§c. Replace them with §e/faction forceleader §cfirst."), + StaffAdminService.kick(faction, "verena")); + members.when(() -> Members.find(faction.getMembers(), "carl")).thenReturn("Carl"); + assertEquals(List.of("§cCarl §cleads the guild Smiths§c. Replace them with §e/guild forceleader §cor delete the guild first."), + StaffAdminService.kick(faction, "carl")); + verify(faction, never()).forceRemoveMember(anyString()); + members.when(() -> Members.find(faction.getMembers(), "bob")).thenReturn("Bob"); + Player bob = mock(Player.class); + members.when(() -> Members.online("Bob")).thenReturn(Optional.of(bob)); + assertEquals(List.of("§aRemoved Bob §afrom Sunsora"), StaffAdminService.kick(faction, "bob")); + verify(faction).forceRemoveMember("Bob"); + verify(faction).updatePrestige(); + verify(bob).sendMessage("§cStaff removed you from Sunsora"); + } + + @Test + void guildInfoShowsMembersSeatAndBank() { + when(smiths.getRGB()).thenReturn("4,5,6"); + List lines = StaffAdminService.guildInfo(smiths); + assertEquals("§6=== §fSmiths §7(smiths) §6===", lines.get(0)); + assertEquals("§7Faction: §fSunsora §7(sunsora)", lines.get(1)); + assertEquals("§7Leader: §fCarl §7(Carl)§f", lines.get(2)); + assertEquals("§7Members (2): §fCarl, Bob", lines.get(3)); + assertEquals("§7Seat: §fnone §7Colour: §f4,5,6", lines.get(4)); + assertEquals("§7Bank: §fnone", lines.get(5)); + when(smiths.hasCapital()).thenReturn(true); + when(smiths.getCapital()).thenReturn(12); + Bank bank = mock(Bank.class); + when(bank.getWealth()).thenReturn(3.0); + when(smiths.getBank()).thenReturn(bank); + lines = StaffAdminService.guildInfo(smiths); + assertEquals("§7Seat: §fprovince 12 §7Colour: §f4,5,6", lines.get(4)); + assertEquals("§7Bank: §f3.00d", lines.get(5)); + Settlement forge = settlement("forge", "Forge", 12); + when(settlements.getByProvince(12)).thenReturn(forge); + assertEquals("§7Seat: §fprovince 12, Forge §7(forge)§f §7Colour: §f4,5,6", StaffAdminService.guildInfo(smiths).get(4)); + when(base.getName()).thenReturn("Sunsora"); + when(base.getMembers()).thenReturn(List.of()); + assertEquals("§7Faction: §fSunsora §7(sunsora) §7- its main guild", StaffAdminService.guildInfo(base).get(1)); + } + + @Test + void guildEditsRefuseTheMainGuild() { + String refusal = "§csunsora §cis the main guild of Sunsora§c; use §e/faction admin %s sunsora §cinstead"; + assertNull(StaffAdminService.baseGuild(smiths, "rename")); + assertEquals(List.of(String.format(refusal, "rename")), StaffAdminService.guildRename(base, "X")); + assertEquals(List.of(String.format(refusal, "setcolour")), StaffAdminService.guildSetColour(base, "1,2,3")); + assertEquals(List.of(String.format(refusal, "setbanner")), StaffAdminService.guildSetBanner(base, item(Material.RED_BANNER))); + assertEquals(List.of(String.format(refusal, "kick")), StaffAdminService.guildKick(base, "Bob")); + } + + @Test + void guildRenameColourAndBannerChange() { + assertEquals(List.of("§cInvalid guild name"), StaffAdminService.guildRename(smiths, "!")); + when(smiths.getName()).thenReturn("Smiths", "Iron Smiths"); + assertEquals(List.of("§aRenamed guild §fSmiths §ato §fIron Smiths"), StaffAdminService.guildRename(smiths, "Iron_Smiths")); + verify(smiths).setName(StaffAdminService.displayName("Iron_Smiths")); + + when(smiths.getName()).thenReturn("Smiths"); + assertEquals(List.of("§cEach RGB value must be between 0 and 255"), StaffAdminService.guildSetColour(smiths, "1,2,999")); + assertEquals(List.of("§aColour of Smiths §aset to §f1,2,3"), StaffAdminService.guildSetColour(smiths, "1,2,3")); + verify(smiths).setRGB("1,2,3"); + + assertEquals(List.of("§cHold the new banner in your main hand"), StaffAdminService.guildSetBanner(smiths, null)); + ItemStack banner = item(Material.RED_BANNER); + assertEquals(List.of("§aBanner of Smiths §achanged"), StaffAdminService.guildSetBanner(smiths, banner)); + verify(smiths).setBanner(banner); + } + + @Test + void guildKickSendsMembersBackToTheMainGuild() { + members.when(() -> Members.find(smiths.getMembers(), "x")).thenReturn(null); + assertEquals(List.of("§cNo member of Smiths §cmatches x"), StaffAdminService.guildKick(smiths, "x")); + members.when(() -> Members.find(smiths.getMembers(), "carl")).thenReturn("Carl"); + assertEquals(List.of("§cCarl §cleads Smiths§c. Replace them with §e/guild forceleader §cfirst."), + StaffAdminService.guildKick(smiths, "carl")); + verify(smiths, never()).kick(anyString()); + members.when(() -> Members.find(smiths.getMembers(), "bob")).thenReturn("Bob"); + assertEquals(List.of("§aMoved Bob §afrom Smiths §ato the main guild of Sunsora"), StaffAdminService.guildKick(smiths, "bob")); + verify(smiths).kick("Bob"); + verify(faction).addMember("Bob"); + Player bob = mock(Player.class); + members.when(() -> Members.online("Bob")).thenReturn(Optional.of(bob)); + StaffAdminService.guildKick(smiths, "bob"); + verify(bob).sendMessage("§cStaff removed you from Smiths§c. You are back in the main guild of Sunsora"); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/economy/TreasuryTopologyTest.java b/src/test/java/net/tfminecraft/simplefactions/economy/TreasuryTopologyTest.java index d2acb9df..cfc0c89b 100644 --- a/src/test/java/net/tfminecraft/simplefactions/economy/TreasuryTopologyTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/economy/TreasuryTopologyTest.java @@ -174,6 +174,15 @@ void capitalLossRevalidatesTitlesUsingTheProjectedRealmAndSubjects(boolean subje assertNotNull(origin.getSettlementHandler().getByProvince(4)); assertEquals(80,origin.getBank().getWealth()); } + @Test void aStaffMoveIsFreeAndLeavesTheFreeCapitalChangeUnspent() { + origin.addProvince(4); + origin.setCapitalMoves(0); + assertTrue(TreasuryTopology.capital(domain.player("Alice"),origin,4,"Staff_Capital",false,0,TreasuryActions.newId(),false)); + assertEquals(4,origin.getCapital()); + assertNotNull(origin.getSettlementHandler().getByProvince(4)); + assertEquals(0,origin.getCapitalMoves()); + assertEquals(100,origin.getBank().getWealth()); + } @Test void anUnregisteredGuildCannotPublishARelocationIntent() { origin.getGuildHandler().restoreTopology(List.of(origin.getOrCreateMainGuild())); assertThrows(IllegalStateException.class,()->move(TreasuryActions.newId())); diff --git a/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java index 15ae09f0..db1cbd6d 100644 --- a/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java @@ -1952,6 +1952,7 @@ void invitingAPlayerWithoutACharacterIsRefused(String name) { @Test void quarantineCommandsAreRoutedBeforeAnyPlayerCheck() { CommandSender console = mock(CommandSender.class); + when(console.hasPermission(Permissions.Permission_Admin)).thenReturn(true); try (MockedStatic quarantine = mockStatic(net.tfminecraft.simplefactions.migration.QuarantineCommands.class, CALLS_REAL_METHODS)) { quarantine diff --git a/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java index b5303a2a..3b872a5a 100644 --- a/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java @@ -214,6 +214,7 @@ void guildRootSuggestionsAddLeaderAndStaffActionsWithoutProvinceOnlyActionsWhenD "rename", "setbanner", "setcolour", + "admin", "dummify", "dummyleader"))); assertFalse(leader.contains("setcapital")); From f962909ab2562f25d75c83882750be0b474d1aac Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:03:32 +0200 Subject: [PATCH 2/6] fix: staff capital repair, character check and banner items - Staff can found the settlement again in a capital province that lost it, as /faction setcapital does for leaders. - A capital move through the treasury needs the staff member to be playing a character; say so instead of throwing on confirm. - Only *_BANNER items count as banners (not banner patterns). - /faction admin set refuses a value. Co-Authored-By: Claude Opus 5.5 --- .../admin/StaffAdminService.java | 35 +++++++++++++------ .../admin/StaffAdminServiceTest.java | 32 +++++++++++++++++ 2 files changed, 57 insertions(+), 10 deletions(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java index 44271ecf..e67ff2f2 100644 --- a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java @@ -28,6 +28,7 @@ public final class StaffAdminService { /** Faction fields staff set with {@code /faction admin set}. */ public static final List FIELDS = List.of("culture", "religion", "rulingsystem", "rulertitle"); + static final String NEEDS_CHARACTER = "§cPlay a character to move a capital: the treasury records who made the change."; private StaffAdminService() {} @@ -55,7 +56,8 @@ static String colourProblem(String rgb, String current) { } static boolean isBanner(ItemStack item) { - return item != null && item.getType().toString().contains("BANNER"); + // Not *_BANNER_PATTERN, which has no banner meta. + return item != null && item.getType().toString().endsWith("_BANNER"); } // ---- Faction ---- @@ -161,23 +163,33 @@ public static CapitalMove previewCapital(Player staff, Faction faction, int prov + "§c. Give it to them with §e/faction forceclaim " + faction.getId() + " §cfirst."); return null; } - if (faction.hasCapital() && faction.getCapital() == province) { + SettlementHandler settlements = faction.getSettlementHandler(); + Settlement existing = settlements.getByProvince(province); + boolean inCapital = faction.hasCapital() && faction.getCapital() == province; + if (inCapital && existing != null) { lines.add("§cProvince " + province + " §cis already the capital of " + faction.getName() + "§c. Rename it with §e/faction admin renamesettlement " + faction.getId() + " capital "); return null; } - SettlementHandler settlements = faction.getSettlementHandler(); - Settlement existing = settlements.getByProvince(province); + if (Members.session(staff).isEmpty()) { + lines.add(NEEDS_CHARACTER); + return null; + } // Also refuses a missing or invalid name when a new settlement would be founded. CapitalResult check = settlements.validateFactionCapital(staff, province, name); if (!check.isSuccess()) { lines.add(check.getMessage()); return null; } - lines.add("§eMove the capital of " + faction.getName() + " §efrom " - + (faction.hasCapital() ? "province " + faction.getCapital() : "nowhere") + " §eto province " + province - + (existing != null ? " §e(" + existing.getName() + "§e)" : " §e(new settlement " + displayName(name) + "§e)")); - List lost = faction.hasCapital() + if (inCapital) { + // A capital whose settlement is gone: found it again in place, as /faction setcapital does. + lines.add("§eFound " + displayName(name) + " §ein the capital of " + faction.getName() + " §e(province " + province + ")"); + } else { + lines.add("§eMove the capital of " + faction.getName() + " §efrom " + + (faction.hasCapital() ? "province " + faction.getCapital() : "nowhere") + " §eto province " + province + + (existing != null ? " §e(" + existing.getName() + "§e)" : " §e(new settlement " + displayName(name) + "§e)")); + } + List lost = faction.hasCapital() && !inCapital ? faction.getProvinceHandler().previewProvincesLostIfCapitalMoved(province) : List.of(); if (!lost.isEmpty()) lines.add("§c" + faction.getName() + " §cwould lose provinces cut off from the new capital: " + lost); lines.add("§7It is free and does not use up their free capital change. Run §e/faction admin confirm §7to apply it."); @@ -190,14 +202,16 @@ public static List moveCapital(Player staff, CapitalMove move, int stand if (faction == null) return List.of("§cThat faction no longer exists. Nothing changed."); if (faction.getCapital() != move.fromCapital()) return List.of("§cTheir capital changed since the preview. Nothing changed."); if (standing != move.province()) return List.of("§cStand in province " + move.province() + " §cto confirm. Nothing changed."); - if (!faction.hasCapital()) { - // A first capital is founded in place, as for a new faction. + if (!faction.hasCapital() || faction.getCapital() == move.province()) { + // A first capital, or a capital without its settlement, is founded in place. CapitalResult result = faction.getSettlementHandler().applyFactionCapital(staff, move.province(), move.name()); if (!result.isSuccess()) return List.of(result.getMessage()); faction.setCapital(move.province()); faction.getProvinceHandler().revalidateClaims(); return List.of(result.getMessage(), "§aSet the capital of " + faction.getName() + " §ato province " + move.province()); } + // The treasury journal records the character that made the change. + if (Members.session(staff).isEmpty()) return List.of(NEEDS_CHARACTER); List before = new ArrayList<>(faction.getProvinces()); if (!TreasuryTopology.capital(staff, faction, move.province(), move.name(), false, 0, TreasuryActions.newId(), false)) { return List.of("§cThe capital move failed. Nothing changed."); @@ -227,6 +241,7 @@ public static List setBanner(Faction faction, ItemStack held) { public static List set(Faction faction, String field, String input) { String value = input.replace("_", " "); + if (value.startsWith("<") && value.endsWith(">")) return List.of("§cEnter a value instead of " + value + "§c."); switch (field.toLowerCase()) { case "culture" -> faction.setCulture(value); case "religion" -> faction.setReligion(value); diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java index 042b300b..f6e0b1c7 100644 --- a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java @@ -38,6 +38,7 @@ import net.tfminecraft.simplefactions.settlement.handler.CapitalResult; import net.tfminecraft.simplefactions.settlement.handler.SettlementHandler; import net.tfminecraft.simplefactions.utils.RandomRGB; +import net.tfminecraft.tlibs.character.CharacterSession; class StaffAdminServiceTest { private MockedStatic members; @@ -91,6 +92,7 @@ void setUp() { factions.when(FactionManager::getCopy).thenReturn(List.of(faction)); colours.when(() -> RandomRGB.isFree(anyString())).thenReturn(true); staff = mock(Player.class); + members.when(() -> Members.session(staff)).thenReturn(Optional.of(mock(CharacterSession.class))); } @AfterEach @@ -140,6 +142,7 @@ void coloursAreCheckedLikeThePlayerCommands() { void onlyBannersCount() { assertEquals(false, StaffAdminService.isBanner(null)); assertEquals(false, StaffAdminService.isBanner(item(Material.STONE))); + assertEquals(false, StaffAdminService.isBanner(item(Material.FLOWER_BANNER_PATTERN))); assertTrue(StaffAdminService.isBanner(item(Material.RED_BANNER))); } @@ -242,6 +245,8 @@ void capitalPreviewRefusesBadProvinces() { when(faction.ownsProvince(395)).thenReturn(true); when(faction.hasCapital()).thenReturn(true); when(faction.getCapital()).thenReturn(395); + Settlement capital = settlement("tower", "Tower", 395); + when(settlements.getByProvince(395)).thenReturn(capital); assertNull(StaffAdminService.previewCapital(staff, faction, 395, null, lines)); assertEquals(List.of("§cProvince 395 §cis already the capital of Sunsora§c. Rename it with §e/faction admin renamesettlement sunsora capital "), lines); lines.clear(); @@ -249,6 +254,27 @@ void capitalPreviewRefusesBadProvinces() { when(settlements.validateFactionCapital(staff, 393, null)).thenReturn(CapitalResult.fail("§cName required")); assertNull(StaffAdminService.previewCapital(staff, faction, 393, null, lines)); assertEquals(List.of("§cName required"), lines); + lines.clear(); + members.when(() -> Members.session(staff)).thenReturn(Optional.empty()); + assertNull(StaffAdminService.previewCapital(staff, faction, 393, null, lines)); + assertEquals(List.of(StaffAdminService.NEEDS_CHARACTER), lines); + } + + @Test + void aCapitalWithoutItsSettlementIsFoundedAgainInPlace() { + List lines = new ArrayList<>(); + when(faction.ownsProvince(395)).thenReturn(true); + when(faction.hasCapital()).thenReturn(true); + when(faction.getCapital()).thenReturn(395); + when(settlements.validateFactionCapital(staff, 395, "Sunsora_Castle")).thenReturn(CapitalResult.ok("ok")); + StaffAdminService.CapitalMove move = StaffAdminService.previewCapital(staff, faction, 395, "Sunsora_Castle", lines); + assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 395, "Sunsora_Castle"), move); + assertEquals("§eFound " + StaffAdminService.displayName("Sunsora_Castle") + " §ein the capital of Sunsora §e(province 395)", lines.get(0)); + assertEquals(2, lines.size()); + verify(provinces, never()).previewProvincesLostIfCapitalMoved(395); + when(settlements.applyFactionCapital(staff, 395, "Sunsora_Castle")).thenReturn(CapitalResult.ok("§aFounded")); + assertEquals(List.of("§aFounded", "§aSet the capital of Sunsora §ato province 395"), StaffAdminService.moveCapital(staff, move, 395)); + verify(faction).setCapital(395); } @Test @@ -326,6 +352,11 @@ void anExistingCapitalMovesFreeThroughTheTreasuryWithoutCountingAMove() { "§eSunsora §elost provinces cut off from their realm: [12]"), StaffAdminService.moveCapital(staff, move, 393)); when(faction.getProvinces()).thenReturn(List.of(395, 393)); assertEquals(List.of("§aMoved the capital of Sunsora §ato province 393"), StaffAdminService.moveCapital(staff, move, 393)); + // The character ended between preview and confirm. + members.when(() -> Members.session(staff)).thenReturn(Optional.empty()); + assertEquals(List.of(StaffAdminService.NEEDS_CHARACTER), StaffAdminService.moveCapital(staff, move, 393)); + topology.verify(() -> TreasuryTopology.capital(eq(staff), eq(faction), eq(393), isNull(), eq(false), eq(0.0), + anyString(), eq(false)), org.mockito.Mockito.times(3)); } } @@ -350,6 +381,7 @@ void colourBannerAndDetailsChange() { verify(faction).setGovernment("Monarchy"); StaffAdminService.set(faction, "rulertitle", "Queen"); verify(faction).setRulerTitle("Queen"); + assertEquals(List.of("§cEnter a value instead of §c."), StaffAdminService.set(faction, "culture", "")); assertEquals(List.of("§cUnknown field motto§c. Use one of: culture, religion, rulingsystem, rulertitle"), StaffAdminService.set(faction, "motto", "x")); } From a42cbb245946253e8eb780bd3a18c61f4df876af Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:18:53 +0200 Subject: [PATCH 3/6] fix: clearer staff capital refusals Standing outside any province says so, and only a capital move that goes through the treasury asks the staff member to play a character; founding a first capital or repairing one in place does not. Co-Authored-By: Claude Opus 5.5 --- .../simplefactions/admin/StaffAdminService.java | 7 ++++++- .../simplefactions/admin/StaffAdminServiceTest.java | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java index e67ff2f2..b645873c 100644 --- a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java @@ -158,6 +158,10 @@ public static CapitalMove previewCapital(Player staff, Faction faction, int prov lines.add("§a[SimpleFactions] §cError! could not resolve province"); return null; } + if (province == 0) { + lines.add("§cThis location has no province!"); + return null; + } if (!faction.ownsProvince(province)) { lines.add("§c" + faction.getName() + " §cdoes not own province " + province + "§c. Give it to them with §e/faction forceclaim " + faction.getId() + " §cfirst."); @@ -171,7 +175,8 @@ public static CapitalMove previewCapital(Player staff, Faction faction, int prov + "§c. Rename it with §e/faction admin renamesettlement " + faction.getId() + " capital "); return null; } - if (Members.session(staff).isEmpty()) { + // Only a real move goes through the treasury, which needs the character making it. + if (faction.hasCapital() && !inCapital && Members.session(staff).isEmpty()) { lines.add(NEEDS_CHARACTER); return null; } diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java index f6e0b1c7..36d9438f 100644 --- a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java @@ -239,6 +239,9 @@ void capitalPreviewRefusesBadProvinces() { assertNull(StaffAdminService.previewCapital(staff, faction, -2, null, lines)); assertEquals(List.of("§a[SimpleFactions] §cError! could not resolve province"), lines); lines.clear(); + assertNull(StaffAdminService.previewCapital(staff, faction, 0, null, lines)); + assertEquals(List.of("§cThis location has no province!"), lines); + lines.clear(); assertNull(StaffAdminService.previewCapital(staff, faction, 9, null, lines)); assertEquals(List.of("§cSunsora §cdoes not own province 9§c. Give it to them with §e/faction forceclaim sunsora §cfirst."), lines); lines.clear(); @@ -267,6 +270,8 @@ void aCapitalWithoutItsSettlementIsFoundedAgainInPlace() { when(faction.hasCapital()).thenReturn(true); when(faction.getCapital()).thenReturn(395); when(settlements.validateFactionCapital(staff, 395, "Sunsora_Castle")).thenReturn(CapitalResult.ok("ok")); + // Founding in place needs no character: it does not go through the treasury. + members.when(() -> Members.session(staff)).thenReturn(Optional.empty()); StaffAdminService.CapitalMove move = StaffAdminService.previewCapital(staff, faction, 395, "Sunsora_Castle", lines); assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 395, "Sunsora_Castle"), move); assertEquals("§eFound " + StaffAdminService.displayName("Sunsora_Castle") + " §ein the capital of Sunsora §e(province 395)", lines.get(0)); From d2d40f7fbf45e05d97cf39fa2c10612280c3ca4b Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:46:22 +0200 Subject: [PATCH 4/6] fix: leaving a faction or guild drops what only members hold Kicks and leaves (/faction kick, /faction leave, /guild kick, /guild leave and both staff kicks) now go through MemberDepartures, so a character who leaves no longer keeps: - guild invites, dividend eligibility or a leadership offer; - a council seat, proposals, election candidacy and ballots, or a movement role, and their vote in a sitting council session (the same rights a death removes); - the Spymaster office, which falls back to the leader; - a place in a faction war host: they leave it as with /warband leave, so leadership passes on and the battle is told, even when offline. Leaving only a guild clears that guild's invite, dividend and offer. Co-Authored-By: Claude Opus 5.5 --- .../admin/StaffAdminService.java | 7 +- .../espionage/EspionageState.java | 8 ++ .../simplefactions/government/Government.java | 9 +- .../simplefactions/guild/Guild.java | 8 +- .../guild/MemberDepartures.java | 39 ++++++ .../managers/CommandManager.java | 12 +- .../warband/CampaignWarbandBattleService.java | 8 +- .../warband/WarbandMembershipService.java | 18 +++ .../admin/StaffAdminServiceTest.java | 15 ++- .../espionage/EspionageStateVacateTest.java | 21 +++ .../guild/GuildDividendEligibilityTest.java | 15 +++ .../guild/MemberDeparturesTest.java | 122 ++++++++++++++++++ .../CampaignWarbandOfflineLeaveTest.java | 29 +++++ .../warband/WarbandMembershipServiceTest.java | 28 ++++ 14 files changed, 317 insertions(+), 22 deletions(-) create mode 100644 src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/espionage/EspionageStateVacateTest.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandOfflineLeaveTest.java diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java index b645873c..04779669 100644 --- a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java @@ -9,6 +9,7 @@ import net.tfminecraft.simplefactions.economy.TreasuryActions; import net.tfminecraft.simplefactions.economy.TreasuryTopology; import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.guild.MemberDepartures; import net.tfminecraft.simplefactions.identity.Members; import net.tfminecraft.simplefactions.managers.FactionManager; import net.tfminecraft.simplefactions.objects.Bank; @@ -273,8 +274,7 @@ public static List kick(Faction faction, String input) { + "§c. Replace them with §e/guild forceleader §cor delete the guild first."); } } - faction.forceRemoveMember(key); - faction.updatePrestige(); + MemberDepartures.leaveRealm(faction, key); Members.online(key).ifPresent(kicked -> kicked.sendMessage("§cStaff removed you from " + faction.getName())); return List.of("§aRemoved " + Members.display(key) + " §afrom " + faction.getName()); } @@ -339,8 +339,7 @@ public static List guildKick(Guild guild, String input) { return List.of("§c" + Members.display(key) + " §cleads " + guild.getName() + "§c. Replace them with §e/guild forceleader §cfirst."); } - guild.kick(key); - guild.getFaction().addMember(key); + MemberDepartures.leaveGuild(guild, key); Members.online(key).ifPresent(kicked -> kicked.sendMessage("§cStaff removed you from " + guild.getName() + "§c. You are back in the main guild of " + guild.getFaction().getName())); return List.of("§aMoved " + Members.display(key) + " §afrom " + guild.getName() + " §ato the main guild of " diff --git a/src/main/java/net/tfminecraft/simplefactions/espionage/EspionageState.java b/src/main/java/net/tfminecraft/simplefactions/espionage/EspionageState.java index 49e78308..35c506b3 100644 --- a/src/main/java/net/tfminecraft/simplefactions/espionage/EspionageState.java +++ b/src/main/java/net/tfminecraft/simplefactions/espionage/EspionageState.java @@ -157,6 +157,14 @@ public SpecialPositionAssignment getSpymaster() { return positions.get(SpecialPosition.SPYMASTER); } + /** A Spymaster who left the realm gives up the office; it falls back to the leader, as for any vacancy. */ + public boolean vacateFor(String memberKey) { + var holder = getSpymaster(); + if (holder == null || !memberKey.equals(holder.memberKey())) return false; + removeSpymaster(); + return true; + } + public void removeSpymaster() { pendingFounders.remove(SpecialPosition.SPYMASTER); pendingFounderCharacters.remove(SpecialPosition.SPYMASTER); diff --git a/src/main/java/net/tfminecraft/simplefactions/government/Government.java b/src/main/java/net/tfminecraft/simplefactions/government/Government.java index 3e65d984..bb74e976 100644 --- a/src/main/java/net/tfminecraft/simplefactions/government/Government.java +++ b/src/main/java/net/tfminecraft/simplefactions/government/Government.java @@ -447,8 +447,15 @@ public boolean canProposePolitical(Player p, Action action) { return true; } - /** Removes only this character's live political rights; other votes retain their order and value. */ public void removeDeadCharacter(String key) { + removeRights(key); + } + + /** + * Removes only this character's live political rights, on death or on leaving the realm; + * other votes retain their order and value. + */ + public void removeRights(String key) { council.getMembers().removeIf(key::equals); if (council.refuses(key)) council.toggleRefuse(key); council.getProposalHandler().getProposals().removeIf(proposal -> key.equals(proposal.getProposer())); diff --git a/src/main/java/net/tfminecraft/simplefactions/guild/Guild.java b/src/main/java/net/tfminecraft/simplefactions/guild/Guild.java index 9da9f95f..ceadeef8 100644 --- a/src/main/java/net/tfminecraft/simplefactions/guild/Guild.java +++ b/src/main/java/net/tfminecraft/simplefactions/guild/Guild.java @@ -497,11 +497,15 @@ public void kick(String member) { /** The death journal has already captured any money owed before this removal. */ public void removeDeadCharacter(String key) { members.removeIf(key::equals); + forgetDeparted(key); + if (key.equals(leader)) leader = ""; + if (company != null) company.removeDeadCharacter(key); + } + /** Someone who left this guild keeps no invite, dividend place or leadership offer in it. */ + public void forgetDeparted(String key) { invites.removeIf(key::equals); dividendEligible.removeIf(key::equals); - if (key.equals(leader)) leader = ""; if (key.equals(leaderOffer)) leaderOffer = null; - if (company != null) company.removeDeadCharacter(key); } public String getLeader() { return isBase() ? host.getLeader() : leader; } diff --git a/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java b/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java new file mode 100644 index 00000000..0f93eee1 --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java @@ -0,0 +1,39 @@ +package net.tfminecraft.simplefactions.guild; + +import net.tfminecraft.simplefactions.SimpleFactions; +import net.tfminecraft.simplefactions.government.session.Session; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.war.battle.warband.WarbandMembershipService; + +/** + * A living character leaving a guild or a realm, by choice or by a kick. Like a + * death, it takes away what only members may hold, so nothing stale is left behind. + */ +public final class MemberDepartures { + private MemberDepartures() {} + + /** From a guild back to the realm's main guild: {@code /guild leave} and {@code /guild kick}. */ + public static void leaveGuild(Guild guild, String key) { + guild.kick(key); + guild.forgetDeparted(key); + guild.getFaction().addMember(key); + } + + /** Out of the realm and every guild in it: {@code /faction leave} and {@code /faction kick}. */ + public static void leaveRealm(Faction faction, String key) { + faction.forceRemoveMember(key); + for (Guild guild : faction.getGuildHandler().getGuilds()) guild.forgetDeparted(key); + // Council seat, proposals, election ballots and candidacy, movement roles. + faction.getGovernment().removeRights(key); + var plugin = SimpleFactions.getInstance(); + if (plugin != null && plugin.getSessionManager() != null) { + Session session = plugin.getSessionManager().getSession(faction.getGovernment().getCouncil()); + // As for a death: their vote and place in a sitting council session go. + if (session != null) session.removeDeadCharacter(key); + } + faction.getEspionage().vacateFor(key); + Members.ref(key).ifPresent(WarbandMembershipService.getInstance()::removeFromFactionWarband); + faction.updatePrestige(); + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java index d60a8277..c41839fa 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java @@ -1,6 +1,7 @@ package net.tfminecraft.simplefactions.managers; import net.tfminecraft.simplefactions.admin.StaffAdminCommand; +import net.tfminecraft.simplefactions.guild.MemberDepartures; import net.tfminecraft.simplefactions.map.AdminLandService; import net.tfminecraft.simplefactions.identity.Members; import net.tfminecraft.simplefactions.economy.TreasuryActions; @@ -300,8 +301,7 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, return true; } // Leaving a guild keeps you in the faction, back in its main guild. - g.kick(Members.key(p)); - g.getFaction().addMember(Members.key(p)); + MemberDepartures.leaveGuild(g, Members.key(p)); p.sendMessage("§aLeft "+g.getName()+" §aand returned to the main guild of "+g.getFaction().getName()); return true; } else if(cmd.getName().equalsIgnoreCase(cmd2) && args[0].equalsIgnoreCase("kick") && args.length >= 2) { @@ -324,8 +324,7 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, return true; } // A guild leader can only remove someone from the guild; they stay in the faction's main guild. - guild.kick(kicked); - guild.getFaction().addMember(kicked); + MemberDepartures.leaveGuild(guild, kicked); p.sendMessage("§aKicked "+Members.display(kicked)); Members.online(kicked).ifPresent(pl -> pl.sendMessage("§a"+Members.name(p)+ " kicked you from "+guild.getName() + "§a. You are back in the main guild of "+guild.getFaction().getName())); @@ -994,7 +993,7 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, p.sendMessage("§cPlayer is a member of a guild"); return true; } - f.forceRemoveMember(kicked); + MemberDepartures.leaveRealm(f, kicked); p.sendMessage("§aKicked "+Members.display(kicked)); Members.online(kicked).ifPresent(pl -> pl.sendMessage("§a"+Members.name(p)+ " kicked you from "+f.getName())); return true; @@ -1084,9 +1083,8 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, p.sendMessage("§cYou are in a guild. Use /guild leave first, then /faction leave"); return true; } - f.forceRemoveMember(Members.key(p)); + MemberDepartures.leaveRealm(f, Members.key(p)); p.sendMessage("§aLeft "+f.getName()); - f.updatePrestige(); return true; } else if(cmd.getName().equalsIgnoreCase(cmd1) && args[0].equalsIgnoreCase("rename") && args.length == 2) { Faction f = FactionManager.getByLeader(Members.key(p)); diff --git a/src/main/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandBattleService.java b/src/main/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandBattleService.java index 966ef1bf..c44633e4 100644 --- a/src/main/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandBattleService.java +++ b/src/main/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandBattleService.java @@ -116,7 +116,11 @@ public static void processLeave(Player player, Warband warband, boolean voluntar if (player == null || warband == null) { return; } - UUID playerId = player.getUniqueId(); + processLeave(player.getUniqueId(), player, warband, voluntaryLeave); + } + + /** {@code player} is null when the account is offline, e.g. a character kicked from its faction. */ + public static void processLeave(UUID playerId, Player player, Warband warband, boolean voluntaryLeave) { boolean wasLeader = playerId.equals(warband.getLeaderId()); CampaignBattleJoinService.CampaignBattleContext ctx = CampaignBattleJoinService.findCampaignBattleForWarband(warband); @@ -124,7 +128,7 @@ public static void processLeave(Player player, Warband warband, boolean voluntar if (ctx != null && ctx.battle().hasStarted() && voluntaryLeave) { CampaignWarbandLeaveBlock.block(ctx.battle().getId(), warband.getId(), playerId); } - detachFromBattle(player, warband); + if (player != null) detachFromBattle(player, warband); warband.removeMember(playerId); if (wasLeader) { diff --git a/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java b/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java index 7995d0b0..6de3194a 100644 --- a/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java +++ b/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java @@ -13,6 +13,7 @@ import net.tfminecraft.simplefactions.objects.Faction; import net.tfminecraft.simplefactions.war.battle.campaign.CampaignBattleJoinService; import net.tfminecraft.simplefactions.war.battle.campaign.CampaignBattleJoinService.CampaignBattleContext; +import net.tfminecraft.simplefactions.war.battle.campaign.warband.CampaignWarbandBattleService; import net.tfminecraft.simplefactions.war.battle.campaign.warband.CampaignWarbandSideLock; import net.tfminecraft.simplefactions.war.battle.engine.core.Battle; import net.tfminecraft.simplefactions.war.battle.engine.core.BattleManager; @@ -129,6 +130,23 @@ boolean attemptRejoin(UUID playerId, String memberKey, Player onlinePlayer) { return true; } + /** + * A character who left their faction leaves its war host too, as with {@code /warband leave}: + * the leadership passes on, the battle is told, and no place is kept to rejoin. + */ + public boolean removeFromFactionWarband(CharacterRef character) { + UUID owner = character.ownerId(); + WarbandRejoinState pending = pendingRejoin.get(owner); + if (pending != null && pending.isFor(character)) pendingRejoin.remove(owner); + Warband warband = WarbandManager.getByMemberId(owner); + if (warband == null || !warband.isFaction() + || !warband.getMemberCharacter(owner).equals(java.util.Optional.of(character))) { + return false; + } + CampaignWarbandBattleService.processLeave(owner, getOnlinePlayer(owner), warband, true); + return true; + } + public WarbandRejoinState getPendingRejoin(UUID playerId) { return pendingRejoin.get(playerId); } diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java index 36d9438f..9fbef31a 100644 --- a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java @@ -3,6 +3,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; @@ -27,6 +28,7 @@ import net.tfminecraft.simplefactions.economy.TreasuryTopology; import net.tfminecraft.simplefactions.guild.Guild; +import net.tfminecraft.simplefactions.guild.MemberDepartures; import net.tfminecraft.simplefactions.identity.Members; import net.tfminecraft.simplefactions.managers.FactionManager; import net.tfminecraft.simplefactions.map.MapSystem; @@ -44,6 +46,7 @@ class StaffAdminServiceTest { private MockedStatic members; private MockedStatic factions; private MockedStatic colours; + private MockedStatic departures; private Faction faction; private GuildHandler guilds; private Guild base; @@ -58,6 +61,7 @@ void setUp() { members = mockStatic(Members.class, CALLS_REAL_METHODS); factions = mockStatic(FactionManager.class, CALLS_REAL_METHODS); colours = mockStatic(RandomRGB.class); + departures = mockStatic(MemberDepartures.class); members.when(() -> Members.display(anyString())).thenAnswer(call -> call.getArgument(0)); members.when(() -> Members.online(anyString())).thenReturn(Optional.empty()); faction = mock(Faction.class); @@ -97,6 +101,7 @@ void setUp() { @AfterEach void tearDown() { + departures.close(); colours.close(); factions.close(); members.close(); @@ -401,13 +406,12 @@ void kickRemovesMembersButNotLeaders() { members.when(() -> Members.find(faction.getMembers(), "carl")).thenReturn("Carl"); assertEquals(List.of("§cCarl §cleads the guild Smiths§c. Replace them with §e/guild forceleader §cor delete the guild first."), StaffAdminService.kick(faction, "carl")); - verify(faction, never()).forceRemoveMember(anyString()); + departures.verify(() -> MemberDepartures.leaveRealm(any(), anyString()), never()); members.when(() -> Members.find(faction.getMembers(), "bob")).thenReturn("Bob"); Player bob = mock(Player.class); members.when(() -> Members.online("Bob")).thenReturn(Optional.of(bob)); assertEquals(List.of("§aRemoved Bob §afrom Sunsora"), StaffAdminService.kick(faction, "bob")); - verify(faction).forceRemoveMember("Bob"); - verify(faction).updatePrestige(); + departures.verify(() -> MemberDepartures.leaveRealm(faction, "Bob")); verify(bob).sendMessage("§cStaff removed you from Sunsora"); } @@ -472,11 +476,10 @@ void guildKickSendsMembersBackToTheMainGuild() { members.when(() -> Members.find(smiths.getMembers(), "carl")).thenReturn("Carl"); assertEquals(List.of("§cCarl §cleads Smiths§c. Replace them with §e/guild forceleader §cfirst."), StaffAdminService.guildKick(smiths, "carl")); - verify(smiths, never()).kick(anyString()); + departures.verify(() -> MemberDepartures.leaveGuild(any(), anyString()), never()); members.when(() -> Members.find(smiths.getMembers(), "bob")).thenReturn("Bob"); assertEquals(List.of("§aMoved Bob §afrom Smiths §ato the main guild of Sunsora"), StaffAdminService.guildKick(smiths, "bob")); - verify(smiths).kick("Bob"); - verify(faction).addMember("Bob"); + departures.verify(() -> MemberDepartures.leaveGuild(smiths, "Bob")); Player bob = mock(Player.class); members.when(() -> Members.online("Bob")).thenReturn(Optional.of(bob)); StaffAdminService.guildKick(smiths, "bob"); diff --git a/src/test/java/net/tfminecraft/simplefactions/espionage/EspionageStateVacateTest.java b/src/test/java/net/tfminecraft/simplefactions/espionage/EspionageStateVacateTest.java new file mode 100644 index 00000000..af15d59e --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/espionage/EspionageStateVacateTest.java @@ -0,0 +1,21 @@ +package net.tfminecraft.simplefactions.espionage; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class EspionageStateVacateTest { + @Test + void onlyTheSpymasterWhoLeftGivesUpTheOffice() { + var state = new EspionageState(); + assertFalse(state.vacateFor(EspionageTestFixtures.key("Spy"))); + state.appoint(EspionageTestFixtures.bind(new SpecialPositionAssignment(), "Spy"), 80); + assertFalse(state.vacateFor(EspionageTestFixtures.key("Other"))); + assertNotNull(state.getSpymaster()); + assertTrue(state.vacateFor(EspionageTestFixtures.key("Spy"))); + assertNull(state.getSpymaster()); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/guild/GuildDividendEligibilityTest.java b/src/test/java/net/tfminecraft/simplefactions/guild/GuildDividendEligibilityTest.java index db40dd83..6f9ef905 100644 --- a/src/test/java/net/tfminecraft/simplefactions/guild/GuildDividendEligibilityTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/guild/GuildDividendEligibilityTest.java @@ -51,6 +51,21 @@ void tenureToggleIncludesEveryone() throws Exception { } } + @Test + void aDepartedMemberKeepsNoInviteDividendPlaceOrLeadershipOffer() throws Exception { + Guild guild = emptyGuild(); + set(guild, "invites", new ArrayList<>(List.of("Bob", "Ann"))); + set(guild, "dividendEligible", new ArrayList<>(List.of("Bob", "Ann"))); + guild.offerLeadership("Bob"); + guild.forgetDeparted("Bob"); + assertEquals(List.of("Ann"), guild.getInvites()); + assertEquals(List.of("Ann"), guild.getDividendEligibleSnapshot()); + assertEquals(null, guild.getLeaderOffer()); + guild.offerLeadership("Ann"); + guild.forgetDeparted("Bob"); + assertEquals("Ann", guild.getLeaderOffer()); + } + private static Guild emptyGuild() throws Exception { Guild guild = mock(Guild.class, withSettings().defaultAnswer(Mockito.CALLS_REAL_METHODS)); set(guild, "members", new ArrayList()); diff --git a/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java b/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java new file mode 100644 index 00000000..2bfbc508 --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java @@ -0,0 +1,122 @@ +package net.tfminecraft.simplefactions.guild; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.CALLS_REAL_METHODS; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.Optional; +import java.util.UUID; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.InOrder; +import org.mockito.MockedStatic; + +import net.tfminecraft.simplefactions.SimpleFactions; +import net.tfminecraft.simplefactions.espionage.EspionageState; +import net.tfminecraft.simplefactions.government.Council; +import net.tfminecraft.simplefactions.government.Government; +import net.tfminecraft.simplefactions.government.session.Session; +import net.tfminecraft.simplefactions.identity.Members; +import net.tfminecraft.simplefactions.managers.SessionManager; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.objects.handler.GuildHandler; +import net.tfminecraft.simplefactions.war.battle.warband.WarbandMembershipService; +import net.tfminecraft.tlibs.character.CharacterRef; + +class MemberDeparturesTest { + private MockedStatic plugin; + private MockedStatic members; + private MockedStatic warbands; + private WarbandMembershipService warbandService; + private Faction faction; + private Guild main; + private Guild smiths; + private Government government; + private Council council; + private EspionageState espionage; + + @BeforeEach + void setUp() { + plugin = mockStatic(SimpleFactions.class); + members = mockStatic(Members.class, CALLS_REAL_METHODS); + warbands = mockStatic(WarbandMembershipService.class); + warbandService = mock(WarbandMembershipService.class); + warbands.when(WarbandMembershipService::getInstance).thenReturn(warbandService); + faction = mock(Faction.class); + main = mock(Guild.class); + smiths = mock(Guild.class); + GuildHandler guilds = mock(GuildHandler.class); + when(guilds.getGuilds()).thenReturn(List.of(main, smiths)); + when(faction.getGuildHandler()).thenReturn(guilds); + government = mock(Government.class); + council = mock(Council.class); + when(government.getCouncil()).thenReturn(council); + when(faction.getGovernment()).thenReturn(government); + espionage = mock(EspionageState.class); + when(faction.getEspionage()).thenReturn(espionage); + when(smiths.getFaction()).thenReturn(faction); + } + + @AfterEach + void tearDown() { + warbands.close(); + members.close(); + plugin.close(); + } + + @Test + void leavingAGuildClearsItsOffersAndReturnsToTheMainGuild() { + MemberDepartures.leaveGuild(smiths, "Bob"); + InOrder order = inOrder(smiths, faction); + order.verify(smiths).kick("Bob"); + order.verify(smiths).forgetDeparted("Bob"); + order.verify(faction).addMember("Bob"); + verify(government, never()).removeRights("Bob"); + } + + @Test + void leavingTheRealmDropsEveryRightLikeADeath() { + SimpleFactions instance = mock(SimpleFactions.class); + SessionManager sessions = mock(SessionManager.class); + Session session = mock(Session.class); + when(instance.getSessionManager()).thenReturn(sessions); + when(sessions.getSession(council)).thenReturn(session); + plugin.when(SimpleFactions::getInstance).thenReturn(instance); + String key = UUID.randomUUID().toString(); + CharacterRef ref = new CharacterRef(UUID.randomUUID(), UUID.fromString(key)); + members.when(() -> Members.ref(key)).thenReturn(Optional.of(ref)); + + MemberDepartures.leaveRealm(faction, key); + + verify(faction).forceRemoveMember(key); + verify(main).forgetDeparted(key); + verify(smiths).forgetDeparted(key); + verify(government).removeRights(key); + verify(session).removeDeadCharacter(key); + verify(espionage).vacateFor(key); + verify(warbandService).removeFromFactionWarband(ref); + verify(faction).updatePrestige(); + } + + @Test + void noPluginSessionOrCharacterIsSkipped() { + MemberDepartures.leaveRealm(faction, "Legacy Name"); + SimpleFactions instance = mock(SimpleFactions.class); + plugin.when(SimpleFactions::getInstance).thenReturn(instance); + MemberDepartures.leaveRealm(faction, "Legacy Name"); + SessionManager sessions = mock(SessionManager.class); + when(instance.getSessionManager()).thenReturn(sessions); + MemberDepartures.leaveRealm(faction, "Legacy Name"); + verify(government, org.mockito.Mockito.times(3)).removeRights("Legacy Name"); + verify(sessions).getSession(council); + verify(warbandService, never()).removeFromFactionWarband(any()); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandOfflineLeaveTest.java b/src/test/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandOfflineLeaveTest.java new file mode 100644 index 00000000..c6326f2c --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/war/battle/campaign/warband/CampaignWarbandOfflineLeaveTest.java @@ -0,0 +1,29 @@ +package net.tfminecraft.simplefactions.war.battle.campaign.warband; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.mockito.Mockito.mockStatic; + +import java.util.UUID; + +import org.junit.jupiter.api.Test; + +import net.tfminecraft.simplefactions.war.battle.persistence.BattlePersistenceService; +import net.tfminecraft.simplefactions.war.battle.campaign.CampaignBattleJoinService; +import net.tfminecraft.simplefactions.war.battle.warband.Warband; + +class CampaignWarbandOfflineLeaveTest { + @Test + void anOfflineLeaderLeavesAndTheLeadershipPasses() { + UUID leader = UUID.randomUUID(); + UUID other = UUID.randomUUID(); + Warband warband = Warband.createWithMemberIds("host", leader, true, leader, other); + try (var joins = mockStatic(CampaignBattleJoinService.class); + var persistence = mockStatic(BattlePersistenceService.class)) { + CampaignWarbandBattleService.processLeave(leader, null, warband, true); + assertFalse(warband.hasMember(leader)); + assertEquals(other, warband.getLeaderId()); + persistence.verify(() -> BattlePersistenceService.persistWarband(warband)); + } + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java index cfe32e86..065d8472 100644 --- a/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java @@ -196,6 +196,34 @@ void pendingLeaderDisplayName() { assertEquals("The Brume Host", shell.getName()); } + @Test + void aCharacterWhoLeftTheirFactionLeavesItsWarHostButNotOtherWarbands() throws Exception { + Warband warband = createOpenWarband("alpha", leaderId, memberId); + CharacterRef ref = warband.getMemberCharacter(memberId).orElseThrow(); + try (var battles = mockStatic( + net.tfminecraft.simplefactions.war.battle.campaign.warband.CampaignWarbandBattleService.class)) { + assertFalse(service.removeFromFactionWarband(ref)); + markAsFaction(warband); + assertFalse(service.removeFromFactionWarband(new CharacterRef(memberId, UUID.randomUUID()))); + assertFalse(service.removeFromFactionWarband(new CharacterRef(UUID.randomUUID(), UUID.randomUUID()))); + assertTrue(service.removeFromFactionWarband(ref)); + battles.verify(() -> net.tfminecraft.simplefactions.war.battle.campaign.warband.CampaignWarbandBattleService + .processLeave(memberId, null, warband, true)); + battles.verifyNoMoreInteractions(); + } + } + + @Test + void aPlaceKeptAfterQuittingIsDroppedOnlyForThatCharacter() { + Warband warband = createOpenWarband("alpha", leaderId, memberId); + CharacterRef ref = warband.getMemberCharacter(memberId).orElseThrow(); + service.handleQuit(member); + assertFalse(service.removeFromFactionWarband(new CharacterRef(memberId, UUID.randomUUID()))); + assertNotNull(service.getPendingRejoin(memberId)); + assertFalse(service.removeFromFactionWarband(ref)); + assertNull(service.getPendingRejoin(memberId)); + } + private Warband createOpenWarband(String id, UUID leader, UUID member) { Warband warband = Warband.createWithMemberIds(id, leader, false, member); warband.addMember(member, Members.session(this.member).orElseThrow().ref()); From 2f98c1c2c01ccbc215860585d858968dacff0180 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sun, 25 Oct 2026 09:39:30 +0100 Subject: [PATCH 5/6] fix: departures keep unrelated warband places; kicks don't block rejoining - A place kept after quitting a player's own warband survives leaving the faction; only a faction war host place is dropped. - Only /faction leave blocks rejoining the battle's host; after a kick the realm may let the member back. - A guild invite from another realm can no longer move a member out of the faction they joined since; they are told to /faction leave first. Co-Authored-By: Claude Opus 5.5 --- .../admin/StaffAdminService.java | 2 +- .../guild/MemberDepartures.java | 6 +-- .../managers/CommandManager.java | 4 +- .../managers/InviteCommands.java | 7 +++- .../warband/WarbandMembershipService.java | 8 ++-- .../admin/StaffAdminServiceTest.java | 5 ++- .../guild/GuildDeleteCommandTest.java | 21 ++++++++++ .../guild/MemberDeparturesTest.java | 16 +++++--- .../managers/CommandManagerCoverageTest.java | 6 +++ .../warband/WarbandMembershipServiceTest.java | 40 ++++++++++++++----- 10 files changed, 86 insertions(+), 29 deletions(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java index 04779669..a7d73b91 100644 --- a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java @@ -274,7 +274,7 @@ public static List kick(Faction faction, String input) { + "§c. Replace them with §e/guild forceleader §cor delete the guild first."); } } - MemberDepartures.leaveRealm(faction, key); + MemberDepartures.leaveRealm(faction, key, false); Members.online(key).ifPresent(kicked -> kicked.sendMessage("§cStaff removed you from " + faction.getName())); return List.of("§aRemoved " + Members.display(key) + " §afrom " + faction.getName()); } diff --git a/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java b/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java index 0f93eee1..66425e46 100644 --- a/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java +++ b/src/main/java/net/tfminecraft/simplefactions/guild/MemberDepartures.java @@ -20,8 +20,8 @@ public static void leaveGuild(Guild guild, String key) { guild.getFaction().addMember(key); } - /** Out of the realm and every guild in it: {@code /faction leave} and {@code /faction kick}. */ - public static void leaveRealm(Faction faction, String key) { + /** Out of the realm and every guild in it: {@code /faction leave} ({@code voluntary}) and {@code /faction kick}. */ + public static void leaveRealm(Faction faction, String key, boolean voluntary) { faction.forceRemoveMember(key); for (Guild guild : faction.getGuildHandler().getGuilds()) guild.forgetDeparted(key); // Council seat, proposals, election ballots and candidacy, movement roles. @@ -33,7 +33,7 @@ public static void leaveRealm(Faction faction, String key) { if (session != null) session.removeDeadCharacter(key); } faction.getEspionage().vacateFor(key); - Members.ref(key).ifPresent(WarbandMembershipService.getInstance()::removeFromFactionWarband); + Members.ref(key).ifPresent(ref -> WarbandMembershipService.getInstance().removeFromFactionWarband(ref, voluntary)); faction.updatePrestige(); } } diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java index c41839fa..7fde41b6 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java @@ -993,7 +993,7 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, p.sendMessage("§cPlayer is a member of a guild"); return true; } - MemberDepartures.leaveRealm(f, kicked); + MemberDepartures.leaveRealm(f, kicked, false); p.sendMessage("§aKicked "+Members.display(kicked)); Members.online(kicked).ifPresent(pl -> pl.sendMessage("§a"+Members.name(p)+ " kicked you from "+f.getName())); return true; @@ -1083,7 +1083,7 @@ private boolean dispatchCommand(CommandSender sender, Command cmd, String label, p.sendMessage("§cYou are in a guild. Use /guild leave first, then /faction leave"); return true; } - MemberDepartures.leaveRealm(f, Members.key(p)); + MemberDepartures.leaveRealm(f, Members.key(p), true); p.sendMessage("§aLeft "+f.getName()); return true; } else if(cmd.getName().equalsIgnoreCase(cmd1) && args[0].equalsIgnoreCase("rename") && args.length == 2) { diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/InviteCommands.java b/src/main/java/net/tfminecraft/simplefactions/managers/InviteCommands.java index 3dfcc375..e2e864ab 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/InviteCommands.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/InviteCommands.java @@ -138,11 +138,16 @@ private static void joinGuild(Player p, String id) { p.sendMessage("§cNo guild by the id " + id); return; } + Guild previous = FactionManager.getGuildByMember(Members.key(p)); + // An invite sent while they had no faction must not move them out of one they joined since. + if (previous != null && previous.getFaction() != g.getFaction()) { + p.sendMessage("§cYou belong to " + previous.getFaction().getName() + "§c. Use §e/faction leave §cfirst."); + return; + } if (!g.consumeInvite(Members.key(p))) { p.sendMessage("§cYou need to be invited to this guild by the leader first!"); return; } - Guild previous = FactionManager.getGuildByMember(Members.key(p)); if (previous != null) { previous.kick(Members.key(p)); } diff --git a/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java b/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java index 6de3194a..87fa2de0 100644 --- a/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java +++ b/src/main/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipService.java @@ -134,16 +134,18 @@ boolean attemptRejoin(UUID playerId, String memberKey, Player onlinePlayer) { * A character who left their faction leaves its war host too, as with {@code /warband leave}: * the leadership passes on, the battle is told, and no place is kept to rejoin. */ - public boolean removeFromFactionWarband(CharacterRef character) { + public boolean removeFromFactionWarband(CharacterRef character, boolean voluntary) { UUID owner = character.ownerId(); WarbandRejoinState pending = pendingRejoin.get(owner); - if (pending != null && pending.isFor(character)) pendingRejoin.remove(owner); + // A place kept in a player's own warband has nothing to do with the faction. + if (pending != null && pending.isFor(character) && pending.hasFaction()) pendingRejoin.remove(owner); Warband warband = WarbandManager.getByMemberId(owner); if (warband == null || !warband.isFaction() || !warband.getMemberCharacter(owner).equals(java.util.Optional.of(character))) { return false; } - CampaignWarbandBattleService.processLeave(owner, getOnlinePlayer(owner), warband, true); + // Only someone who walks out is blocked from rejoining this battle; a kicked member may be let back. + CampaignWarbandBattleService.processLeave(owner, getOnlinePlayer(owner), warband, voluntary); return true; } diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java index 9fbef31a..b2fbbc1a 100644 --- a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; @@ -406,12 +407,12 @@ void kickRemovesMembersButNotLeaders() { members.when(() -> Members.find(faction.getMembers(), "carl")).thenReturn("Carl"); assertEquals(List.of("§cCarl §cleads the guild Smiths§c. Replace them with §e/guild forceleader §cor delete the guild first."), StaffAdminService.kick(faction, "carl")); - departures.verify(() -> MemberDepartures.leaveRealm(any(), anyString()), never()); + departures.verify(() -> MemberDepartures.leaveRealm(any(), anyString(), anyBoolean()), never()); members.when(() -> Members.find(faction.getMembers(), "bob")).thenReturn("Bob"); Player bob = mock(Player.class); members.when(() -> Members.online("Bob")).thenReturn(Optional.of(bob)); assertEquals(List.of("§aRemoved Bob §afrom Sunsora"), StaffAdminService.kick(faction, "bob")); - departures.verify(() -> MemberDepartures.leaveRealm(faction, "Bob")); + departures.verify(() -> MemberDepartures.leaveRealm(faction, "Bob", false)); verify(bob).sendMessage("§cStaff removed you from Sunsora"); } diff --git a/src/test/java/net/tfminecraft/simplefactions/guild/GuildDeleteCommandTest.java b/src/test/java/net/tfminecraft/simplefactions/guild/GuildDeleteCommandTest.java index a124bb3e..652e1498 100644 --- a/src/test/java/net/tfminecraft/simplefactions/guild/GuildDeleteCommandTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/guild/GuildDeleteCommandTest.java @@ -142,6 +142,7 @@ void joiningAnUnknownGuildReportsTheMissingGuildWithoutRemovingCurrentMembership void anExistingInvitationStillTransfersMembershipAndNotifiesTheRealm() { try (Fixture fixture = new Fixture("Joiner", false)) { Guild previous = mock(Guild.class); + when(previous.getFaction()).thenReturn(fixture.faction); fixture.factions.when(() -> FactionManager.canJoinGuild(fixture.player)).thenReturn(true); fixture.factions.when(() -> FactionManager.getGuildByMember("Joiner")).thenReturn(previous); when(fixture.guild.consumeInvite("Joiner")).thenReturn(true); @@ -160,6 +161,26 @@ void anExistingInvitationStillTransfersMembershipAndNotifiesTheRealm() { } } + @Test + void anInvitationFromAnotherRealmCannotPullAMemberOutOfTheirFaction() { + try (Fixture fixture = new Fixture("Joiner", false)) { + Guild previous = mock(Guild.class); + Faction other = mock(Faction.class); + when(other.getName()).thenReturn("Other"); + when(previous.getFaction()).thenReturn(other); + fixture.factions.when(() -> FactionManager.canJoinGuild(fixture.player)).thenReturn(true); + fixture.factions.when(() -> FactionManager.getGuildByMember("Joiner")).thenReturn(previous); + when(fixture.guild.consumeInvite("Joiner")).thenReturn(true); + + assertTrue(fixture.run("join", "smiths")); + + verify(fixture.player).sendMessage("§cYou belong to Other§c. Use §e/faction leave §cfirst."); + verify(previous, never()).kick("Joiner"); + verify(fixture.guild, never()).consumeInvite("Joiner"); + fixture.assertUntouched(); + } + } + private enum FinancialBlock { BANKRUPTCY("§cCannot delete a guild that is in bankruptcy!"), BANK_BALANCE("§cCannot delete a guild while the bank balance is above 0"), diff --git a/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java b/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java index 2bfbc508..1f240c99 100644 --- a/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/guild/MemberDeparturesTest.java @@ -94,7 +94,7 @@ void leavingTheRealmDropsEveryRightLikeADeath() { CharacterRef ref = new CharacterRef(UUID.randomUUID(), UUID.fromString(key)); members.when(() -> Members.ref(key)).thenReturn(Optional.of(ref)); - MemberDepartures.leaveRealm(faction, key); + MemberDepartures.leaveRealm(faction, key, false); verify(faction).forceRemoveMember(key); verify(main).forgetDeparted(key); @@ -102,21 +102,25 @@ void leavingTheRealmDropsEveryRightLikeADeath() { verify(government).removeRights(key); verify(session).removeDeadCharacter(key); verify(espionage).vacateFor(key); - verify(warbandService).removeFromFactionWarband(ref); + verify(warbandService).removeFromFactionWarband(ref, false); verify(faction).updatePrestige(); + + // Walking out, unlike a kick, blocks rejoining the battle. + MemberDepartures.leaveRealm(faction, key, true); + verify(warbandService).removeFromFactionWarband(ref, true); } @Test void noPluginSessionOrCharacterIsSkipped() { - MemberDepartures.leaveRealm(faction, "Legacy Name"); + MemberDepartures.leaveRealm(faction, "Legacy Name", true); SimpleFactions instance = mock(SimpleFactions.class); plugin.when(SimpleFactions::getInstance).thenReturn(instance); - MemberDepartures.leaveRealm(faction, "Legacy Name"); + MemberDepartures.leaveRealm(faction, "Legacy Name", true); SessionManager sessions = mock(SessionManager.class); when(instance.getSessionManager()).thenReturn(sessions); - MemberDepartures.leaveRealm(faction, "Legacy Name"); + MemberDepartures.leaveRealm(faction, "Legacy Name", true); verify(government, org.mockito.Mockito.times(3)).removeRights("Legacy Name"); verify(sessions).getSession(council); - verify(warbandService, never()).removeFromFactionWarband(any()); + verify(warbandService, never()).removeFromFactionWarband(any(), org.mockito.ArgumentMatchers.anyBoolean()); } } diff --git a/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java index db1cbd6d..2496eefa 100644 --- a/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/managers/CommandManagerCoverageTest.java @@ -129,6 +129,7 @@ void setUp() { when(faction.getMembers()).thenReturn(new ArrayList<>(List.of("Leader", "Alice"))); government = mock(Government.class); when(faction.getGovernment()).thenReturn(government); + when(faction.getEspionage()).thenReturn(mock(net.tfminecraft.simplefactions.espionage.EspionageState.class)); guild = mock(Guild.class, RETURNS_DEEP_STUBS); when(guild.getId()).thenReturn("merchants"); when(guild.getIncarnationId()).thenReturn(java.util.UUID.randomUUID()); @@ -908,9 +909,12 @@ void leavingRequiresMembershipAndNonLeadership(String name) { assertTrue(run(name, "leave")); if (name.equals("faction")) { verify(faction).forceRemoveMember("Leader"); + // A departure drops the rights only members hold. + verify(government).removeRights("Leader"); verify(faction).updatePrestige(); } else { verify(guild).kick("Leader"); + verify(guild).forgetDeparted("Leader"); verify(faction).addMember("Leader"); } verify(player).sendMessage(contains("Left ")); @@ -928,6 +932,7 @@ void joiningConsumesAnInvitationBeforeAddingAndNotifyingMembers(String name) { when(faction.consumeInvite("Leader")).thenReturn(true); when(guild.consumeInvite("Leader")).thenReturn(true); Guild previous = mock(Guild.class); + when(previous.getFaction()).thenReturn(faction); factions.when(() -> FactionManager.getGuildByMember("Leader")).thenReturn(previous); Player alice = gui.player("Alice"), outsider = gui.player("Visitor"); online(alice, outsider); @@ -1106,6 +1111,7 @@ void factionKickProtectsLeadersGuildMembersAndNonMembers() { online(alice, visitor); assertTrue(run("faction", "kick", "Alice")); verify(faction).forceRemoveMember("Alice"); + verify(government).removeRights("Alice"); verify(alice).sendMessage(contains("kicked you")); verify(visitor, never()).sendMessage(anyString()); } diff --git a/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java index 065d8472..d0b373ee 100644 --- a/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/war/battle/warband/WarbandMembershipServiceTest.java @@ -202,28 +202,46 @@ void aCharacterWhoLeftTheirFactionLeavesItsWarHostButNotOtherWarbands() throws E CharacterRef ref = warband.getMemberCharacter(memberId).orElseThrow(); try (var battles = mockStatic( net.tfminecraft.simplefactions.war.battle.campaign.warband.CampaignWarbandBattleService.class)) { - assertFalse(service.removeFromFactionWarband(ref)); + assertFalse(service.removeFromFactionWarband(ref, true)); + assertTrue(warband.hasMember(memberId)); markAsFaction(warband); - assertFalse(service.removeFromFactionWarband(new CharacterRef(memberId, UUID.randomUUID()))); - assertFalse(service.removeFromFactionWarband(new CharacterRef(UUID.randomUUID(), UUID.randomUUID()))); - assertTrue(service.removeFromFactionWarband(ref)); + assertFalse(service.removeFromFactionWarband(new CharacterRef(memberId, UUID.randomUUID()), true)); + assertFalse(service.removeFromFactionWarband(new CharacterRef(UUID.randomUUID(), UUID.randomUUID()), true)); + assertTrue(service.removeFromFactionWarband(ref, false)); battles.verify(() -> net.tfminecraft.simplefactions.war.battle.campaign.warband.CampaignWarbandBattleService - .processLeave(memberId, null, warband, true)); + .processLeave(memberId, null, warband, false)); battles.verifyNoMoreInteractions(); } } @Test - void aPlaceKeptAfterQuittingIsDroppedOnlyForThatCharacter() { - Warband warband = createOpenWarband("alpha", leaderId, memberId); - CharacterRef ref = warband.getMemberCharacter(memberId).orElseThrow(); - service.handleQuit(member); - assertFalse(service.removeFromFactionWarband(new CharacterRef(memberId, UUID.randomUUID()))); + void aPlaceKeptInAFactionHostIsDroppedOnlyForThatCharacter() throws Exception { + Warband host = createOpenWarband("host", leaderId, memberId); + markAsFaction(host); + CharacterRef ref = host.getMemberCharacter(memberId).orElseThrow(); + Faction faction = mock(Faction.class); + when(faction.getId()).thenReturn("realm"); + try (var factions = mockStatic(net.tfminecraft.simplefactions.managers.FactionManager.class)) { + factions.when(() -> net.tfminecraft.simplefactions.managers.FactionManager.getByMember(ref.characterId())) + .thenReturn(faction); + service.handleQuit(member); + } + assertTrue(service.getPendingRejoin(memberId).hasFaction()); + assertFalse(service.removeFromFactionWarband(new CharacterRef(memberId, UUID.randomUUID()), true)); assertNotNull(service.getPendingRejoin(memberId)); - assertFalse(service.removeFromFactionWarband(ref)); + assertFalse(service.removeFromFactionWarband(ref, true)); assertNull(service.getPendingRejoin(memberId)); } + @Test + void aPlaceKeptInAPlayersOwnWarbandSurvivesLeavingTheFaction() { + Warband own = createOpenWarband("alpha", leaderId, memberId); + CharacterRef ref = own.getMemberCharacter(memberId).orElseThrow(); + service.handleQuit(member); + assertFalse(service.removeFromFactionWarband(ref, true)); + assertNotNull(service.getPendingRejoin(memberId)); + } + private Warband createOpenWarband(String id, UUID leader, UUID member) { Warband warband = Warband.createWithMemberIds(id, leader, false, member); warband.addMember(member, Members.session(this.member).orElseThrow().ref()); From 5b5a30a1a5a65166810c6dd53a9d7a0202ba3580 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sun, 25 Oct 2026 10:09:40 +0100 Subject: [PATCH 6/6] fix: confirm only the faction that was previewed; console guild completion - A previewed capital move holds the faction itself. If that faction was deleted and refounded under the same id before /faction admin confirm, the new faction is left alone. - The console's /guild completion offers admin (the only /guild command it can run), and guild id completion skips missing guilds. Co-Authored-By: Claude Opus 5.5 --- .../admin/StaffAdminCommand.java | 2 +- .../admin/StaffAdminService.java | 13 +++++---- .../simplefactions/utils/TabCompletion.java | 4 +++ .../admin/StaffAdminCommandTest.java | 4 +-- .../admin/StaffAdminServiceTest.java | 27 ++++++++++++------- .../managers/TabCompletionCoverageTest.java | 8 ++++++ 6 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java index 29ab567a..57632ac5 100644 --- a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminCommand.java @@ -237,7 +237,7 @@ public static List complete(CommandSender sender, String command, String if (args.length == 3) { List ids = new ArrayList<>(); if (guild) { - for (Guild each : FactionManager.getAllGuilds()) ids.add(each.getId()); + for (Guild each : FactionManager.getAllGuilds()) if (each != null) ids.add(each.getId()); } else { for (Faction each : FactionManager.getCopy()) ids.add(each.getId()); } diff --git a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java index a7d73b91..8c523ebf 100644 --- a/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java +++ b/src/main/java/net/tfminecraft/simplefactions/admin/StaffAdminService.java @@ -33,8 +33,11 @@ public final class StaffAdminService { private StaffAdminService() {} - /** A move checked by {@link #previewCapital}, applied by {@link #moveCapital} once staff confirm. */ - public record CapitalMove(String factionId, int fromCapital, int province, String name) {} + /** + * A move checked by {@link #previewCapital}, applied by {@link #moveCapital} once staff confirm. + * It holds the faction itself, so a faction deleted and refounded under the same id is not touched. + */ + public record CapitalMove(Faction faction, int fromCapital, int province, String name) {} /** Display name as the player commands store it: {@code _} for spaces, default colour, hex codes. */ static String displayName(String input) { @@ -199,13 +202,13 @@ public static CapitalMove previewCapital(Player staff, Faction faction, int prov ? faction.getProvinceHandler().previewProvincesLostIfCapitalMoved(province) : List.of(); if (!lost.isEmpty()) lines.add("§c" + faction.getName() + " §cwould lose provinces cut off from the new capital: " + lost); lines.add("§7It is free and does not use up their free capital change. Run §e/faction admin confirm §7to apply it."); - return new CapitalMove(faction.getId(), faction.getCapital(), province, existing != null ? null : name); + return new CapitalMove(faction, faction.getCapital(), province, existing != null ? null : name); } /** Applies a confirmed move if nothing changed since the preview. */ public static List moveCapital(Player staff, CapitalMove move, int standing) { - Faction faction = FactionManager.getByString(move.factionId()); - if (faction == null) return List.of("§cThat faction no longer exists. Nothing changed."); + Faction faction = move.faction(); + if (FactionManager.getByString(faction.getId()) != faction) return List.of("§cThat faction no longer exists. Nothing changed."); if (faction.getCapital() != move.fromCapital()) return List.of("§cTheir capital changed since the preview. Nothing changed."); if (standing != move.province()) return List.of("§cStand in province " + move.province() + " §cto confirm. Nothing changed."); if (!faction.hasCapital() || faction.getCapital() == move.province()) { diff --git a/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java b/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java index c79e877e..a7a39746 100644 --- a/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java +++ b/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java @@ -222,6 +222,10 @@ private List complete(CommandSender sender, Command cmd, String[] args) } return completions; } + // The console can run /guild admin, and nothing else under /guild. + if(cmd.getName().equalsIgnoreCase("guild") && args.length < 2 && !(sender instanceof Player)) { + return Permissions.isAdmin(sender) ? List.of(StaffAdminCommand.SUBCOMMAND) : List.of(); + } if(cmd.getName().equalsIgnoreCase("faction") && sender instanceof Player && Permissions.isAdmin(sender) && args.length == 2 && (args[0].equalsIgnoreCase("delbank") || args[0].equalsIgnoreCase("startelection") diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java index bcffa2fc..42d5a176 100644 --- a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminCommandTest.java @@ -221,7 +221,7 @@ void settlementCommandsNeedProvinces() { @Test void capitalMovesArePreviewedThenConfirmedOnce() { - StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", 1, 5, "Sunsora_Castle"); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove(faction, 1, 5, "Sunsora_Castle"); rest.when(() -> RestServer.getProvince(staff)).thenReturn(5); service.when(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), eq("Sunsora Castle"), any())) .thenAnswer(call -> { @@ -241,7 +241,7 @@ void capitalMovesArePreviewedThenConfirmedOnce() { @Test void aRefusedOrStalePreviewCannotBeConfirmed() { - StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", 1, 5, null); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove(faction, 1, 5, null); rest.when(() -> RestServer.getProvince(staff)).thenReturn(5); service.when(() -> StaffAdminService.previewCapital(eq(staff), eq(faction), eq(5), any(), any())).thenReturn(move); run(staff, "faction", "admin", "movecapital", "sunsora"); diff --git a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java index b2fbbc1a..65baa47b 100644 --- a/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/admin/StaffAdminServiceTest.java @@ -279,7 +279,7 @@ void aCapitalWithoutItsSettlementIsFoundedAgainInPlace() { // Founding in place needs no character: it does not go through the treasury. members.when(() -> Members.session(staff)).thenReturn(Optional.empty()); StaffAdminService.CapitalMove move = StaffAdminService.previewCapital(staff, faction, 395, "Sunsora_Castle", lines); - assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 395, "Sunsora_Castle"), move); + assertEquals(new StaffAdminService.CapitalMove(faction, 395, 395, "Sunsora_Castle"), move); assertEquals("§eFound " + StaffAdminService.displayName("Sunsora_Castle") + " §ein the capital of Sunsora §e(province 395)", lines.get(0)); assertEquals(2, lines.size()); verify(provinces, never()).previewProvincesLostIfCapitalMoved(395); @@ -297,7 +297,7 @@ void capitalPreviewDescribesTheMoveAndAnyLostLand() { when(settlements.validateFactionCapital(staff, 393, "Sunsora_Castle")).thenReturn(CapitalResult.ok("ok")); when(provinces.previewProvincesLostIfCapitalMoved(393)).thenReturn(List.of(12)); StaffAdminService.CapitalMove move = StaffAdminService.previewCapital(staff, faction, 393, "Sunsora_Castle", lines); - assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 393, "Sunsora_Castle"), move); + assertEquals(new StaffAdminService.CapitalMove(faction, 395, 393, "Sunsora_Castle"), move); assertEquals("§eMove the capital of Sunsora §efrom province 395 §eto province 393 §e(new settlement " + StaffAdminService.displayName("Sunsora_Castle") + "§e)", lines.get(0)); assertEquals("§cSunsora §cwould lose provinces cut off from the new capital: [12]", lines.get(1)); @@ -308,7 +308,7 @@ void capitalPreviewDescribesTheMoveAndAnyLostLand() { when(settlements.getByProvince(393)).thenReturn(port); when(settlements.validateFactionCapital(staff, 393, null)).thenReturn(CapitalResult.ok("ok")); when(provinces.previewProvincesLostIfCapitalMoved(393)).thenReturn(List.of()); - assertEquals(new StaffAdminService.CapitalMove("sunsora", 395, 393, null), + assertEquals(new StaffAdminService.CapitalMove(faction, 395, 393, null), StaffAdminService.previewCapital(staff, faction, 393, null, lines)); assertEquals("§eMove the capital of Sunsora §efrom province 395 §eto province 393 §e(Port§e)", lines.get(0)); assertEquals(2, lines.size()); @@ -317,16 +317,25 @@ void capitalPreviewDescribesTheMoveAndAnyLostLand() { when(faction.hasCapital()).thenReturn(false); when(faction.getCapital()).thenReturn(-1); when(settlements.validateFactionCapital(staff, 393, "Ignored")).thenReturn(CapitalResult.ok("ok")); - assertEquals(new StaffAdminService.CapitalMove("sunsora", -1, 393, null), + assertEquals(new StaffAdminService.CapitalMove(faction, -1, 393, null), StaffAdminService.previewCapital(staff, faction, 393, "Ignored", lines)); assertEquals("§eMove the capital of Sunsora §efrom nowhere §eto province 393 §e(Port§e)", lines.get(0)); } @Test void capitalMoveChecksNothingChangedSincePreview() { - StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("gone", 395, 393, null); - assertEquals(List.of("§cThat faction no longer exists. Nothing changed."), StaffAdminService.moveCapital(staff, move, 393)); - move = new StaffAdminService.CapitalMove("sunsora", 395, 393, null); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove(faction, 395, 393, null); + when(faction.getCapital()).thenReturn(395); + String gone = "§cThat faction no longer exists. Nothing changed."; + // Deleted, then refounded under the same id with the same capital: the new faction is left alone. + factions.when(() -> FactionManager.getByString("sunsora")).thenReturn(null); + assertEquals(List.of(gone), StaffAdminService.moveCapital(staff, move, 393)); + Faction refounded = mock(Faction.class); + when(refounded.getCapital()).thenReturn(395); + factions.when(() -> FactionManager.getByString("sunsora")).thenReturn(refounded); + assertEquals(List.of(gone), StaffAdminService.moveCapital(staff, move, 393)); + verify(refounded, never()).getSettlementHandler(); + factions.when(() -> FactionManager.getByString("sunsora")).thenReturn(faction); when(faction.getCapital()).thenReturn(1); assertEquals(List.of("§cTheir capital changed since the preview. Nothing changed."), StaffAdminService.moveCapital(staff, move, 393)); when(faction.getCapital()).thenReturn(395); @@ -336,7 +345,7 @@ void capitalMoveChecksNothingChangedSincePreview() { @Test void aFirstCapitalIsFoundedInPlace() { when(faction.getCapital()).thenReturn(-1); - StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", -1, 393, "Sunsora_Castle"); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove(faction, -1, 393, "Sunsora_Castle"); when(settlements.applyFactionCapital(staff, 393, "Sunsora_Castle")).thenReturn(CapitalResult.fail("§cNo")); assertEquals(List.of("§cNo"), StaffAdminService.moveCapital(staff, move, 393)); verify(faction, never()).setCapital(393); @@ -350,7 +359,7 @@ void aFirstCapitalIsFoundedInPlace() { void anExistingCapitalMovesFreeThroughTheTreasuryWithoutCountingAMove() { when(faction.hasCapital()).thenReturn(true); when(faction.getCapital()).thenReturn(395); - StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove("sunsora", 395, 393, null); + StaffAdminService.CapitalMove move = new StaffAdminService.CapitalMove(faction, 395, 393, null); try (MockedStatic topology = mockStatic(TreasuryTopology.class)) { topology.when(() -> TreasuryTopology.capital(eq(staff), eq(faction), eq(393), isNull(), eq(false), eq(0.0), anyString(), eq(false))).thenReturn(false); diff --git a/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java b/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java index 3b872a5a..382e4b38 100644 --- a/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/managers/TabCompletionCoverageTest.java @@ -852,6 +852,14 @@ void staffConsoleDiscoversOnlySupportedFactionRoots() { assertEquals(List.of("tradegraph"), complete(console, "faction", "trad")); assertTrue(complete(console, "faction", "").contains("admin")); assertFalse(complete(console, "faction", "").contains("create")); + // /guild admin also works from the console, so it is offered there; nothing else is. + assertEquals(List.of("admin"), complete(console, "guild", "")); + assertEquals(List.of("admin"), complete(console, "guild", "AD")); + assertTrue(complete(console, "guild", "cr").isEmpty()); + factions.when(FactionManager::getAllGuilds).thenReturn(Arrays.asList(guild, null)); + assertEquals(List.of("merchants"), complete(console, "guild", "admin", "info", "mer")); + CommandSender visitor = mock(CommandSender.class); + assertTrue(complete(visitor, "guild", "").isEmpty()); Cache.provincesEnabled = false; assertTrue(complete(console, "faction", "tit").isEmpty()); }