From 457b296372e397aa266db47d666e3beb1af114fa Mon Sep 17 00:00:00 2001 From: Florian CUNY Date: Sat, 12 Jan 2019 18:29:20 +0100 Subject: [PATCH] Fixed code smells --- .../api/commands/island/IslandBanCommand.java | 18 +++++++-------- .../commands/island/IslandUnbanCommand.java | 22 +++++++++---------- .../bentobox/database/objects/Island.java | 11 ++++------ .../bentobox/listeners/JoinLeaveListener.java | 7 ++---- .../bentobox/managers/FlagsManager.java | 3 --- 5 files changed, 25 insertions(+), 36 deletions(-) diff --git a/src/main/java/world/bentobox/bentobox/api/commands/island/IslandBanCommand.java b/src/main/java/world/bentobox/bentobox/api/commands/island/IslandBanCommand.java index ff1c741ef..bf39b1d09 100644 --- a/src/main/java/world/bentobox/bentobox/api/commands/island/IslandBanCommand.java +++ b/src/main/java/world/bentobox/bentobox/api/commands/island/IslandBanCommand.java @@ -97,17 +97,15 @@ public class IslandBanCommand extends CompositeCommand { .build(); // Event is not cancelled - if (!banEvent.isCancelled()) { - if (island.ban(issuer.getUniqueId(), target.getUniqueId())) { - issuer.sendMessage("general.success"); - target.sendMessage("commands.island.ban.owner-banned-you", TextVariables.NAME, issuer.getName()); - // If the player is online, has an island and on the banned island, move them home immediately - if (target.isOnline() && getIslands().hasIsland(getWorld(), target.getUniqueId()) && island.onIsland(target.getLocation())) { - getIslands().homeTeleport(getWorld(), target.getPlayer()); - island.getWorld().playSound(target.getLocation(), Sound.ENTITY_GENERIC_EXPLODE, 1F, 1F); - } - return true; + if (!banEvent.isCancelled() && island.ban(issuer.getUniqueId(), target.getUniqueId())) { + issuer.sendMessage("general.success"); + target.sendMessage("commands.island.ban.owner-banned-you", TextVariables.NAME, issuer.getName()); + // If the player is online, has an island and on the banned island, move them home immediately + if (target.isOnline() && getIslands().hasIsland(getWorld(), target.getUniqueId()) && island.onIsland(target.getLocation())) { + getIslands().homeTeleport(getWorld(), target.getPlayer()); + island.getWorld().playSound(target.getLocation(), Sound.ENTITY_GENERIC_EXPLODE, 1F, 1F); } + return true; } } else { issuer.sendMessage("commands.island.ban.cannot-ban-more-players"); diff --git a/src/main/java/world/bentobox/bentobox/api/commands/island/IslandUnbanCommand.java b/src/main/java/world/bentobox/bentobox/api/commands/island/IslandUnbanCommand.java index b2805132a..696d7c483 100644 --- a/src/main/java/world/bentobox/bentobox/api/commands/island/IslandUnbanCommand.java +++ b/src/main/java/world/bentobox/bentobox/api/commands/island/IslandUnbanCommand.java @@ -67,26 +67,26 @@ public class IslandUnbanCommand extends CompositeCommand { } private boolean unban(User issuer, User target) { + Island island = getIslands().getIsland(getWorld(), issuer.getUniqueId()); + // Run the event IslandBaseEvent unbanEvent = IslandEvent.builder() - .island(getIslands().getIsland(getWorld(), issuer.getUniqueId())) + .island(island) .involvedPlayer(target.getUniqueId()) .admin(false) .reason(IslandEvent.Reason.UNBAN) .build(); // Event is not cancelled - if (!unbanEvent.isCancelled()) { - if (getIslands().getIsland(getWorld(), issuer.getUniqueId()).unban(issuer.getUniqueId(), target.getUniqueId())) { - issuer.sendMessage("general.success"); - target.sendMessage("commands.island.unban.you-are-unbanned", TextVariables.NAME, issuer.getName()); - // Set cooldown - if (getSettings().getBanCooldown() > 0 && getParent() != null) { - getParent().getSubCommand("ban").ifPresent(subCommand -> - subCommand.setCooldown(issuer.getUniqueId(), target.getUniqueId(), getSettings().getBanCooldown() * 60)); - } - return true; + if (!unbanEvent.isCancelled() && island.unban(issuer.getUniqueId(), target.getUniqueId())) { + issuer.sendMessage("general.success"); + target.sendMessage("commands.island.unban.you-are-unbanned", TextVariables.NAME, issuer.getName()); + // Set cooldown + if (getSettings().getBanCooldown() > 0 && getParent() != null) { + getParent().getSubCommand("ban").ifPresent(subCommand -> + subCommand.setCooldown(issuer.getUniqueId(), target.getUniqueId(), getSettings().getBanCooldown() * 60)); } + return true; } // Unbanning was blocked, maybe due to an event cancellation. Fail silently. return false; diff --git a/src/main/java/world/bentobox/bentobox/database/objects/Island.java b/src/main/java/world/bentobox/bentobox/database/objects/Island.java index 3f337dec7..3cc5f7a5a 100644 --- a/src/main/java/world/bentobox/bentobox/database/objects/Island.java +++ b/src/main/java/world/bentobox/bentobox/database/objects/Island.java @@ -133,15 +133,12 @@ public class Island implements DataObject { * @param issuer UUID of the issuer, may be null. * Whenever possible, one should be provided. * @param target UUID of the target, must be provided. - * @return {@code true} if the target is successfully banned, {@code false} otherwise. + * @return {@code true} */ public boolean ban(@Nullable UUID issuer, @NonNull UUID target) { - if (target != null) { - setRank(target, RanksManager.BANNED_RANK); - log(new LogEntry.Builder("BAN").data("player", target).data("issuer", issuer).build()); - return true; - } - return false; + setRank(target, RanksManager.BANNED_RANK); + log(new LogEntry.Builder("BAN").data("player", target).data("issuer", issuer).build()); + return true; } /** diff --git a/src/main/java/world/bentobox/bentobox/listeners/JoinLeaveListener.java b/src/main/java/world/bentobox/bentobox/listeners/JoinLeaveListener.java index b45fdc616..311f9dd52 100644 --- a/src/main/java/world/bentobox/bentobox/listeners/JoinLeaveListener.java +++ b/src/main/java/world/bentobox/bentobox/listeners/JoinLeaveListener.java @@ -98,11 +98,8 @@ public class JoinLeaveListener implements Listener { && Bukkit.getOfflinePlayer(uuid).getLastPlayed() < timestamp) .toArray()); - if (!candidates.isEmpty()) { - if (!plugin.getSettings().isAutoOwnershipTransferIgnoreRanks()) { - // Ranks are not ignored, our candidates can only have the highest rank - - } + if (!candidates.isEmpty() && !plugin.getSettings().isAutoOwnershipTransferIgnoreRanks()) { + // Ranks are not ignored, our candidates can only have the highest rank } } }); diff --git a/src/main/java/world/bentobox/bentobox/managers/FlagsManager.java b/src/main/java/world/bentobox/bentobox/managers/FlagsManager.java index a4ce0f597..906364360 100644 --- a/src/main/java/world/bentobox/bentobox/managers/FlagsManager.java +++ b/src/main/java/world/bentobox/bentobox/managers/FlagsManager.java @@ -43,9 +43,6 @@ public class FlagsManager { * @return true if successfully registered, false if not, e.g., because one with the same ID already exists */ public boolean registerFlag(@NonNull Flag flag) { - if (flag == null) { - return false; - } // Check in case the flag id or icon already exists for (Flag fl : flags) { if (fl.getID().equals(flag.getID()) || fl.getIcon().equals(flag.getIcon())) {