- Add more nation-joining tests to the NationUtil.

  - Refactor nation adding towns in the NationCommand class.

Towny/src/main/java/com/palmergames/bukkit/towny/command/NationCommand.java

@@ -696,10 +696,10 @@ private void parseNationJoin(Player player, String[] args) {
 			if (!town.hasEnoughResidentsToJoinANation())
 				throw new TownyException(Translatable.of("msg_err_not_enough_residents_join_nation", town.getName()));
 
-			if (!testNationMaxTowns(nation))
+			if (nation.hasReachedMaxTowns())
 				throw new TownyException(Translatable.of("msg_err_nation_over_town_limit", TownySettings.getMaxTownsPerNation()));
 
-			if (!testNationMaxResidents(nation, town))
+			if (!nation.canAddResidents(town.getNumResidents()))
 				throw new TownyException(Translatable.of("msg_err_cannot_join_nation_over_resident_limit", TownySettings.getMaxResidentsPerNation()));
 
 			if (TownySettings.getNationProximityToCapital() > 0)
@@ -708,9 +708,7 @@ private void parseNationJoin(Player player, String[] args) {
 			// Check if the command is not cancelled
 			BukkitTools.ifCancelledThenThrow(new NationPreAddTownEvent(nation, town));
 
-			List<Town> towns = new ArrayList<>();
-			towns.add(town);
-			nationAdd(nation, towns);
+			nationAdd(nation, town);
 
 		} catch (TownyException e) {
 			TownyMessaging.sendErrorMsg(player, e.getMessage(player));
@@ -1176,7 +1174,7 @@ public void nationAdd(Player player, String[] names) throws TownyException {
 
 		Nation nation = getNationFromPlayerOrThrow(player);
 
-		if (!testNationMaxTowns(nation))
+		if (nation.hasReachedMaxTowns())
 			throw new TownyException(Translatable.of("msg_err_nation_over_town_limit", TownySettings.getMaxTownsPerNation()));
 
 		// The list of valid invites.
@@ -1214,7 +1212,7 @@ public void nationAdd(Player player, String[] names) throws TownyException {
 				continue;
 			}
 
-			if (!testNationMaxResidents(nation, town)) {
+			if (!nation.canAddResidents(town.getNumResidents())) {
 				// Town has too many residents to join the nation
 				removeinvites.add(townname);
 				TownyMessaging.sendErrorMsg(player, Translatable.of("msg_err_cannot_join_nation_over_resident_limit", TownySettings.getMaxResidentsPerNation(), townname));
@@ -1290,57 +1288,61 @@ public static void nationAdd(Player player, Nation nation, List<Town> invited) t
 
 	/**
 	 * Final stage of adding towns to a nation.
-	 * @param nation - Nation being added to.
-	 * @param towns - List of Town(s) being added to Nation.
-	 * @throws AlreadyRegisteredException - Shouldn't happen but could.
+	 * 
+	 * @deprecated since 0.100.1.2 use {@link #nationAdd(Nation, Town)} instead.
+	 * @param nation Nation being added to.
+	 * @param towns  List of Town(s) being added to Nation.
 	 */
-	public static void nationAdd(Nation nation, List<Town> towns) throws AlreadyRegisteredException {
-		for (Town town : towns) {
-			if (!town.hasNation()) {
-				if (!testNationMaxTowns(nation)) {
-					// Nation has hit the max-towns limit.
-					TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_nation_over_town_limit", TownySettings.getMaxTownsPerNation()));
-					TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_cannot_join_nation_over_town_limit", TownySettings.getMaxTownsPerNation()));
-					continue;
-				}
-
-				if (!town.hasEnoughResidentsToJoinANation()) {
-					// Town has dropped below min.-residents-to-join-nation limit. 
-					TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_not_enough_residents_join_nation", town.getName()));
-					TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_not_enough_residents_join_nation", town.getName()));
-					continue;
-				}
+	@Deprecated
+	public static void nationAdd(Nation nation, List<Town> towns) {
+		for (Town town : towns)
+			nationAdd(nation, town);
+	}
 
-				if (!testNationMaxResidents(nation, town)) {
-					// Nation has hit the max-residents limit.
-					TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_cannot_add_nation_over_resident_limit", TownySettings.getMaxResidentsPerNation(), town.getName()));
-					TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_cannot_join_nation_over_resident_limit", TownySettings.getMaxResidentsPerNation()));
-					continue;
-				}
+	/**
+	 * Final stage of adding a town to a nation, via joining or via accepting an
+	 * invite. We re-test the rules for joining a nation in case the town or
+	 * nation's situation has changed since being sent the invite/join confirmation.
+	 * 
+	 * @param nation Nation which would take on a new Town.
+	 * @param town   Town which would join the nation.
+	 */
+	public static void nationAdd(Nation nation, Town town) {
+		if (town.hasNation()) {
+			TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_town_already_belong_nation", town.getName(), town.getNationOrNull().getName()));
+			TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_already_belong_nation"));
+			return;
+		}
 
-				town.setNation(nation);
-				town.save();
-				TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_join_nation", StringMgmt.remUnderscore(town.getName())));
-			}
+		if (!town.hasEnoughResidentsToJoinANation()) {
+			// Town has dropped below min.-residents-to-join-nation limit. 
+			TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_not_enough_residents_join_nation", town.getName()));
+			TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_not_enough_residents_join_nation", town.getName()));
+			return;
+		}
 
+		if (nation.hasReachedMaxTowns()) {
+			// Nation has hit the max-towns limit.
+			TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_nation_over_town_limit", TownySettings.getMaxTownsPerNation()));
+			TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_cannot_join_nation_over_town_limit", TownySettings.getMaxTownsPerNation()));
+			return;
 		}
-		plugin.resetCache();
-		nation.save();
 
-	}
-
-	private static boolean testNationMaxResidents(Nation nation, Town town) {
-		int maxResidentPerNation = TownySettings.getMaxResidentsPerNation();
-		if (maxResidentPerNation < 1)
-			return true;
-		return !(nation.getResidents().size() + town.getResidents().size() > maxResidentPerNation);
+		if (!nation.canAddResidents(town.getNumResidents())) {
+			// Nation has hit the max-residents limit.
+			TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_err_cannot_add_nation_over_resident_limit", TownySettings.getMaxResidentsPerNation(), town.getName()));
+			TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_cannot_join_nation_over_resident_limit", TownySettings.getMaxResidentsPerNation()));
+			return;
+		}
 
-	}
+		try {
+			town.setNation(nation);
+		} catch (AlreadyRegisteredException ignored) {}
+		town.save();
+		TownyMessaging.sendPrefixedNationMessage(nation, Translatable.of("msg_join_nation", StringMgmt.remUnderscore(town.getName())));
 
-	private static boolean testNationMaxTowns(Nation nation) {
-		if (TownySettings.getMaxTownsPerNation() < 1)
-			return true;
-		return !(nation.getTowns().size() >= TownySettings.getMaxTownsPerNation());
+		// Reset the town's player cache to account for potential new plot permissions.
+		TownyAPI.getInstance().getOnlinePlayers(town).forEach(p-> plugin.resetCache(p));
 	}
 
 	private static void nationRevokeInviteTown(CommandSender sender, Nation nation, List<Town> towns) {

Towny/src/main/java/com/palmergames/bukkit/towny/object/Nation.java

@@ -330,6 +330,18 @@ public int getNumResidents() {
 		return numResidents;
 	}
 
+	public boolean canAddResidents(int additionalResidents) {
+		return NationUtil.canAddTownsResidentCount(this, additionalResidents);
+	}
+
+	public boolean hasReachedMaxResidents() {
+		return NationUtil.hasReachedMaximumResidents(this);
+	}
+
+	public boolean hasReachedMaxTowns() {
+		return NationUtil.hasReachedMaximumTowns(this);
+	}
+
 	/**
 	 * Should only be called by Town.removeNation();
 	 * Removes town from {@link #towns} list and will choose a
@@ -738,4 +750,6 @@ public void loadSanctionedTowns(String[] tokens) {
 			}
 		}
 	}
+
+
 }

Towny/src/main/java/com/palmergames/bukkit/towny/object/inviteobjects/TownJoinNationInvite.java

@@ -8,9 +8,6 @@
 import com.palmergames.bukkit.towny.object.Translatable;
 import org.bukkit.command.CommandSender;
 
-import java.util.ArrayList;
-import java.util.List;
-
 public class TownJoinNationInvite extends AbstractInvite<Nation, Town> {
 
 	public TownJoinNationInvite(CommandSender directSender, Town receiver, Nation sender) {
@@ -20,14 +17,8 @@ public TownJoinNationInvite(CommandSender directSender, Town receiver, Nation se
 	@Override
 	public void accept() throws TownyException {
 		Town town = getReceiver();
-		List<Town> towns = new ArrayList<>();
-		towns.add(town);
 		Nation nation = getSender();
-		if(!town.hasNation()){
-			NationCommand.nationAdd(nation, towns);
-		} else {
-			TownyMessaging.sendPrefixedTownMessage(town, Translatable.of("msg_err_already_nation", town.getName()));
-		}
+		NationCommand.nationAdd(nation, town);
 		// Message handled in nationAdd()
 		town.deleteReceivedInvite(this);
 		nation.deleteSentInvite(this);

Towny/src/main/java/com/palmergames/bukkit/towny/utils/NationUtil.java

@@ -60,4 +60,22 @@ private static String getTownJoinedNationDate(Town town, Translator translator)
 	public static boolean hasReachedMaximumAllies(Nation nation) {
 		return TownySettings.getMaxNationAllies() >= 0 && nation.getAllies().size() >= TownySettings.getMaxNationAllies();
 	}
+
+	public static boolean hasReachedMaximumResidents(Nation nation) {
+		int maxResidentsPerNation = TownySettings.getMaxResidentsPerNation();
+		return maxResidentsPerNation > 0 && nation.getResidents().size() >= maxResidentsPerNation;
+	}
+
+	public static boolean canAddTownsResidentCount(Nation nation, int additionalResidents) {
+		if (hasReachedMaximumResidents(nation))
+			return false;
+		int maxResidentPerNation = TownySettings.getMaxResidentsPerNation();
+		return maxResidentPerNation > 0 && (nation.getResidents().size() + additionalResidents) >= maxResidentPerNation;
+	}
+
+	public static boolean hasReachedMaximumTowns(Nation nation) {
+		int maxTownsPerNation = TownySettings.getMaxTownsPerNation();
+		return maxTownsPerNation > 0 && nation.getTowns().size() >= maxTownsPerNation;
+	}
+
 }

Towny/src/main/resources/ChangeLog.txt

@@ -9453,4 +9453,6 @@ v0.92.0.11:
   - Fix returning null in the relational placeholder code, returning the no-relation colour instead.
   - Refactor some of TownyFormatter's methods, mainly moving some component-creation out into their related Util classes.
   - Add NationUtil.
-  - Move max allies test into NationUtil.
+  - Move max allies test into NationUtil.
+  - Add more nation-joining tests to the NationUtil.
+  - Refactor nation adding towns in the NationCommand class.

Towny/src/main/resources/lang/en-US.yml

@@ -710,6 +710,8 @@ msg_err_not_in_town_claim: '&cYou must belong to a town in order to claim plots.
 msg_err_must_belong_town: '&cYou must belong to a town.'
 msg_err_dont_belong_town: '&cYou don''t belong to a town.'
 msg_err_dont_belong_nation: '&bYou don''t belong to a nation.'
+msg_err_already_belong_nation: '&bYou already belong to a nation.'
+msg_err_town_already_belong_nation: '&b%s already belongs to a nation: %s.'
 msg_err_not_same_nation: '&b%s doesn''t belong to your nation.'
 msg_err_rect_auto: '&cOnly towns and residents can use auto.'
 msg_err_invalid_radius: '&cInvalid radius. Use an integer or ''auto''.'