From 329cef6465841a79522ad1115abc5d9e5f38c531 Mon Sep 17 00:00:00 2001 From: Brettflan Date: Wed, 12 Oct 2011 22:31:18 -0500 Subject: [PATCH] Fix for messed up ownership protection handling in regards to ally/enemy status In the process, removed the separate painting handling and made it use the standard block place/destroy checking routine, and otherwise cleaned up the related code a bit --- src/com/massivecraft/factions/Faction.java | 18 ++- .../listeners/FactionsBlockListener.java | 116 +++++++----------- .../listeners/FactionsEntityListener.java | 74 ++--------- .../listeners/FactionsPlayerListener.java | 76 ++++-------- 4 files changed, 88 insertions(+), 196 deletions(-) diff --git a/src/com/massivecraft/factions/Faction.java b/src/com/massivecraft/factions/Faction.java index 94765456..134e1906 100644 --- a/src/com/massivecraft/factions/Faction.java +++ b/src/com/massivecraft/factions/Faction.java @@ -11,6 +11,7 @@ import org.bukkit.entity.Player; import com.massivecraft.factions.iface.EconomyParticipator; import com.massivecraft.factions.iface.RelationParticipator; import com.massivecraft.factions.integration.Econ; +import com.massivecraft.factions.struct.Permission; import com.massivecraft.factions.struct.Relation; import com.massivecraft.factions.struct.Role; import com.massivecraft.factions.util.*; @@ -631,26 +632,31 @@ public class Faction extends Entity implements EconomyParticipator public boolean playerHasOwnershipRights(FPlayer fplayer, FLocation loc) { - // sufficient role to bypass ownership? - if (fplayer.getFaction() == this && fplayer.getRole().isAtLeast(Conf.ownedAreaModeratorsBypass ? Role.MODERATOR : Role.ADMIN)) + // in own faction, with sufficient role or permission to bypass ownership? + if + ( + fplayer.getFaction() == this + && + ( + fplayer.getRole().isAtLeast(Conf.ownedAreaModeratorsBypass ? Role.MODERATOR : Role.ADMIN) + || + Permission.OWNERSHIP_BYPASS.has(fplayer.getPlayer()) + ) + ) { return true; } // make sure claimOwnership is initialized if (claimOwnership.isEmpty()) - { return true; - } // need to check the ownership list, then Set ownerData = claimOwnership.get(loc); // if no owner list, owner list is empty, or player is in owner list, they're allowed if (ownerData == null || ownerData.isEmpty() || ownerData.contains(fplayer.getName().toLowerCase())) - { return true; - } return false; } diff --git a/src/com/massivecraft/factions/listeners/FactionsBlockListener.java b/src/com/massivecraft/factions/listeners/FactionsBlockListener.java index 1b2c91e0..4ab7d693 100644 --- a/src/com/massivecraft/factions/listeners/FactionsBlockListener.java +++ b/src/com/massivecraft/factions/listeners/FactionsBlockListener.java @@ -127,47 +127,34 @@ public class FactionsBlockListener extends BlockListener Faction otherFaction = Board.getFactionAt(new FLocation(target)); if (pistonFaction == otherFaction) - { return true; - } if (otherFaction.isNone()) { if (!Conf.wildernessDenyBuild || Conf.worldsNoWildernessProtection.contains(target.getWorld().getName())) - { return true; - } + return false; } else if (otherFaction.isSafeZone()) { if ( ! Conf.safeZoneDenyBuild) - { return true; - } + return false; } else if (otherFaction.isWarZone()) { if ( ! Conf.warZoneDenyBuild) - { return true; - } + return false; } Relation rel = pistonFaction.getRelationTo(otherFaction); - boolean online = otherFaction.hasPlayersOnline(); - if - ( - (online && (rel.isEnemy() ? Conf.territoryEnemyDenyBuild : (rel.isAlly() ? Conf.territoryAllyDenyBuild : Conf.territoryDenyBuild))) - || - (!online && (rel.isEnemy() ? Conf.territoryEnemyDenyBuildWhenOffline : (rel.isAlly() ? Conf.territoryAllyDenyBuildWhenOffline : Conf.territoryDenyBuildWhenOffline))) - ) - { + if (rel.confDenyBuild(otherFaction.hasPlayersOnline())) return false; - } return true; } @@ -175,106 +162,87 @@ public class FactionsBlockListener extends BlockListener public static boolean playerCanBuildDestroyBlock(Player player, Location location, String action, boolean justCheck) { FPlayer me = FPlayers.i.get(player); - + if (me.isAdminBypassing()) - { return true; - } FLocation loc = new FLocation(location); Faction otherFaction = Board.getFactionAt(loc); - if (otherFaction.isNone()) { if (!Conf.wildernessDenyBuild || Conf.worldsNoWildernessProtection.contains(location.getWorld().getName())) - { return true; // This is not faction territory. Use whatever you like here. - } + if (!justCheck) - { me.sendMessage("You can't "+action+" in the wilderness."); - } + return false; } else if (otherFaction.isSafeZone()) { if (!Conf.safeZoneDenyBuild || Permission.MANAGE_SAFE_ZONE.has(player)) - { return true; - } - if (!justCheck) { + + if (!justCheck) me.sendMessage("You can't "+action+" in a safe zone."); - } + return false; } else if (otherFaction.isWarZone()) { if (!Conf.warZoneDenyBuild || Permission.MANAGE_WAR_ZONE.has(player)) - { return true; - } + if (!justCheck) - { me.sendMessage("You can't "+action+" in a war zone."); - } + return false; } - + Faction myFaction = me.getFaction(); Relation rel = myFaction.getRelationTo(otherFaction); - boolean ownershipFail = Conf.ownedAreasEnabled && (Conf.ownedAreaDenyBuild || Conf.ownedAreaPainBuild) && !otherFaction.playerHasOwnershipRights(me, loc); - - // 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 = (!justCheck) && rel.confPainBuild(online); - boolean deny = rel.confDenyBuild(online); + boolean online = otherFaction.hasPlayersOnline(); + boolean pain = !justCheck && rel.confPainBuild(online); + boolean deny = rel.confDenyBuild(online); - //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) - { - if (!justCheck) - { - me.sendMessage("You can't "+action+" in the territory of "+otherFaction.getTag(myFaction)); - } - return false; - } - } - // Also cancel and/or cause pain if player doesn't have ownership rights for this claim - else if (rel.isMember() && ownershipFail && ! Permission.OWNERSHIP_BYPASS.has(player)) + // hurt the player for building/destroying in other territory? + if (pain) { - if (Conf.ownedAreaPainBuild && !justCheck) + player.damage(Conf.actionDeniedPainAmount); + + if (!deny) + me.sendMessage("It is painful to try to "+action+" in the territory of "+otherFaction.getTag(myFaction)); + } + + // cancel building/destroying in other territory? + if (deny) + { + if (!justCheck) + me.sendMessage("You can't "+action+" in the territory of "+otherFaction.getTag(myFaction)); + + return false; + } + + // Also cancel and/or cause pain if player doesn't have ownership rights for this claim + if (Conf.ownedAreasEnabled && (Conf.ownedAreaDenyBuild || Conf.ownedAreaPainBuild) && !otherFaction.playerHasOwnershipRights(me, loc)) + { + if (!pain && Conf.ownedAreaPainBuild && !justCheck) { player.damage(Conf.actionDeniedPainAmount); + if (!Conf.ownedAreaDenyBuild) - { - me.sendMessage("You are hurt for "+action+" in this territory, it is owned by: "+myFaction.getOwnerListString(loc)); - } + me.sendMessage("It is painful to try to "+action+" in this territory, it is owned by: "+otherFaction.getOwnerListString(loc)); } if (Conf.ownedAreaDenyBuild) { if (!justCheck) - { - me.sendMessage("You can't "+action+" in this territory, it is owned by: "+myFaction.getOwnerListString(loc)); - } + me.sendMessage("You can't "+action+" in this territory, it is owned by: "+otherFaction.getOwnerListString(loc)); + return false; } } - + return true; } } diff --git a/src/com/massivecraft/factions/listeners/FactionsEntityListener.java b/src/com/massivecraft/factions/listeners/FactionsEntityListener.java index 73d30699..7aec88ea 100644 --- a/src/com/massivecraft/factions/listeners/FactionsEntityListener.java +++ b/src/com/massivecraft/factions/listeners/FactionsEntityListener.java @@ -117,14 +117,17 @@ public class FactionsEntityListener extends EntityListener Location loc = event.getLocation(); Faction faction = Board.getFactionAt(new FLocation(loc)); - boolean online = faction.hasPlayersOnline(); - + if (faction.noExplosionsInTerritory()) { // faction is peaceful and has explosions set to disabled event.setCancelled(true); + return; } - else if + + boolean online = faction.hasPlayersOnline(); + + if ( event.getEntity() instanceof Creeper && @@ -418,9 +421,7 @@ public class FactionsEntityListener extends EntityListener return; } - FLocation loc = new FLocation(event.getPainting().getLocation()); - - if ( ! this.playerCanDoPaintings((Player)breaker, loc, "remove")) + if ( ! FactionsBlockListener.playerCanBuildDestroyBlock((Player)breaker, event.getPainting().getLocation(), "remove paintings", false)) { event.setCancelled(true); } @@ -431,71 +432,12 @@ public class FactionsEntityListener extends EntityListener { if (event.isCancelled()) return; - if ( ! this.playerCanDoPaintings(event.getPlayer(), new FLocation(event.getBlock()), "place")) + if ( ! FactionsBlockListener.playerCanBuildDestroyBlock(event.getPlayer(), event.getBlock().getLocation(), "place paintings", false) ) { event.setCancelled(true); } } - public boolean playerCanDoPaintings(Player player, FLocation loc, String action) - { - FPlayer me = FPlayers.i.get(player); - if (me.isAdminBypassing()) - { - return true; - } - - Faction otherFaction = Board.getFactionAt(loc); - - if (otherFaction.isNone()) - { - if (!Conf.wildernessDenyBuild || Conf.worldsNoWildernessProtection.contains(player.getWorld().getName())) - { - return true; // This is not faction territory. Use whatever you like here. - } - me.sendMessage("You can't "+action+" paintings in the wilderness."); - return false; - } - - if (otherFaction.isSafeZone()) - { - if (Permission.MANAGE_SAFE_ZONE.has(player) || !Conf.safeZoneDenyBuild) - { - return true; - } - me.sendMessage("You can't "+action+" paintings in a safe zone."); - return false; - } - else if (otherFaction.isWarZone()) - { - if (Permission.MANAGE_WAR_ZONE.has(player) || !Conf.warZoneDenyBuild) - { - return true; - } - me.sendMessage("You can't "+action+" paintings in a war zone."); - return false; - } - - Faction myFaction = me.getFaction(); - Relation rel = myFaction.getRelationTo(otherFaction); - boolean ownershipFail = Conf.ownedAreasEnabled && Conf.ownedAreaDenyBuild && !otherFaction.playerHasOwnershipRights(me, loc); - - // Cancel if we are not in our own territory and building should be denied - if (!rel.isMember() && rel.confDenyBuild(otherFaction.hasPlayersOnline())) - { - 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 (rel.isMember() && ownershipFail && !Permission.OWNERSHIP_BYPASS.has(player)) - { - me.sendMessage("You can't "+action+" paintings in this territory, it is owned by: "+otherFaction.getOwnerListString(loc)); - return false; - } - - return true; - } - @Override public void onEndermanPickup(EndermanPickupEvent event) { diff --git a/src/com/massivecraft/factions/listeners/FactionsPlayerListener.java b/src/com/massivecraft/factions/listeners/FactionsPlayerListener.java index e384f2d1..7f4ce253 100644 --- a/src/com/massivecraft/factions/listeners/FactionsPlayerListener.java +++ b/src/com/massivecraft/factions/listeners/FactionsPlayerListener.java @@ -231,6 +231,9 @@ public class FactionsPlayerListener extends PlayerListener Faction myFaction = me.getFaction(); // TODO: Why is this ("cost") here and unused??? Should it be used somewhere Brettflan? :) // Olof just commented it out to avoid the error. + // So sayeth Brettflan, answered in a comment since you asked in a comment: + // NOTHING MORE TODO: it was used, but apparently not needed after some changes including this commit: https://github.com/MassiveCraft/Factions/commit/5eaf9c68358c6076bb856baf80fd6496e2ab02ce + // //Faction otherFaction = Board.getFactionAt(to); //double cost = Econ.calculateClaimCost(myFaction.getLandRounded(), otherFaction.isNormal()); @@ -323,90 +326,74 @@ public class FactionsPlayerListener extends PlayerListener { FPlayer me = FPlayers.i.get(player); if (me.isAdminBypassing()) - { return true; - } FLocation loc = new FLocation(location); Faction otherFaction = Board.getFactionAt(loc); - if (otherFaction.hasPlayersOnline()){ + if (otherFaction.hasPlayersOnline()) + { if ( ! Conf.territoryDenyUseageMaterials.contains(material)) - { return true; // Item isn't one we're preventing for online factions. - } } else { if ( ! Conf.territoryDenyUseageMaterialsWhenOffline.contains(material)) - { return true; // Item isn't one we're preventing for offline factions. - } } - - if (otherFaction.isNone()) { if (!Conf.wildernessDenyUseage || Conf.worldsNoWildernessProtection.contains(location.getWorld().getName())) - { return true; // This is not faction territory. Use whatever you like here. - } if (!justCheck) - { me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in the wilderness."); - } + return false; } else if (otherFaction.isSafeZone()) { if (!Conf.safeZoneDenyUseage || Permission.MANAGE_SAFE_ZONE.has(player)) - { return true; - } + if (!justCheck) - { me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in a safe zone."); - } + return false; } else if (otherFaction.isWarZone()) { if (!Conf.warZoneDenyUseage || Permission.MANAGE_WAR_ZONE.has(player)) - { return true; - } + if (!justCheck) - { me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in a war zone."); - } + return false; } - + Faction myFaction = me.getFaction(); Relation rel = myFaction.getRelationTo(otherFaction); - boolean ownershipFail = Conf.ownedAreasEnabled && Conf.ownedAreaDenyUseage && !otherFaction.playerHasOwnershipRights(me, loc); - + // Cancel if we are not in our own territory - if (!rel.isMember() && rel.confDenyUseage()) + if (rel.confDenyUseage()) { if (!justCheck) - { 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 (rel.isMember() && ownershipFail && ! Permission.OWNERSHIP_BYPASS.has(player)) + if (Conf.ownedAreasEnabled && Conf.ownedAreaDenyUseage && !otherFaction.playerHasOwnershipRights(me, loc)) { if (!justCheck) - { - me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in this territory, it is owned by: "+myFaction.getOwnerListString(loc)); - } + me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in this territory, it is owned by: "+otherFaction.getOwnerListString(loc)); + return false; } - + return true; } @@ -414,9 +401,7 @@ public class FactionsPlayerListener extends PlayerListener { FPlayer me = FPlayers.i.get(player); if (me.isAdminBypassing()) - { return true; - } Material material = block.getType(); FLocation loc = new FLocation(block); @@ -424,47 +409,38 @@ public class FactionsPlayerListener extends PlayerListener // no door/chest/whatever protection in wilderness, war zones, or safe zones if (!otherFaction.isNormal()) - { return true; - } // We only care about some material types. if (otherFaction.hasPlayersOnline()) { if ( ! Conf.territoryProtectedMaterials.contains(material)) - { return true; - } } else { if ( ! Conf.territoryProtectedMaterialsWhenOffline.contains(material)) - { return true; - } } - - + Faction myFaction = me.getFaction(); Relation rel = myFaction.getRelationTo(otherFaction); - boolean ownershipFail = Conf.ownedAreasEnabled && Conf.ownedAreaProtectMaterials && !otherFaction.playerHasOwnershipRights(me, loc); - + // You may use any block unless it is another faction's territory... if (rel.isNeutral() || (rel.isEnemy() && Conf.territoryEnemyProtectMaterials) || (rel.isAlly() && Conf.territoryAllyProtectMaterials)) { if (!justCheck) - { 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 (rel.isMember() && ownershipFail && ! Permission.OWNERSHIP_BYPASS.has(player)) + if (Conf.ownedAreasEnabled && Conf.ownedAreaProtectMaterials && !otherFaction.playerHasOwnershipRights(me, loc)) { if (!justCheck) - { - me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in this territory, it is owned by: "+myFaction.getOwnerListString(loc)); - } + me.sendMessage("You can't use "+TextUtil.getMaterialName(material)+" in this territory, it is owned by: "+otherFaction.getOwnerListString(loc)); + return false; }