Updated rank commads

Updated rank command after feedback from Cayorion.
SHould we use role or rank?

src/main/java/com/massivecraft/factions/cmd/CmdFactionsRank.java

@@ -1,5 +1,8 @@
 package com.massivecraft.factions.cmd;
 
+import java.util.List;
+
+import com.massivecraft.factions.Factions;
 import com.massivecraft.factions.Perm;
 import com.massivecraft.factions.Rel;
 import com.massivecraft.factions.cmd.arg.ARMPlayer;
@@ -9,8 +12,10 @@ import com.massivecraft.factions.entity.MConf;
 import com.massivecraft.factions.entity.MFlag;
 import com.massivecraft.factions.entity.MPlayer;
 import com.massivecraft.factions.entity.MPlayerColl;
-import com.massivecraft.factions.util.RelationUtil;
+import com.massivecraft.factions.event.EventFactionsRankChange;
 import com.massivecraft.massivecore.cmd.req.ReqHasPerm;
+import com.massivecraft.massivecore.util.IdUtil;
+import com.massivecraft.massivecore.util.MUtil;
 import com.massivecraft.massivecore.util.Txt;
 
 public class CmdFactionsRank extends FactionsCommand
@@ -19,7 +24,7 @@ public class CmdFactionsRank extends FactionsCommand
 	// CONSTANTS
 	// -------------------------------------------- //
 	
-	// The rank required to do any rank changes
+	// The rank required to do any rank changes.
 	final static Rel rankReq = Rel.OFFICER;
 	
 	// -------------------------------------------- //
@@ -32,8 +37,8 @@ public class CmdFactionsRank extends FactionsCommand
 	private MPlayer target = null;
 	
 	// Roles
-	private Rel senderRole = null;
+	private Rel senderRank = null;
-	private Rel targetRole = null;
+	private Rel targetRank = null;
 	private Rel rank = null;
 	
 	// -------------------------------------------- //
@@ -43,7 +48,7 @@ public class CmdFactionsRank extends FactionsCommand
 	public CmdFactionsRank()
 	{
 		// Aliases
-		this.addAliases("r","rank");
+		this.addAliases("rank");
 	
 		// Args
 		this.addOptionalArg("player", "you");
@@ -77,7 +82,7 @@ public class CmdFactionsRank extends FactionsCommand
 			return;
 		}
 		
-		// Permission check
+		// Permission check.
 		if ( ! Perm.RANK_ACTION.has(sender, true))
 		{
 			return;
@@ -95,16 +100,16 @@ public class CmdFactionsRank extends FactionsCommand
 			return;
 		}
 		
-		// Should we fire an event when rank is changed?
+		EventFactionsRankChange event = new EventFactionsRankChange(sender, target, rank);
-		// Currently we don't.
+		event.run();
+		if (event.isCancelled()) return;
+		rank = event.getNewRank();
 		
-		// Change the rank
+		// Change the rank.
 		this.changeRank();
 	}
 	
 	// This is always run after performing a MassiveCommand.
-	// It might be a bit hacky, but is easier than adding a line of code at every return statement.
-	// Sometimes it is nice to know the exact mechanics of MassiveCore.
 	@Override
 	public void unsetSenderVars()
 	{
@@ -118,7 +123,7 @@ public class CmdFactionsRank extends FactionsCommand
 
 	private boolean registerFields()
 	{
-		// Getting the target and faction
+		// Getting the target and faction.
 		target = this.arg(0, ARMPlayer.getAny(), msender);
 		if (null == target) return false;
 		targetFaction = target.getFaction();
@@ -130,9 +135,9 @@ public class CmdFactionsRank extends FactionsCommand
 			if (null == rank) return false;
 		}
 		
-		// Roles/ranks
+		// Ranks
-		senderRole = msender.getRole();
+		senderRank = msender.getRole();
-		targetRole = target.getRole();
+		targetRank = target.getRole();
 		
 		return true;
 	}
@@ -142,75 +147,84 @@ public class CmdFactionsRank extends FactionsCommand
 		targetFaction = null;
 		target = null;
 		
