Fixes Permissions not behaving the way it was expected within /f perms

Fixes ownership logic and permissions checks

src/main/java/com/massivecraft/factions/Conf.java

@@ -333,16 +333,17 @@ public class Conf {
 	public static transient char[] mapKeyChrs = "\\/#$%=&^ABCDEFGHJKLMNOPQRSTUVWXYZ1234567890abcdeghjmnopqrsuvwxyz?".toCharArray();
 
 
-	// Default Options
+	// Default Options - Is this even shown on the Conf.json?
 	public static boolean useCustomDefaultPermissions = false;
 	public static boolean usePermissionHints = false;
 	public static HashMap<String, DefaultPermissions> defaultFactionPermissions = new HashMap<>();
-	// Custom Ranks
+	// Custom Ranks - Oof I forgot I was doing this _SvenjaReissaus_
 	//public static boolean enableCustomRanks = false; // We will disable it by default to avoid any migration error
 	//public static int maxCustomRanks = 2; // Setting this to -1 will allow unlimited custom ranks
 	// -------------------------------------------- //
 	// Persistance
 	// -------------------------------------------- //
+
 	private static transient Conf i = new Conf();
 
 	static {

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

@@ -18,9 +18,6 @@ public class CmdShowClaims extends FCommand {
 		this.senderMustBePlayer = true;
 		this.senderMustBeMember = true;
 		this.senderMustBeModerator = false;
-		this.senderMustBePlayer = true;
-
-
 	}
 
 	@Override

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

@@ -133,20 +133,7 @@ public class FactionsBlockListener implements Listener {
 		}
 
 		// Check the permission just after making sure the land isn't owned by someone else to avoid bypass.
-
+		return CheckPlayerAccess(player, me, loc, myFaction, access, PermissableAction.fromString(action));
-		if (access != Access.ALLOW && me.getRole() != Role.LEADER) {
-			// TODO: Update this once new access values are added other than just allow / deny.
-			if (access == Access.DENY) {
-				if (!justCheck)
-					me.msg(TL.GENERIC_NOPERMISSION, action);
-				return false;
-			} else if (myFaction.getOwnerListString(loc) != null && !myFaction.getOwnerListString(loc).isEmpty() && !myFaction.getOwnerListString(loc).contains(player.getName())) {
-				if (!justCheck)
-					me.msg("<b>You can't " + action + " in this territory, it is owned by: " + myFaction.getOwnerListString(loc));
-				return false;
-			}
-		}
-		return true;
 	}
 
 	@EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true)
@@ -519,4 +506,32 @@ public class FactionsBlockListener implements Listener {
 			}
 		}
 	}
+
+	/// <summary>
+	///	This checks if the current player can execute an action based on it's factions access and surroundings
+	/// It will grant access in the following priorities:
+	/// - If Faction Land is Owned and the Owner is the current player, or player is faction leader.
+	/// - If Faction Land is not Owned and my access value is not set to DENY
+	/// - If none of the filters above matches, then we consider access is set to ALLOW|UNDEFINED
+	/// This check does not performs any kind of bypass check (i.e.: me.isAdminBypassing())
+	/// </summary>
+	/// <param name="player">The player entity which the check will be made upon</param>
+	/// <param name="me">The Faction player object related to the player</param>
+	/// <param name="loc">The World location where the action is being executed</param>
+	/// <param name="myFaction">The faction of the player being checked</param>
+	/// <param name="access">The current's faction access permission for the action</param>
+	private static boolean CheckPlayerAccess(Player player, FPlayer me, FLocation loc, Faction myFaction, Access access, PermissableAction action) {
+		if (access == null) access = Access.DENY; // Let's deny by default
+		boolean landOwned = (myFaction.doesLocationHaveOwnersSet(loc) && !myFaction.getOwnerList(loc).isEmpty());
+		if (landOwned && myFaction.getOwnerListString(loc).contains(player.getName()) || me.getRole() == Role.LEADER) return true;
+		else if (landOwned && !myFaction.getOwnerListString(loc).contains(player.getName())) {
+			me.msg("<b>You can't " + action + " in this territory, it is owned by: " + myFaction.getOwnerListString(loc));
+			return false;
+		} else if (!landOwned && access == Access.DENY) { // If land is not owned but access is set to DENY anyway
+			me.msg(TL.GENERIC_NOPERMISSION, action);
+			return false;
+		}
+		// We assume faction land is not owned, and the access is not set to DENY, so we allow to execute the action
+		return true;
+	}
 }

src/main/java/com/massivecraft/factions/listeners/FactionsChatListener.java

@@ -60,6 +60,7 @@ public class FactionsChatListener implements Listener {
 				// Just in case player gets demoted while in faction chat.
 				me.msg(TL.COMMAND_CHAT_MOD_ONLY);
 				event.setCancelled(true);
+				me.setChatMode(ChatMode.FACTION);
 				return;
 			}
 

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

@@ -143,25 +143,11 @@ public class FactionsPlayerListener implements Listener {
 			if (!justCheck) {
 				me.msg(TL.PLAYER_USE_TERRITORY, TextUtil.getMaterialName(material), otherFaction.getTag(myFaction));
 			}
-
 			return false;
 		}
 
 		Access access = otherFaction.getAccess(me, PermissableAction.ITEM);
