Cover configuration and faction workflows and preserve state on failure

src/main/java/net/tfminecraft/simplefactions/Cache.java

@@ -17,6 +17,8 @@
 import net.tfminecraft.simplefactions.war.battle.enums.DefenderRespawnMode;
 
 public class Cache {
+    private Cache() {}
+
 	public static String mapRef;
 	/** Chapter slug for a future Archive prefill. Not the live upload folder. */
 	public static String chapterId = "unknown";

src/main/java/net/tfminecraft/simplefactions/army/Military.java

@@ -273,9 +273,9 @@ public List<LevyEntry> getLevies() {
 				if(total+count >= subject.getMembers().size()) {
 					count = subject.getMembers().size() - total;
 				}
-				total+=count;
-				if(count > 0) r.setSentToOverlord(count);
-				else continue;
+				count = Math.max(0, count);
+				total += count;
+				r.setSentToOverlord(count);
 			}
 			levies.add(new LevyEntry(subject, total));
 			for(LevyEntry e : subjectLevies) {

src/main/java/net/tfminecraft/simplefactions/database/JsonUtil.java

@@ -9,6 +9,7 @@
 import com.google.gson.GsonBuilder;
 
 public class JsonUtil {
+    private JsonUtil() {}
     public static final Gson GSON = new GsonBuilder().setPrettyPrinting().create();
 
     public static <T> T readJson(File file, Class<T> type) throws IOException {

src/main/java/net/tfminecraft/simplefactions/diplomacy/DiplomacyQueries.java

@@ -26,9 +26,6 @@ public static boolean hasOfficialRelation(Faction origin, Faction target) {
 			return true;
 		}
 		DiplomacyHandler handler = origin.getDiplomacyHandler();
-		if (handler == null) {
-			return false;
-		}
 		return handler.hasTradeRelation(target.getId()) || handler.hasTreatyRelation(target.getId());
 	}
 

src/main/java/net/tfminecraft/simplefactions/diplomacy/Threshold.java

@@ -10,6 +10,10 @@ public class Threshold {
 
 	public Threshold(ConfigurationSection config) {
 		type = config.getString("mode", "higher_than_or_equal_to");
+		if (!type.equalsIgnoreCase("lower_than_or_equal_to")
+				&& !type.equalsIgnoreCase("higher_than_or_equal_to")) {
+			throw new IllegalArgumentException("Unknown opinion threshold mode: " + type);
+		}
 		opinion = config.getInt("amount", 20);
 		mutual = config.getBoolean("mutual", false);
 	}
@@ -23,26 +27,14 @@ public boolean isMutual() {
 	}
 
 	public boolean fulfilled(int i) {
-		if(type.equalsIgnoreCase("lower_than_or_equal_to")) {
-			if(i > opinion) return false;
-		}
-		else if(type.equalsIgnoreCase("higher_than_or_equal_to")) {
-			if(i < opinion) return false;
-		}
-		return true;
+		return type.equalsIgnoreCase("lower_than_or_equal_to") ? i <= opinion : i >= opinion;
 	}
-	
+
 	public String getFormattedType() {
 		return (new String(type)).replace("_", " ");
 	}
 
 	public String getFormattedShort() {
-		if(type.equalsIgnoreCase("lower_than_or_equal_to")) {
-			return "<=";
-		}
-		else if(type.equalsIgnoreCase("higher_than_or_equal_to")) {
-			return ">=";
-		}
-		return "=";
+		return type.equalsIgnoreCase("lower_than_or_equal_to") ? "<=" : ">=";
 	}
 }

src/main/java/net/tfminecraft/simplefactions/espionage/CharacterNames.java

@@ -89,13 +89,20 @@ private static String activeName(String playerName) {
         if (Files.isDirectory(folder)) {
             try (var files = Files.list(folder)) {
                 for (Path file : files.filter(path -> path.toString().endsWith(".json")).sorted().toList()) {
-                    JsonObject json = JsonUtil.GSON.fromJson(Files.readString(file), JsonObject.class);
-                    if (json != null && json.has("active") && json.get("active").getAsBoolean() && json.has("name")) {
-                        result = json.get("name").getAsString();
-                        break;
+                    try {
+                        JsonObject json = JsonUtil.GSON.fromJson(Files.readString(file), JsonObject.class);
+                        if (json != null && json.has("active") && json.get("active").isJsonPrimitive()
+                                && json.get("active").getAsBoolean() && json.has("name")
+                                && json.get("name").isJsonPrimitive() && json.getAsJsonPrimitive("name").isString()
+                                && !json.get("name").getAsString().isBlank()) {
+                            result = json.get("name").getAsString();
+                            break;
+                        }
+                    } catch (java.io.IOException | com.google.gson.JsonParseException | IllegalStateException ignored) {
+                        // A damaged older record must not hide a later active character.
                     }
                 }
-            } catch (java.io.IOException | com.google.gson.JsonParseException | IllegalStateException ignored) {
+            } catch (java.io.IOException ignored) {
                 // Cache the absence too; account-oriented callers supply their own fallback.
             }
         }

src/main/java/net/tfminecraft/simplefactions/espionage/EspionageCommands.java

@@ -53,7 +53,10 @@ public static boolean handle(Player player, String[] args) {
             } else if (args[0].equalsIgnoreCase("positions")) EspionageView.positions(player, faction, manager);
             else EspionageView.spymasterOffice(player, faction, manager);
         } else if (args[0].equalsIgnoreCase("spymaster") && args.length >= 2 && !args[1].equalsIgnoreCase("sabotage")) {
-            if (args[1].equalsIgnoreCase("remove")) EspionageService.remove(player, faction);
+            if (args[1].equalsIgnoreCase("remove")) {
+                if (args.length == 2) EspionageService.remove(player, faction);
+                else usage(player);
+            }
             else {
                 var candidate = CharacterNames.resolveOnline(player, String.join(" ", java.util.Arrays.copyOfRange(args, 1, args.length)));
                 if (candidate != null) EspionageService.appoint(player, faction, candidate);

src/main/java/net/tfminecraft/simplefactions/espionage/EspionageService.java

@@ -256,7 +256,7 @@ public static boolean appoint(Player actor, Faction faction, Player candidate) {
             return false;
         }
         String characterId = OfficeCharacters.activeCharacterId(candidate);
-        if (characterId == null) {
+        if (characterId == null || OfficeCharacters.isDead(candidate.getUniqueId(), characterId)) {
             actor.sendMessage("§cThat member needs an active roleplay character.");
             return false;
         }

src/main/java/net/tfminecraft/simplefactions/government/Council.java

@@ -73,8 +73,6 @@ public void reorganize() {
                 case ELECTED_COUNCIL:
                     // keep until next election
                     break;
-                default:
-                    break;
             }
 
             proposalHandler.clearProposals();
@@ -144,10 +142,13 @@ public boolean canBeMember(String name, boolean ignoreSize, boolean ignoreRefuse
         for(Guild guild : f.getGuildHandler().getGuilds()) {
             if(guild.isLeader(name)) return true;
         }
-        return true;
+        return false;
     }
 
     public boolean canRemainMember(String name) {
+        if (refuses.contains(name)) {
+            return false;
+        }
         if (f.getLeader().equalsIgnoreCase(name)) {
             return false;
         }
@@ -272,7 +273,7 @@ private void cleanupCouncil() {
                 getMembers().remove(member);
             }
         }
-        for(String refuser : refuses) {
+        for(String refuser : new ArrayList<>(refuses)) {
             if(!canBeMember(refuser, true, true)) {
                 refuses.remove(refuser);
             }

src/main/java/net/tfminecraft/simplefactions/government/Government.java

@@ -177,7 +177,7 @@ public void endMovement(Movement movement) {
 
     public Movement getMovementByLeader(String leader) {
         for (Movement movement : movements) {
-            if (movement.getLeader().equalsIgnoreCase(leader)) {
+            if (movement.getLeader() != null && movement.getLeader().equalsIgnoreCase(leader)) {
                 return movement;
             }
         }
@@ -709,14 +709,10 @@ public String getTimeUntilElectionEnds() {
 
 
     public boolean hasElections(Candidate type) {
-        switch (type) {
-            case LEADER:
-                return hasLeaderElections();
-            case COUNCIL:
-                return hasCouncilElections();
-            default:
-                return false;
-        }
+        return switch (type) {
+            case LEADER -> hasLeaderElections();
+            case COUNCIL -> hasCouncilElections();
+        };
     }
 
     public boolean hasElection() {
@@ -1010,22 +1006,17 @@ public void toggleRepress(Guild guild) {
     }
 
     public void validateFavoursAndRepressions() {
+        // Repression takes precedence if saved state contains both mutually exclusive flags.
         for(Guild guild : f.getGuildHandler().getGuilds()) {
             if(guild.isFavoured() && !canFavour(guild)) {
                 guild.setFavoured(false);
             }
-            if(guild.isRepressed() && !canRepress(guild)) {
-                guild.setRepressed(false);
-            }
         }
         for(Faction vassal : f.getSubjects()) {
             Guild main = vassal.getOrCreateMainGuild();
             if(main.isFavoured() && !canFavour(main)) {
                 main.setFavoured(false);
             }
-            if(main.isRepressed() && !canRepress(main)) {
-                main.setRepressed(false);
-            }
         }
     }
 }

src/main/java/net/tfminecraft/simplefactions/government/election/Election.java

@@ -54,6 +54,7 @@ public void end() {
         this.active = false;
         validateVotes();
         for(Candidate c : Candidate.values()) {
+            previousVotes.get(c).clear();
             for(String candidate : candidates.get(c)) {
                 previousVotes.get(c).put(candidate, getVotes(c, candidate));
             }
@@ -98,7 +99,8 @@ public void validateVotes() {
             if(!gov.hasElections(c)) continue;
             Map<String, String> voteMap = votes.get(c);
             for(String voter : new ArrayList<>(voteMap.keySet())) {
-                if(!canVote(voter)) {
+                // Existing ballots are valid even though their voter cannot submit a second ballot.
+                if(!eligibleVoters.contains(voter) || !gov.getFaction().canVote(voter)) {
                     voteMap.remove(voter);
                 } else {
                     String candidate = voteMap.get(voter);
@@ -249,10 +251,9 @@ public boolean canBeCandidate(Candidate type, String player, boolean includeAlre
                 break;
             case COUNCIL:
                 if(candidates.get(Candidate.COUNCIL).contains(player) && includeAlreadySignedUp) return false;
+                if(!gov.getCouncil().canRemainMember(player)) return false;
                 if(otherCandidateExists(Candidate.COUNCIL, player)) return false;
                 break;
-            default:
-                return true;
         }
         return true;
     }

src/main/java/net/tfminecraft/simplefactions/government/handler/ProposalHandler.java

@@ -8,7 +8,9 @@
 import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
 
-import net.tfminecraft.simplefactions.loaders.LawLoader;
+import net.tfminecraft.simplefactions.loaders.PoliticalActionLoader;
+import net.tfminecraft.simplefactions.government.movement.Action;
+import net.tfminecraft.simplefactions.government.movement.PoliticalAction;
 import net.tfminecraft.simplefactions.government.Government;
 import net.tfminecraft.simplefactions.government.proposal.FeeChange;
 import net.tfminecraft.simplefactions.government.proposal.FeeKind;
@@ -68,7 +70,8 @@ public boolean canBeProposed(Proposal proposal) {
             for(Proposal p : proposals) {
                 if(!p.isTaxProposal()) continue;
                 TaxLawChange c = p.getTaxChange();
-                if(c.getTarget().equals(change.getTarget()) && c.getId().equalsIgnoreCase(change.getId())) return false;
+                if(c.getTarget().equals(change.getTarget()) && (c.getId() == null
+                        ? change.getId() == null : c.getId().equalsIgnoreCase(change.getId()))) return false;
             }
         }
         return true;
@@ -107,6 +110,9 @@ public List<String> serializeProposals() {
                 FeeChange fee = p.getFeeChange();
                 result.add(p.getProposer() + ":fee:" + fee.getKind().name() + ":"
                         + (fee.isGeneral() ? ALL_VEHICLES : fee.getVehicleTypeId()) + ":" + fee.getNewRate());
+            } else if (p.isPoliticalActionProposal()) {
+                result.add(p.getProposer() + ":action:" + p.getPoliticalAction().getAction().name()
+                        + ":" + (p.getTarget() == null ? "" : p.getTarget()));
             }
         }
         return result;
@@ -115,8 +121,11 @@ public List<String> serializeProposals() {
     public void restoreProposals(net.tfminecraft.simplefactions.objects.Faction faction, List<String> serialized) {
         proposals.clear();
         for (String s : serialized) {
-            String proposer = s.split(":", 2)[0];
-            s = s.substring(proposer.length() + 1);
+            if (s == null) continue;
+            int separator = s.indexOf(':');
+            if (separator <= 0) continue;
+            String proposer = s.substring(0, separator);
+            s = s.substring(separator + 1);
             if (s.startsWith("law:")) {
                 String[] parts = s.substring(4).split(":");
                 if (parts.length >= 2) {
@@ -143,20 +152,35 @@ public void restoreProposals(net.tfminecraft.simplefactions.objects.Faction fact
                         FeeKind kind = FeeKind.valueOf(body.substring(0, first));
                         String typeField = body.substring(first + 1, last);
                         String type = ALL_VEHICLES.equals(typeField) ? null : typeField;
+                        double newRate = Double.parseDouble(body.substring(last + 1));
+                        if (!Double.isFinite(newRate)) continue;
                         Proposal p = new Proposal(proposer, gov);
-                        p.setFeeProposal(new FeeChange(kind, type, Double.parseDouble(body.substring(last + 1))));
+                        p.setFeeProposal(new FeeChange(kind, type, newRate));
                         proposals.add(p);
                     } catch (Exception e) {
                         // Skip malformed proposals
                     }
                 }
+            } else if (s.startsWith("action:")) {
+                String[] parts = s.substring(7).split(":", 2);
+                try {
+                    Action kind = Action.valueOf(parts[0]);
+                    PoliticalAction action = PoliticalActionLoader.getByAction(kind);
+                    Proposal proposal = new Proposal(proposer, gov);
+                    proposal.setPoliticalActionProposal(action != null ? action : new PoliticalAction(kind));
+                    if (parts.length == 2 && !parts[1].isEmpty()) proposal.setTarget(parts[1]);
+                    proposals.add(proposal);
+                } catch (IllegalArgumentException e) {
+                    // Skip actions that do not exist in this version.
+                }
             } else if (s.startsWith("tax:")) {
                 String[] parts = s.substring(4).split(":");
                 if (parts.length >= 3) {
                     try {
                         net.tfminecraft.simplefactions.government.proposal.TaxTarget target = net.tfminecraft.simplefactions.government.proposal.TaxTarget.valueOf(parts[0]);
-                        String taxId = parts[1];
+                        String taxId = "null".equals(parts[1]) ? null : parts[1];
                         double newRate = Double.parseDouble(parts[2]);
+                        if (!Double.isFinite(newRate)) continue;
 
                         TaxLawChange tax = new TaxLawChange(target, taxId, newRate);
                         Proposal p = new Proposal(proposer, gov);

src/main/java/net/tfminecraft/simplefactions/government/movement/CoupService.java

@@ -26,9 +26,6 @@ public static void apply(Faction faction, String wantedLeader) {
 
 	private static void adjustCouncil(Faction faction) {
 		LawHandler laws = faction.getLawHandler();
-		if (laws == null) {
-			return;
-		}
 		LawGroup group = laws.getGroup(GOVERNMENT_GROUP);
 		if (group == null || group.getCurrent() == null || group.getCurrent().getId() == null) {
 			return;
@@ -52,9 +49,6 @@ private static void adjustCouncil(Faction faction) {
 
 	private static void clearCouncil(Faction faction) {
 		Government government = faction.getGovernment();
-		if (government == null || government.getCouncil() == null) {
-			return;
-		}
 		Council council = government.getCouncil();
 		council.clearMembers();
 	}

src/main/java/net/tfminecraft/simplefactions/government/movement/Movement.java

@@ -207,9 +207,6 @@ public String foreignBackerBlockReason(Faction faction, boolean staff) {
         if (RelationManager.sameRealm(faction, f)) {
             return MovementJoinCopy.backerSameRealm(staff, faction, f);
         }
-        if (faction.getId().equalsIgnoreCase(f.getId())) {
-            return MovementJoinCopy.backerOwnFaction(staff, f);
-        }
         for (Movement otherMovement : f.getGovernment().getMovements()) {
             if (otherMovement.getId().equals(this.id)) continue;
             if (otherMovement.getForeignBackers().contains(faction)) {
@@ -328,6 +325,9 @@ public boolean canJoin(Object obj, Cause cause) {
 
     public void join(Object obj, Cause cause) {
         if (frozen) return;
+        if (obj instanceof String citizen && isMember(citizen)) return;
+        if (obj instanceof Guild guild && isMember(guild.getLeader())) return;
+        if (obj instanceof Faction faction && isMember(faction.getLeader())) return;
         double power = getPower();
         if(cause == null) {
             // Joining as general supporter
@@ -370,13 +370,7 @@ public void leave(Object obj, Cause cause) {
     }
 
     public void joinAsForeignBacker(Faction backer) {
-        if (!canForeignBackerJoin(backer, false)) return;
-
-        // Check if already backing another movement in this faction
-        for (Movement otherMovement : f.getGovernment().getMovements()) {
-            if (otherMovement.getId().equals(this.id)) continue;
-            if (otherMovement.getForeignBackers().contains(backer)) return;
-        }
+        if (foreignBackers.contains(backer) || !canForeignBackerJoin(backer, false)) return;
 
         foreignBackers.add(backer);
     }
@@ -426,7 +420,7 @@ public List<Cause> getCauses() {
 
     public Cause getCauseByLeader(String leaderName) {
         for (Cause cause : causes) {
-            if (cause.getLeader().equalsIgnoreCase(leaderName)) {
+            if (cause.getLeader() != null && cause.getLeader().equalsIgnoreCase(leaderName)) {
                 return cause;
             }
         }