-		senderRole = null;
+		senderRank = null;
-		targetRole = null;
+		targetRank = null;
 		rank = null;
 	}
 	
 	private void showRank()
 	{
-		String name = target.describeTo(msender) + (target == msender ? "r" : "'s"); 
+		String targetName = target.describeTo(msender, true);
-		msg(Txt.parse("%s <i>rank is %s", name, target.getColorTo(msender)+Txt.getNicedEnum(target.getRole())));
+		String isAre = target == msender ? "are" : "is";
+		String theAan = targetRank == Rel.LEADER ? "the" : Txt.aan(targetRank.name());
+		String rankName = Txt.getNicedEnum(targetRank).toLowerCase();
+		String ofIn = targetRank == Rel.LEADER ? "of" : "in";
+		String factionName = targetFaction.describeTo(msender, true);
+		if (targetFaction == msenderFaction)
+		{
+			factionName = factionName.toLowerCase();
+		}
+		msg("%s <i>%s %s <h>%s <i>%s %s<i>.", targetName, isAre, theAan, rankName, ofIn, factionName);
 	}
 
 	private boolean isPlayerAllowed()
 	{
-		// People with permission don't follow the normal rules
+		// People with permission don't follow the normal rules.
 		if (msender.isUsingAdminMode())
 		{
 			return true;
 		}
 		
-		// If somone gets the leadership of wilderness (Which has happened before)
+		// If somone gets the leadership of wilderness (Which has happened before).
-		// We can at least try to limit their powers
+		// We can at least try to limit their powers.
 		if (targetFaction.isNone())
 		{
-			msg("<b>Wilderness doesn't use ranks sorry :(");
+			msg("%s <b>doesn't use ranks sorry :(", targetFaction.getName() );
 			return false;
 		}
 		
 		if (targetFaction != msenderFaction)
 		{
-			// Don't change ranks outside of your faction
+			// Don't change ranks outside of your faction.
-			msg(Txt.parse("%s <b>is not in the same faction as you", target.describeTo(msender)));
+			msg("%s <b>is not in the same faction as you.", target.describeTo(msender));
 			return false;
 		}
 		
 		if (target == msender)
 		{
-			// Don't change your own rank
+			// Don't change your own rank.
 			msg("<b>The target player mustn't be yourself.");
 			return false;
 		}
 		
 
-		if (senderRole.isLessThan(rankReq))
+		if (senderRank.isLessThan(rankReq))
 		{
-			// You need a specific rank to change ranks
+			// You need a specific rank to change ranks.
-			msg(Txt.parse("<b>You must be %s or higher to change ranks",Txt.getNicedEnum(rankReq).toLowerCase()));
+			msg("<b>You must be %s or higher to change ranks.", Txt.getNicedEnum(rankReq).toLowerCase());
 			return false;
 		}
 		
 		// The following two if statements could be merged. 
 		// But isn't for the sake of nicer error messages.
-		if (senderRole == targetRole)
+		if (senderRank == targetRank)
 		{
-			// You can't change someones rank if it is equal to yours
+			// You can't change someones rank if it is equal to yours.
-			msg(Txt.parse("<b>%s can't manage eachother",Txt.getNicedEnum(rankReq)+"s"));
+			msg("<b>%s can't manage eachother.", Txt.getNicedEnum(rankReq)+"s");
 			return false;
 		}
 		
-		if (senderRole.isLessThan(targetRole))
+		if (senderRank.isLessThan(targetRank))
 		{
-			// You can't change someones rank if it is higher than yours
+			// You can't change someones rank if it is higher than yours.
-			msg(Txt.parse("<b>You can't manage people higher ranked than you"));
+			msg("<b>You can't manage people of higher rank.");
 			return false;
 		}
 		
-		if (senderRole.isAtMost(rank) && senderRole != Rel.LEADER)
+		if (senderRank.isAtMost(rank) && senderRank != Rel.LEADER)
 		{
-			// You can't set ranks equal to or higer than your own. Unless you are the leader
+			// You can't set ranks equal to or higer than your own. Unless you are the leader.
-			msg("<b>You can't set ranks higher than or equal to your own");
+			msg("<b>You can't set ranks higher than or equal to your own.");
 			return false;
 		}
 		
@@ -223,21 +237,21 @@ public class CmdFactionsRank extends FactionsCommand
 		// Just a nice msg. It would however be caught by an if statement below.
 		if (target.getRole() == Rel.RECRUIT && arg(1).equalsIgnoreCase("demote"))
 		{
-			msg("<b>You can't demote a recruit");
+			msg("%s <b>is already recruit.", target.describeTo(msender));
 			return false;
 		}
 		
 		// Just a nice msg. It would however be caught by an if statement below.
 		if (target.getRole() == Rel.LEADER && arg(1).equalsIgnoreCase("promote"))
 		{
-			msg("<b>You can't promote the leader");
+			msg("%s <b>is already leader.", target.describeTo(msender));
 			return false;
 		}
 		
 		// There must be a change, else it is all waste of time.
 		if (target.getRole() == rank)
 		{
-			msg("<b>Player already has that rank");
+			msg("%s <b>already has that rank.", target.describeTo(msender));
 			return false;
 		}
 		
@@ -259,24 +273,24 @@ public class CmdFactionsRank extends FactionsCommand
 
 	private void changeRankLeader()
 	{
-		// If there is a current leader. Demote & inform them
+		// If there is a current leader. Demote & inform them.
 		MPlayer targetFactionCurrentLeader = targetFaction.getLeader();
 		if (targetFactionCurrentLeader != null)
 		{
-			// Inform & demote the old leader
+			// Inform & demote the old leader.
 			targetFactionCurrentLeader.setRole(Rel.OFFICER);
 			if (targetFactionCurrentLeader != msender)
 			{
-				// They kinda know if they fired the command themself
+				// They kinda know if they fired the command themself.
 				targetFactionCurrentLeader.msg("<i>You have been demoted from the position of faction leader by %s<i>.", msender.describeTo(targetFactionCurrentLeader, true));
 			}
 		}
 		
-		// Inform & promote the new leader
+		// Inform & promote the new leader.
 		target.setRole(Rel.LEADER);
 		if (target != msender)
 		{
-			// They kinda know if they fired the command themself
+			// They kinda know if they fired the command themself.
 			target.msg("<i>You have been promoted to the position of faction leader by %s<i>.", msender.describeTo(target, true));
 		}
 		
@@ -284,28 +298,49 @@ public class CmdFactionsRank extends FactionsCommand
 		msg("<i>You have promoted %s<i> to the position of faction leader.", target.describeTo(msender, true));
 		
 		// Inform everyone
-		for (MPlayer mplayer : MPlayerColl.get().getAllOnline())
+		for (MPlayer recipient : MPlayerColl.get().getAllOnline())
 		{
-			String changerName = senderIsConsole ? "A server admin" : RelationUtil.describeThatToMe(msender, mplayer, true);
+			String changerName = senderIsConsole ? "A server admin" : msender.describeTo(recipient);
-			mplayer.msg("%s<i> gave %s<i> the leadership of %s<i>.", changerName, target.describeTo(mplayer), targetFaction.describeTo(mplayer));
+			recipient.msg("%s<i> gave %s<i> the leadership of %s<i>.", changerName, target.describeTo(recipient), targetFaction.describeTo(recipient));
 		}
 	}
 	
 	private void changeRankOther()
 	{
 		// If the target is currently the leader and faction isn't permanent...
-		if (targetRole == Rel.LEADER && (!MConf.get().permanentFactionsDisableLeaderPromotion || !targetFaction.getFlag(MFlag.ID_PERMANENT)))
+		if (targetRank == Rel.LEADER && !MConf.get().permanentFactionsDisableLeaderPromotion && targetFaction.getFlag(MFlag.ID_PERMANENT))
 		{
-			// ...we must promote a new one
+			// ...we must promote a new one.
 			targetFaction.promoteNewLeader();
 		}
+		// But if still no leader exists...
+		if (targetFaction.getLeader() == null && ! targetFaction.getFlag(MFlag.ID_PERMANENT))
+		{
+			// ...we will disband it.
+			// I'm kinda lazy, so I just make the console perform the command.
+			Factions.get().getOuterCmdFactions().cmdFactionsDisband.execute(IdUtil.getConsole(), MUtil.list( targetFaction.getName() ));
+		}
+		
+		List<MPlayer> recipients = targetFaction.getMPlayers();
+		if ( ! recipients.contains(msender))
+		{
+			recipients.add(msender);
+		}
 		
 		// Were they demoted or promoted?
-		String change = (rank.isLessThan(targetRole) ? "demoted" : "promoted");
+		String change = (rank.isLessThan(targetRank) ? "demoted" : "promoted");
 		
 		// The rank will be set before the msg, so they have the appropriate prefix.
 		target.setRole(rank);
+		String oldRankName = Txt.getNicedEnum(targetRank).toLowerCase();
 		String rankName = Txt.getNicedEnum(rank).toLowerCase();
-		msenderFaction.msg("%s<i> was %s to being %s %s in your faction.", target.describeTo(msenderFaction, true), change, Txt.aan(rankName), rankName);
+
+		for(MPlayer recipient : recipients)
+		{
+			String targetName = target.describeTo(recipient, true);
+			String wasWere = recipient == target ? "were" : "was";
+			recipient.msg("%s<i> %s %s from %s to <h>%s <i>in %s<i>.", targetName, wasWere, change, oldRankName, rankName, targetFaction.describeTo(msender));
+		}
+
 	}
 }

