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

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;
 
@@ -45,6 +44,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());
 	}
@@ -171,6 +173,7 @@ public class FPlayer extends PlayerEntity implements EconomyParticipator
 		if (Factions.i.exists(this.getFactionId()))
 		{
 			Faction currentFaction = this.getFaction();
+			currentFaction.removeFPlayer(this);
 			if (currentFaction.isNormal())
 			{
 				currentFaction.clearClaimOwnership(this.getId());
@@ -604,8 +607,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

@@ -27,7 +27,11 @@ public class Faction extends Entity implements EconomyParticipator
 	
 	// FIELD: claimOwnership
 	private Map<FLocation, Set<String>> claimOwnership = new ConcurrentHashMap<FLocation, Set<String>>();
-	
+
+	// 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; 
@@ -265,7 +269,7 @@ public class Faction extends Entity implements EconomyParticipator
 		}
 		
 		double ret = 0;
-		for (FPlayer fplayer : this.getFPlayers())
+		for (FPlayer fplayer : fplayers)
 		{
 			ret += fplayer.getPower();
 		}
@@ -284,7 +288,7 @@ public class Faction extends Entity implements EconomyParticipator
 		}
 		
 		double ret = 0;
-		for (FPlayer fplayer : this.getFPlayers())
+		for (FPlayer fplayer : fplayers)
 		{
 			ret += fplayer.getPowerMax();
 		}
@@ -322,31 +326,48 @@ 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.isPlayerFreeType()) return;
 
 		for (FPlayer fplayer : FPlayers.i.get())
 		{
 			if (fplayer.getFaction() == this)
 			{
-				ret.add(fplayer);
+				fplayers.add(fplayer);
 			}
 		}
+	}
+	public boolean addFPlayer(FPlayer fplayer)
+	{
+		if (this.isPlayerFreeType()) return false;
 
+		return fplayers.add(fplayer);
+	}
+	public boolean removeFPlayer(FPlayer fplayer)
+	{
+		if (this.isPlayerFreeType()) 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);
 			}
@@ -359,9 +380,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() == Role.ADMIN)
+			if (fplayer.getRole() == Role.ADMIN)
 			{
 				return fplayer;
 			}
@@ -374,9 +395,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

@@ -80,7 +80,13 @@ public class Factions extends EntityCollection<Faction>
 			if (faction.getTag().contains(" "))
 				faction.setTag("WarZone");
 		}
-		
+
+		// populate all faction player lists
+		for (Faction faction : i.get())
+		{
+			faction.refreshFPlayers();
+		}
+
 		return true;
 	}