-		if (access != null && access != Access.UNDEFINED) {
+		return CheckPlayerAccess(player, me, loc, myFaction, access, PermissableAction.ITEM);
-			// TODO: Update this once new access values are added other than just allow / deny.
-			if ((myFaction.getOwnerListString(loc) != null && !myFaction.getOwnerListString(loc).isEmpty() && myFaction.getOwnerListString(loc).contains(player.getName()))) {
-				return true;
-			} else if (myFaction.getOwnerListString(loc) != null && !myFaction.getOwnerListString(loc).isEmpty() && !myFaction.getOwnerListString(loc).contains(player.getName())) {
-				me.msg("<b>You can't use items in this territory, it is owned by: " + myFaction.getOwnerListString(loc));
-				return false;
-			} else if (access == Access.DENY) {
-				me.msg(TL.GENERIC_NOPERMISSION, PermissableAction.ITEM);
-				return false;
-			}
-		}
-
-		return true;
 	}
 
 	@SuppressWarnings("deprecation")
@@ -194,7 +180,6 @@ public class FactionsPlayerListener implements Listener {
 		}
 
 		PermissableAction action = null;
-
 		if (SavageFactions.plugin.mc113) {
 			switch (block.getType()) {
 				case LEVER:
@@ -325,12 +310,15 @@ public class FactionsPlayerListener implements Listener {
 		}
 
 		// We only care about some material types.
+		/// Who was the idiot?
 		if (otherFaction.hasPlayersOnline()) {
-			if (!Conf.territoryProtectedMaterials.contains(material))
+			if (Conf.territoryProtectedMaterials.contains(material)) {
-				return true;
+				return false;
+			}
 		} else {
-			if (!Conf.territoryProtectedMaterialsWhenOffline.contains(material))
+			if (Conf.territoryProtectedMaterialsWhenOffline.contains(material)) {
-				return true;
+				return false;
+			}
 		}
 
 		// Move up access check to check for exceptions
@@ -356,21 +344,10 @@ public class FactionsPlayerListener implements Listener {
 			}
 		}
 
-		if (access != Access.ALLOW && me.getRole() != Role.LEADER) {
+		return CheckPlayerAccess(player, me, loc, myFaction, access, action);
-			// TODO: Update this once new access values are added other than just allow / deny.
-			if ((myFaction.getOwnerListString(loc) != null && !myFaction.getOwnerListString(loc).isEmpty() && myFaction.getOwnerListString(loc).contains(player.getName()))) {
-				return true;
-			} else if (myFaction.getOwnerListString(loc) != null && !myFaction.getOwnerListString(loc).isEmpty() && !myFaction.getOwnerListString(loc).contains(player.getName())) {
-				me.msg("<b>You can't " + action + " in this territory, it is owned by: " + myFaction.getOwnerListString(loc));
-				return false;
-			} else if (access == Access.DENY) {
-				me.msg(TL.GENERIC_NOPERMISSION, action);
-				return false;
-			}
-		}
-		return true;
 	}
 
+
 	public static boolean preventCommand(String fullCmd, Player player) {
 		if ((Conf.territoryNeutralDenyCommands.isEmpty() && Conf.territoryEnemyDenyCommands.isEmpty() && Conf.permanentFactionMemberDenyCommands.isEmpty() && Conf.warzoneDenyCommands.isEmpty())) {
 			return false;
@@ -1070,4 +1047,33 @@ public class FactionsPlayerListener implements Listener {
 			return attempts;
 		}
 	}
+	/// <summary>
+	///	This checks if the current player can execute an action based on it's factions access and surroundings
+	/// It will grant access in the following priorities:
+	/// - If Faction Land is Owned and the Owner is the current player, or player is faction leader.
+	/// - If Faction Land is not Owned and my access value is not set to DENY
+	/// - If none of the filters above matches, then we consider access is set to ALLOW|UNDEFINED
+	/// This check does not performs any kind of bypass check (i.e.: me.isAdminBypassing())
+	/// </summary>
+	/// <param name="player">The player entity which the check will be made upon</param>
+	/// <param name="me">The Faction player object related to the player</param>
+	/// <param name="loc">The World location where the action is being executed</param>
+	/// <param name="myFaction">The faction of the player being checked</param>
+	/// <param name="access">The current's faction access permission for the action</param>
+	private static boolean CheckPlayerAccess(Player player, FPlayer me, FLocation loc, Faction myFaction, Access access, PermissableAction action) {
+		if (access != null && access != Access.UNDEFINED) {
+			// TODO: Update this once new access values are added other than just allow / deny.
+			boolean landOwned = (myFaction.doesLocationHaveOwnersSet(loc) && !myFaction.getOwnerList(loc).isEmpty());
+			if (landOwned && myFaction.getOwnerListString(loc).contains(player.getName()) || me.getRole() == Role.LEADER) return true;
+			else if (landOwned && !myFaction.getOwnerListString(loc).contains(player.getName())) {
+				me.msg("<b>You can't do that in this territory, it is owned by: " + myFaction.getOwnerListString(loc));
+				return false;
+			} else if (!landOwned && access != Access.DENY) return true;
+			else {
+				me.msg(TL.GENERIC_NOPERMISSION, action);
+				return false;
+			}
+		}
+		return true;
+	}
 }