Fixed code smells

This commit is contained in:
tastybento 2019-10-31 14:12:44 -07:00
parent 007f071234
commit 85334079da
10 changed files with 241 additions and 225 deletions

13
pom.xml
View File

@ -189,6 +189,15 @@
<version>${level.version}</version>
<scope>provided</scope>
</dependency>
<!-- Static analysis -->
<!-- We are using Eclipse's annotations. If you're using IDEA, update
your project settings to take these into account for in real time static
analysis -->
<dependency>
<groupId>org.eclipse.jdt</groupId>
<artifactId>org.eclipse.jdt.annotation</artifactId>
<version>2.2.200</version>
</dependency>
</dependencies>
<build>
@ -300,8 +309,8 @@
<configuration>
<append>true</append>
<excludes>
<!-- This is required to prevent Jacoco from adding synthetic fields
to a JavaBean class (causes errors in testing) -->
<!-- This is required to prevent Jacoco from adding
synthetic fields to a JavaBean class (causes errors in testing) -->
<exclude>**/*Names*</exclude>
</excludes>
</configuration>

View File

@ -115,9 +115,8 @@ public class Warp extends Addon {
// Register commands
this.getPlugin().getAddonsManager().getGameModeAddons().forEach(gameModeAddon -> {
if (!this.settings.getDisabledGameModes().contains(gameModeAddon.getDescription().getName()))
{
if (gameModeAddon.getPlayerCommand().isPresent())
if (!this.settings.getDisabledGameModes().contains(gameModeAddon.getDescription().getName())
&& gameModeAddon.getPlayerCommand().isPresent())
{
this.registeredWorlds.add(gameModeAddon.getOverWorld());
@ -125,7 +124,6 @@ public class Warp extends Addon {
new WarpsCommand(this, gameModeAddon.getPlayerCommand().get());
this.hooked = true;
}
}
});
if (hooked)

View File

@ -11,6 +11,7 @@ import org.bukkit.Material;
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;
@ -33,12 +34,7 @@ public class WarpPanelManager {
PanelItemBuilder pib = new PanelItemBuilder()
.name(addon.getSettings().getNameFormat() + addon.getPlugin().getPlayers().getName(warpOwner))
.description(getSign(world, warpOwner))
.clickHandler((panel, clicker, click, slot) -> { {
clicker.closeInventory();
addon.getWarpSignsManager().warpPlayer(world, clicker, warpOwner);
return true;
}
});
.clickHandler((panel, clicker, click, slot) -> hander(panel, world, clicker, warpOwner));
Material icon = getSignIcon(world, warpOwner);
if (icon.equals(Material.PLAYER_HEAD)) {
return pib.icon(addon.getPlayers().getName(warpOwner)).build();
@ -47,16 +43,17 @@ public class WarpPanelManager {
}
}
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) -> { {
private boolean hander(Panel panel, World world, User clicker, UUID warpOwner) {
clicker.closeInventory();
addon.getWarpSignsManager().warpPlayer(world, clicker, warpOwner);
return true;
}
})
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))
.icon(Material.END_CRYSTAL).build();
}
@ -110,7 +107,7 @@ public class WarpPanelManager {
}
PanelBuilder panelBuilder = new PanelBuilder()
.user(user)
.name(user.getTranslation("warps.title") + " " + String.valueOf(index + 1));
.name(user.getTranslation("warps.title") + " " + (index + 1));
int i = index * PANEL_MAX_SIZE;
for (; i < (index * PANEL_MAX_SIZE + PANEL_MAX_SIZE) && i < warps.size(); i++) {

View File

@ -6,6 +6,7 @@ import java.util.UUID;
import org.bukkit.Bukkit;
import org.bukkit.ChatColor;
import org.bukkit.Location;
import org.bukkit.World;
import org.bukkit.block.Block;
import org.bukkit.block.Sign;
import org.bukkit.event.EventHandler;
@ -46,20 +47,17 @@ public class WarpSignsListener implements Listener {
@EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true)
public void onSignBreak(BlockBreakEvent e) {
Block b = e.getBlock();
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 ((addon.getPlugin().getIWM().inWorld(b.getWorld()) && !addon.inRegisteredWorld(b.getWorld()))
|| (!addon.getPlugin().getIWM().inWorld(b.getWorld()) && !addon.getSettings().isAllowInOtherWorlds()) ) {
if ((inWorld && !addon.inRegisteredWorld(b.getWorld())) || (!inWorld && !addon.getSettings().isAllowInOtherWorlds()) ) {
return;
}
User user = User.getInstance(e.getPlayer());
Sign s = (Sign) b.getState();
if (s == null) {
return;
}
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
@ -67,8 +65,9 @@ public class WarpSignsListener implements Listener {
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" : "welcomewarpsigns.mod.removesign";
if ((list.containsKey(user.getUniqueId()) && list.get(user.getUniqueId()).equals(s.getLocation()))
|| user.isOp() || user.hasPermission(addon.getPermPrefix(e.getBlock().getWorld()) + "mod.removesign")) {
|| user.isOp() || user.hasPermission(reqPerm)) {
addon.getWarpSignsManager().removeWarp(s.getLocation());
Bukkit.getPluginManager().callEvent(new WarpRemoveEvent(addon, s.getLocation(), user.getUniqueId()));
} else {
@ -88,8 +87,8 @@ public class WarpSignsListener implements Listener {
@EventHandler(priority = EventPriority.NORMAL, ignoreCancelled = true)
public void onSignWarpCreate(SignChangeEvent e) {
Block b = e.getBlock();
if ((addon.getPlugin().getIWM().inWorld(b.getWorld()) && !addon.inRegisteredWorld(b.getWorld()))
|| (!addon.getPlugin().getIWM().inWorld(b.getWorld()) && !addon.getSettings().isAllowInOtherWorlds()) ) {
boolean inWorld = addon.getPlugin().getIWM().inWorld(b.getWorld());
if ((inWorld && !addon.inRegisteredWorld(b.getWorld())) || (!inWorld && !addon.getSettings().isAllowInOtherWorlds()) ) {
return;
}
String title = e.getLine(0);
@ -97,29 +96,13 @@ public class WarpSignsListener implements Listener {
// Check if someone is changing their own sign
if (title.equalsIgnoreCase(addon.getSettings().getWelcomeLine())) {
// Welcome sign detected - check permissions
if (!(user.hasPermission(addon.getPermPrefix(b.getWorld()) + "island.addwarp"))) {
user.sendMessage("warps.error.no-permission");
user.sendMessage("general.errors.no-permission", "[permission]", addon.getPermPrefix(b.getWorld()) + "island.addwarp");
if (noPerms(user, b.getWorld(), inWorld)) {
return;
}
if (addon.getPlugin().getIWM().inWorld(b.getWorld())) {
// Get level if level addon is available
Long level = addon.getLevel(Util.getWorld(b.getWorld()), user.getUniqueId());
if (level != null && level < addon.getSettings().getWarpLevelRestriction()) {
user.sendMessage("warps.error.not-enough-level");
user.sendMessage("warps.error.your-level-is",
"[level]", String.valueOf(level),
"[required]", String.valueOf(addon.getSettings().getWarpLevelRestriction()));
return;
}
// Check that the player is on their island
if (!(plugin.getIslands().userIsOnIsland(b.getWorld(), user))) {
user.sendMessage("warps.error.not-on-island");
if (inWorld && noLevelOrIsland(user, b.getWorld())) {
e.setLine(0, ChatColor.RED + addon.getSettings().getWelcomeLine());
return;
}
}
// Check if the player already has a sign
final Location oldSignLoc = addon.getWarpSignsManager().getWarp(b.getWorld(), user.getUniqueId());
if (oldSignLoc == null) {
@ -135,7 +118,6 @@ public class WarpSignsListener implements Listener {
if (oldSignBlock.getType().name().contains("SIGN")) {
// The block is still a sign
Sign oldSign = (Sign) oldSignBlock.getState();
if (oldSign != null) {
if (oldSign.getLine(0).equalsIgnoreCase(ChatColor.GREEN + addon.getSettings().getWelcomeLine())) {
oldSign.setLine(0, ChatColor.RED + addon.getSettings().getWelcomeLine());
oldSign.update(true, false);
@ -144,7 +126,6 @@ public class WarpSignsListener implements Listener {
Bukkit.getPluginManager().callEvent(new WarpRemoveEvent(addon, oldSign.getLocation(), user.getUniqueId()));
}
}
}
// Set up the new warp sign
addSign(e, user, b);
}
@ -152,6 +133,42 @@ public class WarpSignsListener implements Listener {
}
private boolean noLevelOrIsland(User user, World world) {
// Get level if level addon is available
Long level = addon.getLevel(Util.getWorld(world), user.getUniqueId());
if (level != null && level < addon.getSettings().getWarpLevelRestriction()) {
user.sendMessage("warps.error.not-enough-level");
user.sendMessage("warps.error.your-level-is",
"[level]", String.valueOf(level),
"[required]", String.valueOf(addon.getSettings().getWarpLevelRestriction()));
return true;
}
// Check that the player is on their island
if (!(plugin.getIslands().userIsOnIsland(world, user))) {
user.sendMessage("warps.error.not-on-island");
return true;
}
return false;
}
/**
* Check if player has permission to execute command
* @param user - user
* @param world - world that the warp is in
* @param inWorld - true if warp is in a game world
* @return true if player does not have the required perms, false otherwise
*/
private boolean noPerms(User user, World world, boolean inWorld) {
String permReq = inWorld ? addon.getPermPrefix(world) + "island.addwarp" : "welcomewarpsigns.addwarp";
if (!(user.hasPermission(permReq))) {
user.sendMessage("warps.error.no-permission");
user.sendMessage("general.errors.no-permission", "[permission]", permReq);
return true;
}
return false;
}
private void addSign(SignChangeEvent e, User user, Block b) {
if (addon.getWarpSignsManager().addWarp(user.getUniqueId(), b.getLocation())) {
user.sendMessage("warps.success");

View File

@ -25,6 +25,8 @@ import org.bukkit.block.Block;
import org.bukkit.block.BlockFace;
import org.bukkit.block.Sign;
import org.bukkit.permissions.PermissionAttachmentInfo;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;
import world.bentobox.bentobox.BentoBox;
import world.bentobox.bentobox.api.addons.Addon;
@ -59,7 +61,8 @@ public class WarpSignsManager {
* @param world - world
* @return map of warps
*/
public Map<UUID, Location> getWarpMap(World world) {
@NonNull
public Map<UUID, Location> getWarpMap(@Nullable World world) {
return worldsWarpList.computeIfAbsent(Util.getWorld(world), k -> new HashMap<>());
}
@ -108,6 +111,7 @@ public class WarpSignsManager {
* - the warp requested
* @return Location of warp or null
*/
@Nullable
public Location getWarp(World world, UUID playerUUID) {
return getWarpMap(world).get(playerUUID);
}
@ -117,6 +121,7 @@ public class WarpSignsManager {
* @param location to search
* @return Name of warp owner or empty string if there is none
*/
@NonNull
public String getWarpOwner(Location location) {
return getWarpMap(location.getWorld()).entrySet().stream().filter(en -> en.getValue().equals(location))
.findFirst().map(en -> plugin.getPlayers().getName(en.getKey())).orElse("");
@ -126,11 +131,12 @@ public class WarpSignsManager {
* Get sorted list of warps with most recent players listed first
* @return UUID list
*/
public List<UUID> getSortedWarps(World world) {
@NonNull
public List<UUID> getSortedWarps(@NonNull World world) {
// Remove any null locations - this can happen if an admin changes the name of the world and signs point to old locations
getWarpMap(world).values().removeIf(Objects::isNull);
// Bigger value of time means a more recent login
TreeMap<Long, UUID> map = new TreeMap<Long, UUID>();
TreeMap<Long, UUID> map = new TreeMap<>();
getWarpMap(world).entrySet().forEach(en -> {
UUID uuid = en.getKey();
// If never played, will be zero
@ -160,7 +166,8 @@ public class WarpSignsManager {
*
* @return UUID set of warps
*/
public Set<UUID> listWarps(World world) {
@NonNull
public Set<UUID> listWarps(@NonNull World world) {
// Remove any null locations
getWarpMap(world).values().removeIf(Objects::isNull);
return getWarpMap(world).entrySet().stream().filter(e -> Util.sameWorld(world, e.getValue().getWorld())).map(Map.Entry::getKey).collect(Collectors.toSet());
@ -194,14 +201,12 @@ public class WarpSignsManager {
Block b = loc.getBlock();
if (b.getType().name().contains("SIGN")) {
Sign s = (Sign) b.getState();
if (s != null) {
if (s.getLine(0).equalsIgnoreCase(ChatColor.GREEN + addon.getSettings().getWelcomeLine())) {
s.setLine(0, ChatColor.RED + addon.getSettings().getWelcomeLine());
s.update(true, false);
}
}
}
}
/**
* Removes a warp at a location.
@ -257,7 +262,8 @@ public class WarpSignsManager {
* @param uuid - player's uuid
* @return Sign's content and type
*/
public SignCache getSignInfo(World world, UUID uuid) {
@NonNull
public SignCache getSignInfo(@NonNull World world, @NonNull UUID uuid) {
List<String> result = new ArrayList<>();
//get the sign info
Location signLocation = getWarp(world, uuid);
@ -309,7 +315,7 @@ public class WarpSignsManager {
* @param directionFacing - direction that sign is facing
* @param pvp - true if this location allowed PVP
*/
private void warpPlayer(User user, Location inFront, UUID signOwner, BlockFace directionFacing, boolean pvp) {
private void warpPlayer(@NonNull User user, @NonNull Location inFront, @NonNull UUID signOwner, @NonNull BlockFace directionFacing, boolean pvp) {
// convert blockface to angle
float yaw = blockFaceToFloat(directionFacing);
final Location actualWarp = new Location(inFront.getWorld(), inFront.getBlockX() + 0.5D, inFront.getBlockY(),
@ -334,7 +340,7 @@ public class WarpSignsManager {
* @param face
* @return degrees
*/
private float blockFaceToFloat(BlockFace face) {
private float blockFaceToFloat(@NonNull BlockFace face) {
switch (face) {
case EAST:
return 90F;
@ -380,7 +386,7 @@ public class WarpSignsManager {
* @param user - user who is warping
* @param owner - owner of the warp
*/
public void warpPlayer(World world, User user, UUID owner) {
public void warpPlayer(@NonNull World world, @NonNull User user, @NonNull UUID owner) {
final Location warpSpot = getWarp(world, owner);
// Check if the warp spot is safe
if (warpSpot == null) {
@ -447,7 +453,6 @@ public class WarpSignsManager {
}
if (!(plugin.getIslands().isSafeLocation(warpSpot))) {
user.sendMessage("warps.error.not-safe");
return;
} else {
final Location actualWarp = new Location(warpSpot.getWorld(), warpSpot.getBlockX() + 0.5D, warpSpot.getBlockY(),
warpSpot.getBlockZ() + 0.5D);
@ -458,7 +463,6 @@ public class WarpSignsManager {
user.getWorld().playSound(user.getLocation(), Sound.ENTITY_BAT_TAKEOFF, 1F, 1F);
}
user.teleport(actualWarp);
return;
}
}
@ -467,7 +471,7 @@ public class WarpSignsManager {
* @param playerUUID - player's UUID
* @return true if they have warp
*/
public boolean hasWarp(World world, UUID playerUUID) {
public boolean hasWarp(@NonNull World world, @NonNull UUID playerUUID) {
return getWarpMap(world).containsKey(playerUUID);
}
@ -485,7 +489,7 @@ public class WarpSignsManager {
* @param defaultValue Default value that will be returned if permission not found.
* @return String value that follows permissionPrefix.
*/
private String getPermissionValue(User user, String permissionPrefix, String defaultValue)
private String getPermissionValue(@NonNull User user, @NonNull String permissionPrefix, @NonNull String defaultValue)
{
if (user.isPlayer())
{

View File

@ -54,7 +54,7 @@ public class WarpCommand extends CompositeCommand {
return true;
} else {
// Check if this is part of a name
UUID foundWarp = warpList.stream().filter(u -> getPlayers().getName(u).toLowerCase().equals(args.get(0).toLowerCase())
UUID foundWarp = warpList.stream().filter(u -> getPlayers().getName(u).equalsIgnoreCase(args.get(0))
|| getPlayers().getName(u).toLowerCase().startsWith(args.get(0).toLowerCase())).findFirst().orElse(null);
if (foundWarp == null) {
user.sendMessage("warps.error.does-not-exist");

View File

@ -63,7 +63,6 @@ public class WarpInitiateEvent extends Event implements Cancellable {
@Override
public boolean isCancelled() {
// TODO Auto-generated method stub
return cancelled;
}

View File

@ -18,7 +18,9 @@ public class WarpsData implements DataObject {
@Expose
private Map<Location, UUID> warpSigns = new HashMap<>();
public WarpsData() {}
public WarpsData() {
// Required by YAML database
}
@Override
public String getUniqueId() {

View File

@ -203,16 +203,6 @@ public class WarpSignsListenerTest {
verify(addon).inRegisteredWorld(Mockito.eq(world));
}
@Test
public void testOnSignBreakNullState() {
WarpSignsListener wsl = new WarpSignsListener(addon);
BlockBreakEvent e = new BlockBreakEvent(block, player);
when(block.getState()).thenReturn(null);
wsl.onSignBreak(e);
assertFalse(e.isCancelled());
verify(block).getState();
}
@Test
public void testOnSignNotWelcomeSign() {
WarpSignsListener wsl = new WarpSignsListener(addon);