Code smell - simplified listener

BentoBoxWorld · Oct 31, 2019 · 5555871 · 5555871
1 parent 6d788b6
commit 5555871
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 26 deletions.
diff --git a/src/main/java/world/bentobox/warps/WarpPanelManager.java b/src/main/java/world/bentobox/warps/WarpPanelManager.java
@@ -11,7 +11,6 @@
 import org.bukkit.World;
 import org.bukkit.inventory.ItemStack;
 
-import world.bentobox.bentobox.api.panels.Panel;
 import world.bentobox.bentobox.api.panels.PanelItem;
 import world.bentobox.bentobox.api.panels.builders.PanelBuilder;
 import world.bentobox.bentobox.api.panels.builders.PanelItemBuilder;
@@ -34,7 +33,7 @@ private PanelItem getPanelItem(World world, UUID warpOwner) {
         PanelItemBuilder pib = new PanelItemBuilder()
                 .name(addon.getSettings().getNameFormat() + addon.getPlugin().getPlayers().getName(warpOwner))
                 .description(getSign(world, warpOwner))
-                .clickHandler((panel, clicker, click, slot) -> hander(panel, world, clicker, warpOwner));
+                .clickHandler((panel, clicker, click, slot) -> hander(world, clicker, warpOwner));
         Material icon = getSignIcon(world, warpOwner);
         if (icon.equals(Material.PLAYER_HEAD)) {
             return pib.icon(addon.getPlayers().getName(warpOwner)).build();
@@ -43,7 +42,7 @@ private PanelItem getPanelItem(World world, UUID warpOwner) {
         }
     }
 
-    private boolean hander(Panel panel, World world, User clicker, UUID warpOwner) {
+    private boolean hander(World world, User clicker, UUID warpOwner) {
         clicker.closeInventory();
         addon.getWarpSignsManager().warpPlayer(world, clicker, warpOwner);
         return true;
@@ -53,7 +52,7 @@ private PanelItem getRandomButton(World world, User user, UUID warpOwner) {
         ///give @p minecraft:player_head{display:{Name:"{\"text\":\"Question Mark\"}"},SkullOwner:"MHF_Question"} 1
         return new PanelItemBuilder()
                 .name(addon.getSettings().getNameFormat() + user.getTranslation("warps.random"))
-                .clickHandler((panel, clicker, click, slot) -> hander(panel, world, clicker, warpOwner))
+                .clickHandler((panel, clicker, click, slot) -> hander(world, clicker, warpOwner))
                 .icon(Material.END_CRYSTAL).build();
     }
 

diff --git a/src/main/java/world/bentobox/warps/WarpSignsListener.java b/src/main/java/world/bentobox/warps/WarpSignsListener.java
@@ -9,6 +9,7 @@
 import org.bukkit.World;
 import org.bukkit.block.Block;
 import org.bukkit.block.Sign;
+import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
@@ -50,35 +51,38 @@ public void onSignBreak(BlockBreakEvent e) {
         boolean inWorld = addon.getPlugin().getIWM().inWorld(b.getWorld());
         // Signs only
         // FIXME: When we drop support for 1.13, switch to Tag.SIGNS
-        if (!e.getBlock().getType().name().contains("SIGN")) {
-            return;
-        }
-        if ((inWorld && !addon.inRegisteredWorld(b.getWorld())) || (!inWorld && !addon.getSettings().isAllowInOtherWorlds()) ) {
+        if (!e.getBlock().getType().name().contains("SIGN")
+                || (inWorld && !addon.inRegisteredWorld(b.getWorld()))
+                || (!inWorld && !addon.getSettings().isAllowInOtherWorlds()) ) {
             return;
         }
         User user = User.getInstance(e.getPlayer());
-        Sign s = (Sign) b.getState();
-        if (s.getLine(0).equalsIgnoreCase(ChatColor.GREEN + addon.getSettings().getWelcomeLine())) {
-            // Do a quick check to see if this sign location is in
-            // the list of warp signs
-            Map<UUID, Location> list = addon.getWarpSignsManager().getWarpMap(b.getWorld());
-            if (list.containsValue(s.getLocation())) {
-                // Welcome sign detected - check to see if it is
-                // this player's sign
-                String reqPerm = inWorld ? addon.getPermPrefix(e.getBlock().getWorld()) + "mod.removesign" : Warp.WELCOME_WARP_SIGNS + ".mod.removesign";
-                if ((list.containsKey(user.getUniqueId()) && list.get(user.getUniqueId()).equals(s.getLocation()))
-                        || user.isOp()  || user.hasPermission(reqPerm)) {
-                    addon.getWarpSignsManager().removeWarp(s.getLocation());
-                    Bukkit.getPluginManager().callEvent(new WarpRemoveEvent(addon, s.getLocation(), user.getUniqueId()));
-                } else {
-                    // Someone else's sign - not allowed
-                    user.sendMessage("warps.error.no-remove");
-                    e.setCancelled(true);
-                }
+        if (isWarpSign(b)) {
+            if (isPlayersSign(e.getPlayer(), b, inWorld)) {
+                addon.getWarpSignsManager().removeWarp(b.getLocation());
+                Bukkit.getPluginManager().callEvent(new WarpRemoveEvent(addon, b.getLocation(), user.getUniqueId()));
+            } else {
+                // Someone else's sign - not allowed
+                user.sendMessage("warps.error.no-remove");
+                e.setCancelled(true);
             }
         }
     }
 
+    private boolean isPlayersSign(Player player, Block b, boolean inWorld) {
+        // Welcome sign detected - check to see if it is this player's sign
+        Map<UUID, Location> list = addon.getWarpSignsManager().getWarpMap(b.getWorld());
+        String reqPerm = inWorld ? addon.getPermPrefix(b.getWorld()) + "mod.removesign" : Warp.WELCOME_WARP_SIGNS + ".mod.removesign";
+        return ((list.containsKey(player.getUniqueId()) && list.get(player.getUniqueId()).equals(b.getLocation()))
+                || player.isOp()  || player.hasPermission(reqPerm));
+    }
+
+    private boolean isWarpSign(Block b) {
+        Sign s = (Sign) b.getState();
+        return s.getLine(0).equalsIgnoreCase(ChatColor.GREEN + addon.getSettings().getWelcomeLine())
+                && addon.getWarpSignsManager().getWarpMap(b.getWorld()).containsValue(s.getLocation());
+    }
+
     /**
      * Event handler for Sign Changes
      *

diff --git a/src/test/java/world/bentobox/warps/WarpSignsListenerTest.java b/src/test/java/world/bentobox/warps/WarpSignsListenerTest.java
@@ -124,6 +124,7 @@ public void setUp() throws Exception {
         Location location = mock(Location.class);
         when(location.getBlock()).thenReturn(block);
         when(s.getLocation()).thenReturn(location);
+        when(block.getLocation()).thenReturn(location);
         list.put(uuid, location);
         // Player is in world
         when(wsm.getWarpMap(Mockito.eq(world))).thenReturn(list);