From e7f44790049a4d1187367826eb6949af92affe74 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:31:54 +0200 Subject: [PATCH 1/3] Add staff commands to edit faction titles /faction title lets staff (simplefactions.admin) list and inspect titles, see which titles a province belongs to, rename titles, change their map colour, move provinces between titles, move lower titles between parents, and toggle title-complete. Edits apply live, are saved back to Input/.json, and queue the affected web map regions. Works from the console too. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../simplefactions/loaders/TitleLoader.java | 58 ++++ .../managers/CommandManager.java | 5 + .../simplefactions/tiers/Title.java | 14 +- .../tiers/admin/TitleAdminCommand.java | 284 ++++++++++++++++++ .../tiers/admin/TitleAdminService.java | 275 +++++++++++++++++ .../simplefactions/utils/TabCompletion.java | 8 + .../loaders/TitleLoaderSaveTest.java | 64 ++++ .../tiers/admin/TitleAdminServiceTest.java | 187 ++++++++++++ 8 files changed, 894 insertions(+), 1 deletion(-) create mode 100644 src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java create mode 100644 src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java create mode 100644 src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java diff --git a/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java b/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java index df9a94cf..ba17869a 100644 --- a/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java +++ b/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java @@ -8,6 +8,8 @@ import java.io.*; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.*; public class TitleLoader { @@ -80,6 +82,62 @@ public static List getByTier(Tier tier){ return list; } + /** Writes one title's current state back to its tier file, keeping the other entries and their order. */ + public static boolean saveTitle(Title title) { + return saveTitle(title, inputFolder); + } + + static boolean saveTitle(Title title, File folder) { + File file = new File(folder, title.getTier().getId().toLowerCase() + ".json"); + JsonObject root = new JsonObject(); + if (file.exists()) { + try (Reader reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)) { + root = JsonParser.parseReader(reader).getAsJsonObject(); + } catch (Exception e) { + // Never overwrite a file we could not read: that would drop every other title in the tier. + e.printStackTrace(); + return false; + } + } + JsonObject entry = root.has(title.getId()) && root.get(title.getId()).isJsonObject() + ? root.getAsJsonObject(title.getId()) + : new JsonObject(); + writeEntry(entry, title); + root.add(title.getId(), entry); + + File tmp = new File(folder, file.getName() + ".tmp"); + try { + folder.mkdirs(); + try (Writer writer = new OutputStreamWriter(new FileOutputStream(tmp), StandardCharsets.UTF_8)) { + new GsonBuilder().setPrettyPrinting().create().toJson(root, writer); + } + Files.move(tmp.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING); + return true; + } catch (IOException e) { + e.printStackTrace(); + tmp.delete(); + return false; + } + } + + static void writeEntry(JsonObject entry, Title title) { + if (title.getName() != null) entry.addProperty("name", title.getName()); + if (title.getRgb() != null) entry.addProperty("rgb", title.getRgb()); + if (entry.has("title-complete") || title.isTitleComplete()) { + entry.addProperty("title-complete", String.valueOf(title.isTitleComplete())); + } + if (entry.has("provinces") || !title.getProvinces().isEmpty()) { + JsonArray provinceArray = new JsonArray(); + for (int provinceId : title.getProvinces()) provinceArray.add(provinceId); + entry.add("provinces", provinceArray); + } + if (entry.has("titles") || !title.getTitles().isEmpty()) { + JsonArray titleArray = new JsonArray(); + for (String titleId : title.getTitles()) titleArray.add(titleId); + entry.add("titles", titleArray); + } + } + //Create new public static Title createNewTitle(Tier tier, String id, String name, String rgb, List<Integer> provinces, List<String> usedTitles, boolean titleComplete) { diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java index edac8194..504954b7 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/CommandManager.java @@ -39,6 +39,7 @@ import net.tfminecraft.simplefactions.installation.handler.ConstructResult; import net.tfminecraft.simplefactions.settlement.handler.CapitalResult; import net.tfminecraft.simplefactions.tiers.Title; +import net.tfminecraft.simplefactions.tiers.admin.TitleAdminCommand; import net.tfminecraft.simplefactions.utils.DisplayNameGate; import net.tfminecraft.simplefactions.utils.DisplayNameGate.NameOperation; import net.tfminecraft.simplefactions.utils.Formatter; @@ -60,6 +61,10 @@ public class CommandManager implements Listener, CommandExecutor{ @Override public boolean onCommand(CommandSender sender, Command cmd, String label, String[] args) { + // Title editing is staff-only and also works from the console. + if(cmd.getName().equalsIgnoreCase(cmd1) && args.length >= 1 && args[0].equalsIgnoreCase(TitleAdminCommand.SUBCOMMAND)) { + return TitleAdminCommand.handle(sender, args); + } if(sender instanceof Player) { Player p = (Player) sender; if((cmd.getName().equalsIgnoreCase(cmd1) || cmd.getName().equalsIgnoreCase(cmd2)) && args.length < 1) { diff --git a/src/main/java/net/tfminecraft/simplefactions/tiers/Title.java b/src/main/java/net/tfminecraft/simplefactions/tiers/Title.java index 9875d8a0..633c5ced 100644 --- a/src/main/java/net/tfminecraft/simplefactions/tiers/Title.java +++ b/src/main/java/net/tfminecraft/simplefactions/tiers/Title.java @@ -84,7 +84,19 @@ public List<Integer> getProvinces() { public List<String> getTitles() { return titles; } - + + public void setName(String name) { + this.name = name; + } + + public void setRgb(String rgb) { + this.rgb = rgb; + } + + public void setTitleComplete(boolean titleComplete) { + this.titleComplete = titleComplete; + } + public boolean canGrant(Faction f) { return f.getTitles(tier).size() >= 2; } diff --git a/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java new file mode 100644 index 00000000..fc9a3f01 --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java @@ -0,0 +1,284 @@ +package net.tfminecraft.simplefactions.tiers.admin; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; + +import org.bukkit.ChatColor; +import org.bukkit.command.CommandSender; + +import net.tfminecraft.simplefactions.Cache; +import net.tfminecraft.simplefactions.SimpleFactions; +import net.tfminecraft.simplefactions.loaders.TierLoader; +import net.tfminecraft.simplefactions.loaders.TitleLoader; +import net.tfminecraft.simplefactions.managers.FactionManager; +import net.tfminecraft.simplefactions.managers.TitleManager; +import net.tfminecraft.simplefactions.objects.Faction; +import net.tfminecraft.simplefactions.tiers.Tier; +import net.tfminecraft.simplefactions.tiers.Title; +import net.tfminecraft.simplefactions.utils.Permissions; + +/** {@code /faction title ...}: staff-only editing of title names, colours and de jure contents. */ +public final class TitleAdminCommand { + public static final String SUBCOMMAND = "title"; + public static final List<String> SUBCOMMANDS = List.of("list", "info", "where", "rename", "setcolour", + "addprovince", "removeprovince", "addtitle", "removetitle", "setcomplete"); + private static final String[] USAGE = { + "§6/faction title list [tier] §7- list titles", + "§6/faction title info <title> §7- show a title's parts, parent and holder", + "§6/faction title where <province> §7- show which titles a province belongs to", + "§6/faction title rename <title> <name...> §7- change the display name", + "§6/faction title setcolour <title> <R,G,B> §7- change the map colour", + "§6/faction title addprovince <title> <province...> §7- add or move provinces into a title", + "§6/faction title removeprovince <title> <province...> §7- leave provinces untitled", + "§6/faction title addtitle <title> <lower title> §7- add or move a lower title into a title", + "§6/faction title removetitle <title> <lower title> §7- take a lower title out", + "§6/faction title setcomplete <title> <true|false> §7- require every part to form it", + }; + + private TitleAdminCommand() { + } + + public static boolean handle(CommandSender sender, String[] args) { + if (!Permissions.isAdmin(sender)) { + sender.sendMessage("§a[SimpleFactions]§c You do not have access to this command"); + return true; + } + if (!Cache.requireProvinces(sender)) { + return true; + } + if (args.length < 2) { + sender.sendMessage(USAGE); + return true; + } + String sub = args[1].toLowerCase(); + switch (sub) { + case "list" -> list(sender, args); + case "info" -> { + if (args.length != 3) sender.sendMessage(usage("info")); + else info(sender, args[2]); + } + case "where" -> { + if (args.length != 3) sender.sendMessage(usage("where")); + else where(sender, args[2]); + } + case "rename" -> { + if (args.length < 4) sender.sendMessage(usage("rename")); + else apply(sender, TitleAdminService.rename(args[2], String.join(" ", Arrays.copyOfRange(args, 3, args.length)))); + } + case "setcolour", "setcolor" -> { + if (args.length < 4) sender.sendMessage(usage("setcolour")); + else apply(sender, TitleAdminService.setColour(args[2], String.join("", Arrays.copyOfRange(args, 3, args.length)))); + } + case "addprovince" -> { + if (args.length < 4) sender.sendMessage(usage("addprovince")); + else apply(sender, TitleAdminService.addProvinces(args[2], Arrays.asList(args).subList(3, args.length), + TitleAdminCommand::provinceExists)); + } + case "removeprovince" -> { + if (args.length < 4) sender.sendMessage(usage("removeprovince")); + else apply(sender, TitleAdminService.removeProvinces(args[2], Arrays.asList(args).subList(3, args.length))); + } + case "addtitle" -> { + if (args.length != 4) sender.sendMessage(usage("addtitle")); + else apply(sender, TitleAdminService.addTitle(args[2], args[3])); + } + case "removetitle" -> { + if (args.length != 4) sender.sendMessage(usage("removetitle")); + else apply(sender, TitleAdminService.removeTitle(args[2], args[3])); + } + case "setcomplete" -> { + if (args.length != 4) sender.sendMessage(usage("setcomplete")); + else apply(sender, TitleAdminService.setComplete(args[2], args[3])); + } + default -> sender.sendMessage(USAGE); + } + return true; + } + + private static String usage(String sub) { + for (String line : USAGE) { + if (line.startsWith("§6/faction title " + sub + " ")) return line; + } + return USAGE[0]; + } + + private static boolean provinceExists(int id) { + return SimpleFactions.getInstance().getProvinceManager().get(id) != null; + } + + private static void apply(CommandSender sender, TitleAdminService.Result result) { + for (String line : result.lines()) { + sender.sendMessage(line); + } + if (!result.ok()) return; + + for (Title title : result.changed()) { + if (!TitleLoader.saveTitle(title)) { + sender.sendMessage("§a[SimpleFactions]§c Could not save " + title.getId() + " to Input/" + + title.getTier().getId().toLowerCase() + ".json. The change is live but will be lost on restart; check the console."); + } + } + for (TitleAdminService.MapKey key : result.regenerate()) { + FactionManager.getMap().enqueue(key.tier(), key.rgb()); + } + if (!result.regenerate().isEmpty()) { + sender.sendMessage("§7The web map updates on its next cycle."); + } + // Holders keep their titles; staff decide whether to use destroytitle/granttitle. + for (Title title : result.changed()) { + Faction owner = TitleManager.getOwner(title); + if (owner == null) continue; + int held = title.getCurrentAmount(owner, TitleManager.getProvinces(owner), TitleManager.getTitles(owner)); + String colour = held > 0 ? "§7" : "§e"; + sender.sendMessage(colour + title.getName() + " is held by " + owner.getName() + colour + ", who controls §f" + + held + "/" + title.getNeededAmount() + colour + " of its " + (title.isComposite() ? "titles" : "provinces") + "."); + } + SimpleFactions.getInstance().getLogger().info("[TitleAdmin] " + sender.getName() + ": " + + ChatColor.stripColor(String.join(" | ", result.lines()))); + } + + private static void list(CommandSender sender, String[] args) { + Tier only = null; + if (args.length >= 3) { + only = TierLoader.getByString(args[2]); + if (only == null) { + sender.sendMessage("§a[SimpleFactions]§c No tier with the id " + args[2]); + return; + } + } + for (Tier tier : TierLoader.get()) { + if (only != null && !tier.getId().equalsIgnoreCase(only.getId())) continue; + List<Title> titles = TitleLoader.getByTier(tier); + if (titles.isEmpty()) continue; + sender.sendMessage(tier.getName() + " §7(" + titles.size() + ")"); + for (Title title : titles) { + Faction owner = TitleManager.getOwner(title); + String parts = title.isComposite() + ? title.getTitles().size() + " titles" + : title.getProvinces().size() + " provinces"; + sender.sendMessage(" §7" + title.getId() + " §f" + title.getName() + " §8(" + parts + ")" + + (owner != null ? " §7held by " + owner.getName() : "")); + } + } + } + + private static void info(CommandSender sender, String id) { + Title title = TitleLoader.getById(id); + if (title == null) { + sender.sendMessage("§a[SimpleFactions]§c No title with the id " + id); + return; + } + sender.sendMessage("§6=== §f" + title.getName() + " §7(" + title.getId() + ") §6==="); + sender.sendMessage("§7Tier: " + title.getTier().getName() + " §7Colour: §f" + title.getRgb() + + " §7Title-complete: §f" + title.isTitleComplete()); + if (!title.getProvinces().isEmpty()) { + sender.sendMessage("§7Provinces: §f" + title.getProvinces()); + } + if (!title.getTitles().isEmpty()) { + List<String> parts = new ArrayList<>(); + for (String childId : title.getTitles()) { + Title child = TitleLoader.getById(childId); + parts.add(child == null ? "§c" + childId + " (missing)§f" : childId + " (" + child.getName() + ")"); + } + sender.sendMessage("§7Titles: §f" + String.join(", ", parts)); + sender.sendMessage("§7All provinces: §f" + TitleManager.getProvinces(title)); + } + Title parent = TitleLoader.getByTitle(title); + sender.sendMessage("§7Part of: §f" + (parent == null ? "none" : parent.getId() + " (" + parent.getName() + ")")); + Faction owner = TitleManager.getOwner(title); + sender.sendMessage("§7Holder: §f" + (owner == null ? "none" : owner.getName())); + } + + private static void where(CommandSender sender, String raw) { + int province; + try { + province = Integer.parseInt(raw); + } catch (NumberFormatException e) { + sender.sendMessage("§a[SimpleFactions]§c Province ids must be numbers"); + return; + } + if (!provinceExists(province)) { + sender.sendMessage("§a[SimpleFactions]§c No province with the id " + province); + return; + } + List<String> chain = new ArrayList<>(); + Set<Title> seen = new LinkedHashSet<>(); + for (Title t = TitleLoader.getByProvince(province); t != null && seen.add(t); t = TitleLoader.getByTitle(t)) { + chain.add(t.getId() + " (" + t.getName() + ")"); + } + Faction owner = FactionManager.getByProvince(province); + sender.sendMessage("§7Province §f" + province + "§7: " + (chain.isEmpty() ? "§funtitled" : "§f" + String.join(" §7→ §f", chain))); + sender.sendMessage("§7Controlled by: §f" + (owner == null ? "nobody" : owner.getName())); + } + + public static List<String> complete(String[] args) { + List<String> completions = new ArrayList<>(); + if (args.length == 2) { + completions.addAll(SUBCOMMANDS); + return filtered(completions, args[1]); + } + String sub = args[1].toLowerCase(); + if (args.length == 3) { + if (sub.equals("list")) { + for (Tier tier : TierLoader.get()) { + if (!TitleLoader.getByTier(tier).isEmpty()) completions.add(tier.getId()); + } + } else if (sub.equals("where")) { + completions.add("<province>"); + } else if (SUBCOMMANDS.contains(sub) || sub.equals("setcolor")) { + for (Title title : TitleLoader.getTitles()) { + if (sub.equals("addprovince") && title.isComposite()) continue; + if ((sub.equals("addtitle") || sub.equals("removetitle")) && !title.isComposite()) continue; + completions.add(title.getId()); + } + } + return filtered(completions, args[2]); + } + Title title = TitleLoader.getById(args[2]); + String last = args[args.length - 1]; + switch (sub) { + case "rename" -> { + if (args.length == 4) completions.add("<name>"); + } + case "setcolour", "setcolor" -> { + if (args.length == 4) completions.add(title != null && title.getRgb() != null ? title.getRgb() : "R,G,B"); + } + case "addprovince" -> completions.add("<province>"); + case "removeprovince" -> { + if (title != null) { + for (int province : title.getProvinces()) completions.add(String.valueOf(province)); + } + } + case "addtitle" -> { + if (args.length == 4 && title != null) { + for (Title child : TitleLoader.getTitles()) { + if (child.getTier().getTier() == title.getTier().getTier() - 1 && !title.getTitles().contains(child.getId())) { + completions.add(child.getId()); + } + } + } + } + case "removetitle" -> { + if (args.length == 4 && title != null) completions.addAll(title.getTitles()); + } + case "setcomplete" -> { + if (args.length == 4) { + completions.add("true"); + completions.add("false"); + } + } + default -> { + } + } + return filtered(completions, last); + } + + private static List<String> filtered(List<String> completions, String prefix) { + String lower = prefix == null ? "" : prefix.toLowerCase(); + completions.removeIf(s -> !s.toLowerCase().startsWith(lower)); + return completions; + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java new file mode 100644 index 00000000..a74d296f --- /dev/null +++ b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java @@ -0,0 +1,275 @@ +package net.tfminecraft.simplefactions.tiers.admin; + +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.function.IntPredicate; + +import net.tfminecraft.simplefactions.loaders.TitleLoader; +import net.tfminecraft.simplefactions.tiers.Title; + +/** + * Staff edits to the title definitions in Input/<tier>.json. Title ids never change here: + * faction saves, wars and the web map all reference them. + */ +public final class TitleAdminService { + public static final String PREFIX = "§a[SimpleFactions]§c "; + public static final int MAX_NAME_LENGTH = 48; + + private TitleAdminService() { + } + + /** A web map region to regenerate: the title's tier and its rgb key. */ + public record MapKey(String tier, String rgb) { + } + + public record Result(boolean ok, List<String> lines, Set<Title> changed, Set<MapKey> regenerate) { + static Result error(String message) { + return new Result(false, List.of(PREFIX + message), Set.of(), Set.of()); + } + } + + private static final class Change { + private final List<String> lines = new ArrayList<>(); + private final Set<Title> changed = new LinkedHashSet<>(); + private final Set<MapKey> regenerate = new LinkedHashSet<>(); + + void touch(Title title) { + changed.add(title); + // Parents cover the moved provinces too. The set guards against a hand-edited cycle. + Set<Title> seen = new LinkedHashSet<>(); + for (Title t = title; t != null && seen.add(t); t = TitleLoader.getByTitle(t)) { + regenerate.add(key(t)); + } + } + + Result done() { + return new Result(true, lines, changed, regenerate); + } + } + + static MapKey key(Title title) { + return new MapKey(title.getTier().getId(), title.getRgb()); + } + + public static Result rename(String id, String rawName) { + Title title = TitleLoader.getById(id); + if (title == null) return Result.error("No title with the id " + id); + String name = rawName == null ? "" : rawName.trim().replaceAll("\\s+", " "); + if (name.isEmpty()) return Result.error("The name cannot be empty"); + if (name.contains("§")) return Result.error("Names cannot contain § colour codes"); + if (name.length() > MAX_NAME_LENGTH) return Result.error("Names can be at most " + MAX_NAME_LENGTH + " characters"); + String old = title.getName(); + if (name.equals(old)) return Result.error(title.getId() + " is already called " + name); + title.setName(name); + Change change = new Change(); + change.touch(title); + change.lines.add("§aRenamed §7" + title.getId() + " §afrom §f" + old + " §ato §f" + name); + return change.done(); + } + + public static Result setColour(String id, String rawRgb) { + Title title = TitleLoader.getById(id); + if (title == null) return Result.error("No title with the id " + id); + String rgb = parseRgb(rawRgb); + if (rgb == null) return Result.error("Colour must be R,G,B with each value 0-255, e.g. 200,168,80"); + String old = title.getRgb(); + if (rgb.equals(old)) return Result.error(title.getId() + " already uses " + rgb); + for (Title other : TitleLoader.getByTier(title.getTier())) { + if (other != title && rgb.equals(other.getRgb())) { + return Result.error(other.getId() + " already uses " + rgb + "; colours must be unique within a tier"); + } + } + Change change = new Change(); + // The web map keys regions by rgb, so the old colour's overlay must be cleared too. + if (old != null) change.regenerate.add(new MapKey(title.getTier().getId(), old)); + title.setRgb(rgb); + change.touch(title); + change.lines.add("§aSet the colour of §f" + title.getName() + " §7(" + title.getId() + ") §afrom §7" + old + " §ato §f" + rgb); + return change.done(); + } + + public static Result setComplete(String id, String rawValue) { + Title title = TitleLoader.getById(id); + if (title == null) return Result.error("No title with the id " + id); + if (!"true".equalsIgnoreCase(rawValue) && !"false".equalsIgnoreCase(rawValue)) { + return Result.error("Value must be true or false"); + } + boolean value = Boolean.parseBoolean(rawValue.toLowerCase()); + if (title.isTitleComplete() == value) return Result.error(title.getId() + " title-complete is already " + value); + title.setTitleComplete(value); + Change change = new Change(); + change.changed.add(title); + change.lines.add("§aSet title-complete of §f" + title.getName() + " §7(" + title.getId() + ") §ato §f" + value + + (value ? " §7(forming needs every de jure part)" : " §7(forming uses the de jure percentage)")); + return change.done(); + } + + public static Result addProvinces(String id, List<String> rawProvinces, IntPredicate provinceExists) { + Title title = TitleLoader.getById(id); + if (title == null) return Result.error("No title with the id " + id); + if (title.isComposite()) return Result.error(title.getId() + " is made of titles, not provinces. Use addtitle"); + List<Integer> provinces = parseProvinces(rawProvinces, provinceExists); + if (provinces == null) return Result.error(invalidProvinces(rawProvinces, provinceExists)); + + // Validate the whole batch first so a rejected province leaves nothing half-applied. + List<Integer> toAdd = new ArrayList<>(); + for (int province : provinces) { + if (title.getProvinces().contains(province) || toAdd.contains(province)) continue; + Title previous = TitleLoader.getByProvince(province); + if (previous != null) { + long left = previous.getProvinces().stream().filter(p -> !provinces.contains(p)).count(); + if (left == 0 && previous.getTitles().isEmpty()) { + return Result.error("Moving province " + province + " would leave " + previous.getId() + " with no provinces"); + } + } + toAdd.add(province); + } + if (toAdd.isEmpty()) return Result.error(title.getId() + " already has " + (provinces.size() == 1 ? "that province" : "those provinces")); + + Change change = new Change(); + for (int province : toAdd) { + Title previous = TitleLoader.getByProvince(province); + if (previous != null) { + previous.getProvinces().remove(Integer.valueOf(province)); + change.touch(previous); + change.lines.add("§eMoved province §f" + province + " §efrom §f" + previous.getName() + " §7(" + previous.getId() + ")"); + } else { + change.lines.add("§aAdded province §f" + province); + } + title.getProvinces().add(province); + } + change.touch(title); + change.lines.add("§a" + title.getName() + " §7(" + title.getId() + ") §anow has provinces §f" + title.getProvinces()); + return change.done(); + } + + public static Result removeProvinces(String id, List<String> rawProvinces) { + Title title = TitleLoader.getById(id); + if (title == null) return Result.error("No title with the id " + id); + List<Integer> provinces = parseProvinces(rawProvinces, p -> true); + if (provinces == null) return Result.error(invalidProvinces(rawProvinces, p -> true)); + for (int province : provinces) { + if (!title.getProvinces().contains(province)) { + return Result.error("Province " + province + " is not part of " + title.getId()); + } + } + long left = title.getProvinces().stream().filter(p -> !provinces.contains(p)).count(); + if (left == 0 && title.getTitles().isEmpty()) { + return Result.error("That would leave " + title.getId() + " with no provinces"); + } + for (int province : provinces) { + title.getProvinces().remove(Integer.valueOf(province)); + } + Change change = new Change(); + change.touch(title); + change.lines.add("§aRemoved provinces §f" + provinces + " §afrom §f" + title.getName() + " §7(" + title.getId() + ")" + + " §7- they are now untitled"); + return change.done(); + } + + public static Result addTitle(String id, String childId) { + Title parent = TitleLoader.getById(id); + if (parent == null) return Result.error("No title with the id " + id); + Title child = TitleLoader.getById(childId); + if (child == null) return Result.error("No title with the id " + childId); + if (!parent.isComposite()) return Result.error(parent.getId() + " is made of provinces, not titles. Use addprovince"); + if (child.getTier().getTier() != parent.getTier().getTier() - 1) { + return Result.error(child.getId() + " is a " + child.getTier().getId() + "; " + parent.getId() + + " can only contain titles one tier below it"); + } + if (containsId(parent.getTitles(), child.getId())) return Result.error(parent.getId() + " already contains " + child.getId()); + Title previous = TitleLoader.getByTitle(child); + if (previous != null && previous.getTitles().size() <= 1 && previous.getProvinces().isEmpty()) { + return Result.error("Moving " + child.getId() + " would leave " + previous.getId() + " empty"); + } + Change change = new Change(); + if (previous != null) { + change.touch(previous); + removeId(previous.getTitles(), child.getId()); + change.lines.add("§eMoved §f" + child.getName() + " §7(" + child.getId() + ") §efrom §f" + previous.getName() + + " §7(" + previous.getId() + ")"); + } + parent.getTitles().add(child.getId()); + change.touch(parent); + change.lines.add("§a" + parent.getName() + " §7(" + parent.getId() + ") §anow contains §f" + parent.getTitles()); + return change.done(); + } + + public static Result removeTitle(String id, String childId) { + Title parent = TitleLoader.getById(id); + if (parent == null) return Result.error("No title with the id " + id); + if (!containsId(parent.getTitles(), childId)) return Result.error(childId + " is not part of " + parent.getId()); + if (parent.getTitles().size() <= 1 && parent.getProvinces().isEmpty()) { + return Result.error("That would leave " + parent.getId() + " empty"); + } + Change change = new Change(); + change.touch(parent); + removeId(parent.getTitles(), childId); + change.lines.add("§aRemoved §f" + childId + " §afrom §f" + parent.getName() + " §7(" + parent.getId() + ")"); + return change.done(); + } + + /** Normalises "R,G,B" (spaces allowed) to "r,g,b", or returns null. */ + static String parseRgb(String raw) { + if (raw == null) return null; + String[] parts = raw.split(","); + if (parts.length != 3) return null; + StringBuilder out = new StringBuilder(); + for (String part : parts) { + int value; + try { + value = Integer.parseInt(part.trim()); + } catch (NumberFormatException e) { + return null; + } + if (value < 0 || value > 255) return null; + if (out.length() > 0) out.append(','); + out.append(value); + } + return out.toString(); + } + + /** Accepts space- or comma-separated ids; null when any id is malformed or unknown. */ + static List<Integer> parseProvinces(List<String> raw, IntPredicate provinceExists) { + List<Integer> out = new ArrayList<>(); + for (String arg : raw) { + for (String part : arg.split(",")) { + if (part.isBlank()) continue; + int province; + try { + province = Integer.parseInt(part.trim()); + } catch (NumberFormatException e) { + return null; + } + if (!provinceExists.test(province)) return null; + if (!out.contains(province)) out.add(province); + } + } + return out.isEmpty() ? null : out; + } + + private static String invalidProvinces(List<String> raw, IntPredicate provinceExists) { + for (String arg : raw) { + for (String part : arg.split(",")) { + if (part.isBlank()) continue; + try { + int province = Integer.parseInt(part.trim()); + if (!provinceExists.test(province)) return "No province with the id " + province; + } catch (NumberFormatException e) { + return "Province ids must be numbers: " + part.trim(); + } + } + } + return "Give at least one province id"; + } + + private static boolean containsId(List<String> ids, String id) { + return ids.stream().anyMatch(s -> s.equalsIgnoreCase(id)); + } + + private static void removeId(List<String> ids, String id) { + ids.removeIf(s -> s.equalsIgnoreCase(id)); + } +} diff --git a/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java b/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java index 202689a4..24c11f4c 100644 --- a/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java +++ b/src/main/java/net/tfminecraft/simplefactions/utils/TabCompletion.java @@ -23,6 +23,7 @@ import net.tfminecraft.simplefactions.objects.Faction; import net.tfminecraft.simplefactions.objects.request.MercenaryInviteRequest; import net.tfminecraft.simplefactions.tiers.Title; +import net.tfminecraft.simplefactions.tiers.admin.TitleAdminCommand; import net.tfminecraft.simplefactions.installation.Installation; import net.tfminecraft.simplefactions.installation.InstallationKind; import net.tfminecraft.simplefactions.laws.LawGroup; @@ -162,6 +163,12 @@ private List<String> completeInstallationIds(Player p, String prefix, boolean in @Override public List<String> onTabComplete (CommandSender sender, Command cmd, String label, String[] args){ + if(cmd.getName().equalsIgnoreCase("faction") && args.length >= 2 && args[0].equalsIgnoreCase(TitleAdminCommand.SUBCOMMAND)) { + if(!Permissions.isAdmin(sender) || !Cache.provincesEnabled) { + return new ArrayList<>(); + } + return TitleAdminCommand.complete(args); + } if(cmd.getName().equalsIgnoreCase("company")) { if(sender instanceof Player p) { return completeCompany(p, args); @@ -329,6 +336,7 @@ else if(cmd.getName().equalsIgnoreCase("faction") && args.length >= 0 && args.le completions.add("destroytitle"); completions.add("granttitle"); completions.add("usurp"); + completions.add(TitleAdminCommand.SUBCOMMAND); } completions.add("reloadconfigs"); completions.add("transfersubject"); diff --git a/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java b/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java new file mode 100644 index 00000000..39c9624b --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java @@ -0,0 +1,64 @@ +package net.tfminecraft.simplefactions.loaders; + +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 java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + +import net.tfminecraft.simplefactions.tiers.Tier; +import net.tfminecraft.simplefactions.tiers.Title; + +class TitleLoaderSaveTest { + @TempDir + Path folder; + + @Test + void saveKeepsOtherEntriesTheirOrderAndUnknownFields() throws Exception { + Files.writeString(folder.resolve("county.json"), + "{\"COUNTY_1\": {\"name\": \"Urseilos\", \"provinces\": [3, 1], \"rgb\": \"1,1,1\", \"note\": \"keep me\"}," + + " \"COUNTY_2\": {\"name\": \"Ardentos\", \"provinces\": [4], \"rgb\": \"2,2,2\"}}", + StandardCharsets.UTF_8); + Title title = title("{\"name\":\"Urseilos\",\"provinces\":[3,1],\"rgb\":\"1,1,1\"}"); + title.setName("New Urseilos"); + title.getProvinces().add(9); + + assertTrue(TitleLoader.saveTitle(title, folder.toFile())); + + JsonObject root = JsonParser.parseString(Files.readString(folder.resolve("county.json"))).getAsJsonObject(); + assertEquals(List.of("COUNTY_1", "COUNTY_2"), List.copyOf(root.keySet())); + JsonObject saved = root.getAsJsonObject("COUNTY_1"); + assertEquals("New Urseilos", saved.get("name").getAsString()); + assertEquals("[3,1,9]", saved.get("provinces").toString()); + assertEquals("keep me", saved.get("note").getAsString()); + assertFalse(saved.has("title-complete")); + assertFalse(saved.has("titles")); + assertEquals("Ardentos", root.getAsJsonObject("COUNTY_2").get("name").getAsString()); + assertFalse(Files.exists(folder.resolve("county.json.tmp"))); + } + + @Test + void saveRefusesToOverwriteAnUnreadableFile() throws Exception { + Files.writeString(folder.resolve("county.json"), "{ not json", StandardCharsets.UTF_8); + + assertFalse(TitleLoader.saveTitle(title("{\"name\":\"Urseilos\",\"provinces\":[1],\"rgb\":\"1,1,1\"}"), folder.toFile())); + assertEquals("{ not json", Files.readString(folder.resolve("county.json"))); + } + + private static Title title(String json) { + YamlConfiguration config = new YamlConfiguration(); + config.set("name", "county"); + config.set("tier", 2); + return new Title(new Tier("county", config), "COUNTY_1", JsonParser.parseString(json).getAsJsonObject()); + } +} diff --git a/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java new file mode 100644 index 00000000..8b07afe1 --- /dev/null +++ b/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java @@ -0,0 +1,187 @@ +package net.tfminecraft.simplefactions.tiers.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 java.util.List; +import java.util.Set; +import java.util.function.IntPredicate; + +import org.bukkit.configuration.file.YamlConfiguration; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import com.google.gson.JsonParser; + +import net.tfminecraft.simplefactions.loaders.TitleLoader; +import net.tfminecraft.simplefactions.tiers.Tier; +import net.tfminecraft.simplefactions.tiers.Title; +import net.tfminecraft.simplefactions.tiers.admin.TitleAdminService.MapKey; +import net.tfminecraft.simplefactions.tiers.admin.TitleAdminService.Result; + +class TitleAdminServiceTest { + private static final IntPredicate ANY_PROVINCE = p -> p > 0 && p < 1000; + + private Title county1; + private Title county2; + private Title county3; + private Title duchy1; + private Title duchy2; + + @BeforeEach + void setUp() { + Tier county = tier("county", 2); + Tier duchy = tier("duchy", 3); + county1 = title(county, "COUNTY_1", "{\"name\":\"Urseilos\",\"rgb\":\"1,1,1\",\"provinces\":[1,2,3]}"); + county2 = title(county, "COUNTY_2", "{\"name\":\"Ardentos\",\"rgb\":\"2,2,2\",\"provinces\":[4]}"); + county3 = title(county, "COUNTY_3", "{\"name\":\"Vedrasos\",\"rgb\":\"3,3,3\",\"provinces\":[5,6]}"); + duchy1 = title(duchy, "DUCHY_1", "{\"name\":\"Atrarcha\",\"rgb\":\"1,1,1\",\"titles\":[\"COUNTY_1\",\"COUNTY_2\"]}"); + duchy2 = title(duchy, "DUCHY_2", "{\"name\":\"Heliovera\",\"rgb\":\"9,9,9\",\"titles\":[\"COUNTY_3\"]}"); + TitleLoader.getTitles().clear(); + TitleLoader.getTitles().addAll(List.of(county1, county2, county3, duchy1, duchy2)); + } + + @AfterEach + void tearDown() { + TitleLoader.getTitles().clear(); + } + + @Test + void renameCollapsesWhitespaceAndQueuesItsRegion() { + Result result = TitleAdminService.rename("county_1", " New Urseilos "); + + assertTrue(result.ok()); + assertEquals("New Urseilos", county1.getName()); + assertEquals(Set.of(county1), result.changed()); + assertTrue(result.regenerate().contains(new MapKey("county", "1,1,1"))); + assertTrue(result.regenerate().contains(new MapKey("duchy", "1,1,1"))); + } + + @Test + void renameRejectsColourCodesAndBlankNames() { + assertFalse(TitleAdminService.rename("COUNTY_1", "§cRed").ok()); + assertFalse(TitleAdminService.rename("COUNTY_1", " ").ok()); + assertFalse(TitleAdminService.rename("MISSING", "Name").ok()); + assertEquals("Urseilos", county1.getName()); + } + + @Test + void setColourNormalisesAndClearsTheOldRegion() { + Result result = TitleAdminService.setColour("COUNTY_1", "10, 20 ,30"); + + assertTrue(result.ok()); + assertEquals("10,20,30", county1.getRgb()); + assertTrue(result.regenerate().contains(new MapKey("county", "1,1,1"))); + assertTrue(result.regenerate().contains(new MapKey("county", "10,20,30"))); + } + + @Test + void setColourMustBeUniqueWithinATierOnly() { + assertFalse(TitleAdminService.setColour("COUNTY_1", "2,2,2").ok()); + assertEquals("1,1,1", county1.getRgb()); + // A duchy may reuse a county's colour: the map keeps each tier separate. + assertTrue(TitleAdminService.setColour("DUCHY_2", "3,3,3").ok()); + assertFalse(TitleAdminService.setColour("COUNTY_1", "256,0,0").ok()); + assertFalse(TitleAdminService.setColour("COUNTY_1", "1,2").ok()); + } + + @Test + void addProvinceMovesItFromItsOldTitle() { + Result result = TitleAdminService.addProvinces("COUNTY_3", List.of("1"), ANY_PROVINCE); + + assertTrue(result.ok()); + assertEquals(List.of(2, 3), county1.getProvinces()); + assertEquals(List.of(5, 6, 1), county3.getProvinces()); + assertEquals(Set.of(county1, county3), result.changed()); + assertTrue(result.regenerate().containsAll(Set.of( + new MapKey("county", "1,1,1"), new MapKey("county", "3,3,3"), + new MapKey("duchy", "1,1,1"), new MapKey("duchy", "9,9,9")))); + } + + @Test + void addProvinceAcceptsUntitledAndCommaSeparatedIds() { + Result result = TitleAdminService.addProvinces("COUNTY_2", List.of("7,8", "9"), ANY_PROVINCE); + + assertTrue(result.ok()); + assertEquals(List.of(4, 7, 8, 9), county2.getProvinces()); + } + + @Test + void addProvinceRejectsTheWholeBatchWhenOneWouldEmptyATitle() { + Result result = TitleAdminService.addProvinces("COUNTY_1", List.of("5", "4"), ANY_PROVINCE); + + assertFalse(result.ok()); + assertEquals(List.of(1, 2, 3), county1.getProvinces()); + assertEquals(List.of(4), county2.getProvinces()); + assertEquals(List.of(5, 6), county3.getProvinces()); + } + + @Test + void addProvinceRejectsUnknownProvincesAndCompositeTitles() { + assertFalse(TitleAdminService.addProvinces("COUNTY_1", List.of("5000"), ANY_PROVINCE).ok()); + assertFalse(TitleAdminService.addProvinces("COUNTY_1", List.of("abc"), ANY_PROVINCE).ok()); + assertFalse(TitleAdminService.addProvinces("DUCHY_1", List.of("7"), ANY_PROVINCE).ok()); + assertFalse(TitleAdminService.addProvinces("COUNTY_1", List.of("1"), ANY_PROVINCE).ok()); + } + + @Test + void removeProvinceKeepsAtLeastOne() { + assertFalse(TitleAdminService.removeProvinces("COUNTY_2", List.of("4")).ok()); + assertFalse(TitleAdminService.removeProvinces("COUNTY_1", List.of("9")).ok()); + + Result result = TitleAdminService.removeProvinces("COUNTY_1", List.of("1", "3")); + assertTrue(result.ok()); + assertEquals(List.of(2), county1.getProvinces()); + } + + @Test + void addTitleMovesALowerTitleBetweenParents() { + Result result = TitleAdminService.addTitle("DUCHY_2", "county_2"); + + assertTrue(result.ok()); + assertEquals(List.of("COUNTY_1"), duchy1.getTitles()); + assertEquals(List.of("COUNTY_3", "COUNTY_2"), duchy2.getTitles()); + assertEquals(Set.of(duchy1, duchy2), result.changed()); + } + + @Test + void addTitleChecksTiersAndEmptyParents() { + assertFalse(TitleAdminService.addTitle("DUCHY_1", "DUCHY_2").ok()); + assertFalse(TitleAdminService.addTitle("COUNTY_1", "COUNTY_2").ok()); + assertFalse(TitleAdminService.addTitle("DUCHY_1", "COUNTY_3").ok()); + assertFalse(TitleAdminService.addTitle("DUCHY_1", "COUNTY_1").ok()); + } + + @Test + void removeTitleKeepsAtLeastOne() { + assertFalse(TitleAdminService.removeTitle("DUCHY_2", "COUNTY_3").ok()); + assertFalse(TitleAdminService.removeTitle("DUCHY_1", "COUNTY_3").ok()); + + Result result = TitleAdminService.removeTitle("DUCHY_1", "county_2"); + assertTrue(result.ok()); + assertEquals(List.of("COUNTY_1"), duchy1.getTitles()); + } + + @Test + void setCompleteNeedsABoolean() { + assertFalse(TitleAdminService.setComplete("COUNTY_1", "yes").ok()); + Result result = TitleAdminService.setComplete("COUNTY_1", "TRUE"); + assertTrue(result.ok()); + assertTrue(county1.isTitleComplete()); + assertTrue(result.regenerate().isEmpty()); + assertFalse(TitleAdminService.setComplete("COUNTY_1", "true").ok()); + } + + private static Tier tier(String id, int level) { + YamlConfiguration config = new YamlConfiguration(); + config.set("name", id); + config.set("tier", level); + return new Tier(id, config); + } + + private static Title title(Tier tier, String id, String json) { + return new Title(tier, id, JsonParser.parseString(json).getAsJsonObject()); + } +} From 431233bdbaaf618f0d744a1cb5a600fe5a0091de Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sun, 27 Sep 2026 14:39:12 +0200 Subject: [PATCH 2/3] Reject unknown province ids in title commands ProvinceManager.get falls back to an empty Province, so it never returned null and any id passed the existence check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- .../tfminecraft/simplefactions/managers/ProvinceManager.java | 5 +++++ .../simplefactions/tiers/admin/TitleAdminCommand.java | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java b/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java index 98a1b375..e9e96256 100644 --- a/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java +++ b/src/main/java/net/tfminecraft/simplefactions/managers/ProvinceManager.java @@ -50,6 +50,11 @@ public Province get(int id) { return provinces.getOrDefault(id, new Province()); } + /** {@link #get} never returns null, so use this to check an id against provinces.txt. */ + public boolean contains(int id) { + return provinces.containsKey(id); + } + public void start(Map<Integer, Province> map) { provinces = map; } diff --git a/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java index fc9a3f01..c78899a9 100644 --- a/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java +++ b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminCommand.java @@ -106,7 +106,7 @@ private static String usage(String sub) { } private static boolean provinceExists(int id) { - return SimpleFactions.getInstance().getProvinceManager().get(id) != null; + return SimpleFactions.getInstance().getProvinceManager().contains(id); } private static void apply(CommandSender sender, TitleAdminService.Result result) { From db14cd5cfa56121b66c2bf4517ee1976ca60ad39 Mon Sep 17 00:00:00 2001 From: XxFran10xX <318299142+XxFran10xX@users.noreply.github.com> Date: Sun, 27 Sep 2026 18:54:25 +0200 Subject: [PATCH 3/3] Address review: save failures, null fields, parent id case - Return false (and remove the temp file) when Gson fails to write. - Keep null-valued fields in other entries when rewriting a tier file. - Find a lower title's old parent with case-insensitive id matching, so a mixed-case id in the JSON can't leave it with two parents. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --- .../tfminecraft/simplefactions/loaders/TitleLoader.java | 4 ++-- .../simplefactions/tiers/admin/TitleAdminService.java | 8 +++++++- .../simplefactions/loaders/TitleLoaderSaveTest.java | 3 ++- .../tiers/admin/TitleAdminServiceTest.java | 9 +++++++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java b/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java index ba17869a..02ecd518 100644 --- a/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java +++ b/src/main/java/net/tfminecraft/simplefactions/loaders/TitleLoader.java @@ -109,11 +109,11 @@ static boolean saveTitle(Title title, File folder) { try { folder.mkdirs(); try (Writer writer = new OutputStreamWriter(new FileOutputStream(tmp), StandardCharsets.UTF_8)) { - new GsonBuilder().setPrettyPrinting().create().toJson(root, writer); + new GsonBuilder().serializeNulls().setPrettyPrinting().create().toJson(root, writer); } Files.move(tmp.toPath(), file.toPath(), StandardCopyOption.REPLACE_EXISTING); return true; - } catch (IOException e) { + } catch (IOException | JsonIOException e) { e.printStackTrace(); tmp.delete(); return false; diff --git a/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java index a74d296f..32bb9606 100644 --- a/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java +++ b/src/main/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminService.java @@ -180,7 +180,13 @@ public static Result addTitle(String id, String childId) { + " can only contain titles one tier below it"); } if (containsId(parent.getTitles(), child.getId())) return Result.error(parent.getId() + " already contains " + child.getId()); - Title previous = TitleLoader.getByTitle(child); + Title previous = null; + for (Title t : TitleLoader.getTitles()) { + if (t != parent && containsId(t.getTitles(), child.getId())) { + previous = t; + break; + } + } if (previous != null && previous.getTitles().size() <= 1 && previous.getProvinces().isEmpty()) { return Result.error("Moving " + child.getId() + " would leave " + previous.getId() + " empty"); } diff --git a/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java b/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java index 39c9624b..7b60be7c 100644 --- a/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/loaders/TitleLoaderSaveTest.java @@ -27,7 +27,7 @@ class TitleLoaderSaveTest { void saveKeepsOtherEntriesTheirOrderAndUnknownFields() throws Exception { Files.writeString(folder.resolve("county.json"), "{\"COUNTY_1\": {\"name\": \"Urseilos\", \"provinces\": [3, 1], \"rgb\": \"1,1,1\", \"note\": \"keep me\"}," - + " \"COUNTY_2\": {\"name\": \"Ardentos\", \"provinces\": [4], \"rgb\": \"2,2,2\"}}", + + " \"COUNTY_2\": {\"name\": \"Ardentos\", \"provinces\": [4], \"rgb\": \"2,2,2\", \"legacy\": null}}", StandardCharsets.UTF_8); Title title = title("{\"name\":\"Urseilos\",\"provinces\":[3,1],\"rgb\":\"1,1,1\"}"); title.setName("New Urseilos"); @@ -44,6 +44,7 @@ void saveKeepsOtherEntriesTheirOrderAndUnknownFields() throws Exception { assertFalse(saved.has("title-complete")); assertFalse(saved.has("titles")); assertEquals("Ardentos", root.getAsJsonObject("COUNTY_2").get("name").getAsString()); + assertTrue(root.getAsJsonObject("COUNTY_2").has("legacy"), "null-valued fields in other entries are kept"); assertFalse(Files.exists(folder.resolve("county.json.tmp"))); } diff --git a/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java b/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java index 8b07afe1..1ff61d25 100644 --- a/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java +++ b/src/test/java/net/tfminecraft/simplefactions/tiers/admin/TitleAdminServiceTest.java @@ -146,6 +146,15 @@ void addTitleMovesALowerTitleBetweenParents() { assertEquals(Set.of(duchy1, duchy2), result.changed()); } + @Test + void addTitleFindsTheOldParentWhateverTheIdCase() { + duchy1.getTitles().set(1, "county_2"); + + assertTrue(TitleAdminService.addTitle("DUCHY_2", "COUNTY_2").ok()); + assertEquals(List.of("COUNTY_1"), duchy1.getTitles()); + assertEquals(List.of("COUNTY_3", "COUNTY_2"), duchy2.getTitles()); + } + @Test void addTitleChecksTiersAndEmptyParents() { assertFalse(TitleAdminService.addTitle("DUCHY_1", "DUCHY_2").ok());