src/main/java/com/massivecraft/factions/event/EventFactionsRankChange.java

@@ -0,0 +1,40 @@
+package com.massivecraft.factions.event;
+
+import org.bukkit.command.CommandSender;
+import org.bukkit.event.HandlerList;
+
+import com.massivecraft.factions.Rel;
+import com.massivecraft.factions.entity.MPlayer;
+
+public class EventFactionsRankChange extends EventFactionsAbstractSender
+{
+	// -------------------------------------------- //
+	// REQUIRED EVENT CODE
+	// -------------------------------------------- //
+	
+	private static final HandlerList handlers = new HandlerList();
+	@Override public HandlerList getHandlers() { return handlers; }
+	public static HandlerList getHandlerList() { return handlers; }
+	
+	// -------------------------------------------- //
+	// FIELDS
+	// -------------------------------------------- //
+	
+	private final MPlayer mplayer;
+	public MPlayer getMPlayer() { return this.mplayer; }
+	
+	private Rel newRank;
+	public Rel getNewRank() { return this.newRank; }
+	public void setNewRank(Rel newRole) { this.newRank = newRole; }
+	
+	// -------------------------------------------- //
+	// CONSTRUCT
+	// -------------------------------------------- //
+	
+	public EventFactionsRankChange(CommandSender sender, MPlayer mplayer, Rel newRank)
+	{
+		super(sender);
+		this.mplayer = mplayer;
+		this.newRank = newRank;
+	}
+}