Fix for allies bypassing ownership if Ally options allowed them to build/interact/use in allied territory

Also consolidated some Conf lookups into new functions for Relations, to simplify and de-uglify the code a bit

src/com/massivecraft/factions/listeners/FactionsBlockListener.java

@@ -179,23 +179,25 @@ public class FactionsBlockListener extends BlockListener {
 			me.sendMessage("You can't "+action+" in a war zone.");
 			return false;
 		}
-
+		
 		Faction myFaction = me.getFaction();
 		Relation rel = myFaction.getRelation(otherFaction);
-
+		boolean ownershipFail = Conf.ownedAreasEnabled && (Conf.ownedAreaDenyBuild || Conf.ownedAreaPainBuild) && !otherFaction.playerHasOwnershipRights(me, loc);
-		// Cancel if we are not in our own territory
+		
-		if (myFaction != otherFaction) {
+		// Cancel and/or cause pain (depending on configuration) if we are not in our own territory
+		if (!rel.isMember()) {
 			boolean online = otherFaction.hasPlayersOnline();
-			boolean pain = (online && (rel.isEnemy() ? Conf.territoryEnemyPainBuild : (rel.isAlly() ? Conf.territoryAllyPainBuild : Conf.territoryPainBuild))) 
+			boolean pain = rel.confPainBuild(online);
-						|| (!online && (rel.isEnemy() ? Conf.territoryEnemyPainBuildWhenOffline : (rel.isAlly() ? Conf.territoryAllyPainBuildWhenOffline : Conf.territoryPainBuildWhenOffline)));
+			boolean deny = rel.confDenyBuild(online);
-			boolean deny = (online && (rel.isEnemy() ? Conf.territoryEnemyDenyBuild : (rel.isAlly() ? Conf.territoryAllyDenyBuild : Conf.territoryDenyBuild)))
+
-						|| (!online && (rel.isEnemy() ? Conf.territoryEnemyDenyBuildWhenOffline : (rel.isAlly() ? Conf.territoryAllyDenyBuildWhenOffline : Conf.territoryDenyBuildWhenOffline)));
-			//added by Bladedpenguin@gmail.com
 			//hurt the player for building/destroying?
 			if (pain) {
 				player.damage(Conf.actionDeniedPainAmount);
 				if (!deny) {
 					me.sendMessage("You are hurt for "+action+" in the territory of "+otherFaction.getTag(myFaction));
+					if (!Conf.ownedAreaDenyBuild) {
+						return true;
+					}
 				}
 			}
 			if (deny) {
@@ -203,12 +205,8 @@ public class FactionsBlockListener extends BlockListener {
 				return false;
 			}
 		}
-		// Also cancel if player doesn't have ownership rights for this claim
+		// Also cancel and/or cause pain if player doesn't have ownership rights for this claim
-		else if (
+		if (ownershipFail && !Factions.hasPermOwnershipBypass(player)
-			   Conf.ownedAreasEnabled
-			&& (Conf.ownedAreaDenyBuild || Conf.ownedAreaPainBuild)
-			&& !myFaction.playerHasOwnershipRights(me, loc)
-			&& !Factions.hasPermOwnershipBypass(player)
 			) {
 			if (Conf.ownedAreaPainBuild){
 				player.damage(Conf.actionDeniedPainAmount);

src/com/massivecraft/factions/listeners/FactionsEntityListener.java

@@ -372,26 +372,16 @@ public class FactionsEntityListener extends EntityListener {
 
 		Faction myFaction = me.getFaction();
 		Relation rel = myFaction.getRelation(otherFaction);
+		boolean ownershipFail = Conf.ownedAreasEnabled && Conf.ownedAreaDenyBuild && !otherFaction.playerHasOwnershipRights(me, loc);
 
-		// Cancel if we are not in our own territory
+		// Cancel if we are not in our own territory and building should be denied
-		if (myFaction != otherFaction) {
+		if (!rel.isMember() && rel.confDenyBuild(otherFaction.hasPlayersOnline())) {
-			boolean online = otherFaction.hasPlayersOnline();
+			me.sendMessage("You can't "+action+" paintings in the territory of "+otherFaction.getTag(myFaction));
-			if (
+			return false;
-				   (online && (rel.isEnemy() ? Conf.territoryEnemyDenyBuild : (rel.isAlly() ? Conf.territoryAllyDenyBuild : Conf.territoryDenyBuild)))
-				|| (!online && (rel.isEnemy() ? Conf.territoryEnemyDenyBuildWhenOffline : (rel.isAlly() ? Conf.territoryAllyDenyBuildWhenOffline : Conf.territoryDenyBuildWhenOffline)))
-				) {
-				me.sendMessage("You can't "+action+" paintings in the territory of "+otherFaction.getTag(myFaction));
-				return false;
-			}
 		}
 		// Also cancel if player doesn't have ownership rights for this claim
-		else if (
+		else if (ownershipFail && (!rel.isMember() || !Factions.hasPermOwnershipBypass(player))) {
-			   Conf.ownedAreasEnabled
+			me.sendMessage("You can't "+action+" paintings in this territory, it is owned by: "+otherFaction.getOwnerListString(loc));
-			&& Conf.ownedAreaDenyBuild
-			&& !myFaction.playerHasOwnershipRights(me, loc)
-			&& !Factions.hasPermOwnershipBypass(player)
-			) {
-			me.sendMessage("You can't "+action+" paintings in this territory, it is owned by: "+myFaction.getOwnerListString(loc));
 			return false;
 		}
 

src/com/massivecraft/factions/listeners/FactionsPlayerListener.java

@@ -322,25 +322,19 @@ public class FactionsPlayerListener extends PlayerListener{
 		
 		Faction myFaction = me.getFaction();
 		Relation rel = myFaction.getRelation(otherFaction);
-
+		boolean ownershipFail = Conf.ownedAreasEnabled && Conf.ownedAreaDenyUseage && !otherFaction.playerHasOwnershipRights(me, loc);
+		
 		// Cancel if we are not in our own territory
-		if (myFaction != otherFaction) {
+		if (!rel.isMember() && rel.confDenyUseage()) {
-			if (rel.isEnemy() ? Conf.territoryEnemyDenyUseage : (rel.isAlly() ? Conf.territoryAllyDenyUseage : Conf.territoryDenyUseage)) {
+			me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in the territory of "+otherFaction.getTag(myFaction));
-				me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in the territory of "+otherFaction.getTag(myFaction));
+			return false;
-				return false;
-			}
 		}
 		// Also cancel if player doesn't have ownership rights for this claim
-		else if (
+		else if (ownershipFail && (!rel.isMember() || !Factions.hasPermOwnershipBypass(player))) {
-			   Conf.ownedAreasEnabled
-			&& Conf.ownedAreaDenyUseage
-			&& !myFaction.playerHasOwnershipRights(me, loc)
-			&& !Factions.hasPermOwnershipBypass(player)
-			) {
 			me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in this territory, it is owned by: "+myFaction.getOwnerListString(loc));
 			return false;
 		}
-
+		
 		return true;
 	}
 
@@ -364,28 +358,19 @@ public class FactionsPlayerListener extends PlayerListener{
 				return true;
 			}
 		}
-
+		
 		FPlayer me = FPlayer.get(player);
 		Faction myFaction = me.getFaction();
 		Relation rel = myFaction.getRelation(otherFaction);
-
+		boolean ownershipFail = Conf.ownedAreasEnabled && Conf.ownedAreaProtectMaterials && !otherFaction.playerHasOwnershipRights(me, loc);
-		if ((rel.isEnemy() && !Conf.territoryEnemyProtectMaterials) || (rel.isAlly() && !Conf.territoryAllyProtectMaterials)) {
+		
-			return true;
-		}
-
 		// You may use any block unless it is another faction's territory...
-		if (otherFaction.isNormal() && myFaction != otherFaction) {
+		if ((rel.isEnemy() && Conf.territoryEnemyProtectMaterials) || (rel.isAlly() && Conf.territoryAllyProtectMaterials)) {
 			me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in the territory of "+otherFaction.getTag(myFaction));
 			return false;
 		}
 		// Also cancel if player doesn't have ownership rights for this claim
-		else if (
+		else if (ownershipFail && (!rel.isMember() || !Factions.hasPermOwnershipBypass(player))) {
-			   myFaction == otherFaction
-			&& Conf.ownedAreasEnabled
-			&& Conf.ownedAreaProtectMaterials
-			&& !myFaction.playerHasOwnershipRights(me, loc)
-			&& !Factions.hasPermOwnershipBypass(player)
-			) {
 			me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in this territory, it is owned by: "+myFaction.getOwnerListString(loc));
 			return false;
 		}

src/com/massivecraft/factions/struct/Relation.java

@@ -25,19 +25,19 @@ public enum Relation {
 	}
 	
 	public boolean isMember() {
-		return this == Relation.MEMBER;
+		return this.value == MEMBER.value;
 	}
 	
 	public boolean isAlly() {
-		return this == Relation.ALLY;
+		return this.value == ALLY.value;
 	}
 	
 	public boolean isNeutral() {
-		return this == Relation.NEUTRAL;
+		return this.value == NEUTRAL.value;
 	}
 	
 	public boolean isEnemy() {
-		return this == Relation.ENEMY;
+		return this.value == ENEMY.value;
 	}
 	
 	public boolean isAtLeast(Relation relation) {
@@ -49,14 +49,79 @@ public enum Relation {
 	}
 	
 	public ChatColor getColor() {
-		if (this == Relation.MEMBER) {
+		if (this.value == MEMBER.value) {
 			return Conf.colorMember;
-		} else if (this == Relation.ALLY) {
+		} else if (this.value == ALLY.value) {
 			return Conf.colorAlly;
-		} else if (this == Relation.NEUTRAL) {
+		} else if (this.value == NEUTRAL.value) {
 			return Conf.colorNeutral;
 		} else {
 			return Conf.colorEnemy;
 		}
 	}
+
+	// return appropriate Conf setting for DenyBuild based on this relation and their online status
+	public boolean confDenyBuild(boolean online) {
+		if (online) {
+			if (isEnemy()) {
+				return Conf.territoryEnemyDenyBuild;
+			}
+			else if (isAlly()) {
+				return Conf.territoryAllyDenyBuild;
+			}
+			else {
+				return Conf.territoryDenyBuild;
+			}
+		}
+		else {
+			if (isEnemy()) {
+				return Conf.territoryEnemyDenyBuildWhenOffline;
+			}
+			else if (isAlly()) {
+				return Conf.territoryAllyDenyBuildWhenOffline;
+			}
+			else {
+				return Conf.territoryDenyBuildWhenOffline;
+			}
+		}
+	}
+
+	// return appropriate Conf setting for PainBuild based on this relation and their online status
+	public boolean confPainBuild(boolean online) {
+		if (online) {
+			if (isEnemy()) {
+				return Conf.territoryEnemyPainBuild;
+			}
+			else if (isAlly()) {
+				return Conf.territoryAllyPainBuild;
+			}
+			else {
+				return Conf.territoryPainBuild;
+			}
+		}
+		else {
+			if (isEnemy()) {
+				return Conf.territoryEnemyPainBuildWhenOffline;
+			}
+			else if (isAlly()) {
+				return Conf.territoryAllyPainBuildWhenOffline;
+			}
+			else {
+				return Conf.territoryPainBuildWhenOffline;
+			}
+		}
+	}
+
+	// return appropriate Conf setting for DenyUseage based on this relation
+	public boolean confDenyUseage() {
+		if (isEnemy()) {
+			return Conf.territoryEnemyDenyUseage;
+		}
+		else if (isAlly()) {
+			return Conf.territoryAllyDenyUseage;
+		}
+		else {
+			return Conf.territoryDenyUseage;
+		}
+	}
 }