Optimization: Factions (as objects) now maintain a list of FPlayers in the faction for faster lookup

INTEGRATION NOTE: getFPlayers() and getFPlayersWhereOnline(boolean online) now return Set<FPlayer> instead of ArrayList<FPlayer>, so other plugins which hook into those methods will probably need to be updated.

src/com/massivecraft/factions/FPlayer.java

@@ -1,6 +1,5 @@
 package com.massivecraft.factions;
 
-import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -48,6 +47,9 @@ public class FPlayer extends PlayerEntity implements EconomyParticipator
 	public boolean hasFaction() { return ! factionId.equals("0"); }
 	public void setFaction(Faction faction)
 	{
+		Faction oldFaction = this.getFaction();
+		if (oldFaction != null) oldFaction.removeFPlayer(this);
+		faction.addFPlayer(this);
 		this.factionId = faction.getId();
 		SpoutFeatures.updateAppearances(this.getPlayer());
 	}
@@ -130,6 +132,10 @@ public class FPlayer extends PlayerEntity implements EconomyParticipator
 	
 	public final void resetFactionData(boolean doSpotUpdate)
 	{
+		Faction currentFaction = this.getFaction();
+		if (currentFaction != null)
+			currentFaction.removeFPlayer(this);
+
 		this.factionId = "0"; // The default neutral faction
 		this.chatMode = ChatMode.PUBLIC;
 		this.role = Rel.MEMBER;
@@ -491,8 +497,7 @@ public class FPlayer extends PlayerEntity implements EconomyParticipator
 		}
 
 		// Am I the last one in the faction?
-		ArrayList<FPlayer> fplayers = myFaction.getFPlayers();
+		if (myFaction.getFPlayers().size() == 1)
-		if (fplayers.size() == 1 && fplayers.get(0) == this)
 		{
 			// Transfer all money
 			if (Econ.shouldBeUsed())

src/com/massivecraft/factions/Faction.java

@@ -23,6 +23,10 @@ public class Faction extends Entity implements EconomyParticipator
 	// FIELD: relationWish
 	private Map<String, Rel> relationWish;
 
+	// FIELD: fplayers
+	// speedy lookup of players in faction
+	private Set<FPlayer> fplayers = new HashSet<FPlayer>();
+
 	// FIELD: invites
 	// Where string is a lowercase player name
 	private Set<String> invites; 
@@ -279,7 +283,7 @@ public class Faction extends Entity implements EconomyParticipator
 		}
 		
 		double ret = 0;
-		for (FPlayer fplayer : this.getFPlayers())
+		for (FPlayer fplayer : fplayers)
 		{
 			ret += fplayer.getPower();
 		}
@@ -298,7 +302,7 @@ public class Faction extends Entity implements EconomyParticipator
 		}
 		
 		double ret = 0;
-		for (FPlayer fplayer : this.getFPlayers())
+		for (FPlayer fplayer : fplayers)
 		{
 			ret += fplayer.getPowerMax();
 		}
@@ -337,30 +341,47 @@ public class Faction extends Entity implements EconomyParticipator
 	// FPlayers
 	// -------------------------------
 
-	public ArrayList<FPlayer> getFPlayers()
+	// maintain the reference list of FPlayers in this faction
+	public void refreshFPlayers()
 	{
-		ArrayList<FPlayer> ret = new ArrayList<FPlayer>();
+		fplayers.clear();
-		//if (this.isPlayerFreeType()) return ret;
+		if (this.isNone()) return;
 
 		for (FPlayer fplayer : FPlayers.i.get())
 		{
 			if (fplayer.getFaction() == this)
 			{
-				ret.add(fplayer);
+				fplayers.add(fplayer);
 			}
 		}
+	}
+	public boolean addFPlayer(FPlayer fplayer)
+	{
+		if (this.isNone()) return false;
+
+		return fplayers.add(fplayer);
+	}
+	public boolean removeFPlayer(FPlayer fplayer)
+	{
+		if (this.isNone()) return false;
+
+		return fplayers.remove(fplayer);
+	}
 
+	public Set<FPlayer> getFPlayers()
+	{
+		// return a shallow copy of the FPlayer list, to prevent tampering and concurrency issues
+		Set<FPlayer> ret = new HashSet(fplayers);
 		return ret;
 	}
 	
-	public ArrayList<FPlayer> getFPlayersWhereOnline(boolean online)
+	public Set<FPlayer> getFPlayersWhereOnline(boolean online)
 	{
-		ArrayList<FPlayer> ret = new ArrayList<FPlayer>();
+		Set<FPlayer> ret = new HashSet<FPlayer>();
-		//if (this.isPlayerFreeType()) return ret;
 
-		for (FPlayer fplayer : FPlayers.i.get())
+		for (FPlayer fplayer : fplayers)
 		{
-			if (fplayer.getFaction() == this && fplayer.isOnline() == online)
+			if (fplayer.isOnline() == online)
 			{
 				ret.add(fplayer);
 			}
@@ -373,9 +394,9 @@ public class Faction extends Entity implements EconomyParticipator
 	{
 		//if ( ! this.isNormal()) return null;
 		
-		for (FPlayer fplayer : FPlayers.i.get())
+		for (FPlayer fplayer : fplayers)
 		{
-			if (fplayer.getFaction() == this && fplayer.getRole() == Rel.LEADER)
+			if (fplayer.getRole() == Rel.LEADER)
 			{
 				return fplayer;
 			}
@@ -388,9 +409,9 @@ public class Faction extends Entity implements EconomyParticipator
 		ArrayList<FPlayer> ret = new ArrayList<FPlayer>();
 		//if ( ! this.isNormal()) return ret;
 		
-		for (FPlayer fplayer : FPlayers.i.get())
+		for (FPlayer fplayer : fplayers)
 		{
-			if (fplayer.getFaction() == this && fplayer.getRole() == role)
+			if (fplayer.getRole() == role)
 			{
 				ret.add(fplayer);
 			}

src/com/massivecraft/factions/Factions.java

@@ -93,6 +93,12 @@ public class Factions extends EntityCollection<Faction>
 		if (warZone != null && ! warZone.getFlag(FFlag.PERMANENT))
 			setFlagsForWarZone(warZone);
 
+		// populate all faction player lists
+		for (Faction faction : i.get())
+		{
+			faction.refreshFPlayers();
+		}
+
 		return true;
 	}