diff --git a/CHANGELOG.md b/CHANGELOG.md index 12dcaa7..58204a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Unreleased] +### Fixed +- `/pl edit` and `/pl remove` now treat `lineIndex` as 1-based, matching `COMMANDS.md`/`USER_GUIDE.md`, instead of silently indexing into the lore list as 0-based +- `/pl edit` and `/pl remove` no longer throw an uncaught exception when given a non-numeric index or no index at all; they now send a player-facing error message +- The default (no-argument) command no longer tells players to type the non-existent `/lp help`; it now correctly says `/pl help` + ## [1.1] ### Added diff --git a/pom.xml b/pom.xml index f311441..f072e91 100644 --- a/pom.xml +++ b/pom.xml @@ -14,6 +14,7 @@ 1.8 UTF-8 + 5.10.2 @@ -27,6 +28,11 @@ ${java.version} + + org.apache.maven.plugins + maven-surefire-plugin + 3.2.5 + org.apache.maven.plugins maven-shade-plugin @@ -80,5 +86,17 @@ 1.0 compile + + org.junit.jupiter + junit-jupiter + ${junit.version} + test + + + org.mockito + mockito-core + 5.11.0 + test + diff --git a/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java b/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java index 455ebe1..1f63b4b 100644 --- a/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java +++ b/src/main/java/dansplugins/playerlore/commands/DefaultCommand.java @@ -27,7 +27,7 @@ public boolean execute(CommandSender commandSender) { commandSender.sendMessage(ChatColor.AQUA + "Requested by: Rochelle"); commandSender.sendMessage(ChatColor.AQUA + "Wiki: https://github.com/dmccoystephenson/PlayerLore/wiki"); commandSender.sendMessage(""); - commandSender.sendMessage(ChatColor.AQUA + "For a list of commands, type /lp help"); + commandSender.sendMessage(ChatColor.AQUA + "For a list of commands, type /pl help"); return true; } diff --git a/src/main/java/dansplugins/playerlore/commands/EditCommand.java b/src/main/java/dansplugins/playerlore/commands/EditCommand.java index fdbac37..79bd7d5 100644 --- a/src/main/java/dansplugins/playerlore/commands/EditCommand.java +++ b/src/main/java/dansplugins/playerlore/commands/EditCommand.java @@ -38,7 +38,18 @@ public boolean execute(CommandSender commandSender, String[] args) { Player player = (Player) commandSender; // get line to edit - int lineIndex = Integer.parseInt(args[0]); + if (args.length == 0) { + player.sendMessage(ChatColor.RED + "Usage: /pl edit (lineIndex) \"new line of lore\""); + return false; + } + + int lineIndex; + try { + lineIndex = Integer.parseInt(args[0]); + } catch (NumberFormatException e) { + player.sendMessage(ChatColor.RED + "Line index must be a number."); + return false; + } // get line of lore ArgumentParser argumentParser = new ArgumentParser(); @@ -69,12 +80,12 @@ public boolean execute(CommandSender commandSender, String[] args) { lore = new ArrayList<>(); } - if (lineIndex >= lore.size()) { + if (lineIndex < 1 || lineIndex > lore.size()) { player.sendMessage(ChatColor.RED + "There aren't that many lines of lore."); return false; } - lore.set(lineIndex, ChatColor.WHITE + lineOfLore); + lore.set(lineIndex - 1, ChatColor.WHITE + lineOfLore); itemMeta.setLore(lore); item.setItemMeta(itemMeta); diff --git a/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java b/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java index 881d50a..bf0ee36 100644 --- a/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java +++ b/src/main/java/dansplugins/playerlore/commands/RemoveCommand.java @@ -37,7 +37,18 @@ public boolean execute(CommandSender commandSender, String[] args) { Player player = (Player) commandSender; // get line to edit - int lineIndex = Integer.parseInt(args[0]); + if (args.length == 0) { + player.sendMessage(ChatColor.RED + "Usage: /pl remove (lineIndex)"); + return false; + } + + int lineIndex; + try { + lineIndex = Integer.parseInt(args[0]); + } catch (NumberFormatException e) { + player.sendMessage(ChatColor.RED + "Line index must be a number."); + return false; + } // get item ItemStack item = player.getInventory().getItemInMainHand(); @@ -59,12 +70,12 @@ public boolean execute(CommandSender commandSender, String[] args) { lore = new ArrayList<>(); } - if (lineIndex >= lore.size()) { + if (lineIndex < 1 || lineIndex > lore.size()) { player.sendMessage(ChatColor.RED + "There aren't that many lines of lore."); return false; } - lore.remove(lineIndex); + lore.remove(lineIndex - 1); itemMeta.setLore(lore); item.setItemMeta(itemMeta); diff --git a/src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java b/src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java new file mode 100644 index 0000000..05ccd2a --- /dev/null +++ b/src/test/java/dansplugins/playerlore/commands/DefaultCommandTest.java @@ -0,0 +1,23 @@ +package dansplugins.playerlore.commands; + +import static org.mockito.ArgumentMatchers.contains; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import dansplugins.playerlore.PlayerLore; +import org.bukkit.command.CommandSender; +import org.junit.jupiter.api.Test; + +public class DefaultCommandTest { + + @Test + public void execute_pointsPlayersToTheRegisteredHelpCommand() { + PlayerLore playerLore = mock(PlayerLore.class); + CommandSender commandSender = mock(CommandSender.class); + DefaultCommand defaultCommand = new DefaultCommand(playerLore); + + defaultCommand.execute(commandSender); + + verify(commandSender).sendMessage(contains("/pl help")); + } +} diff --git a/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java b/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java new file mode 100644 index 0000000..b99212e --- /dev/null +++ b/src/test/java/dansplugins/playerlore/commands/EditCommandTest.java @@ -0,0 +1,90 @@ +package dansplugins.playerlore.commands; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.bukkit.ChatColor; +import org.bukkit.Material; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.inventory.meta.ItemMeta; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class EditCommandTest { + + private EditCommand editCommand; + private Player player; + private PlayerInventory inventory; + private ItemStack item; + private ItemMeta itemMeta; + + @BeforeEach + public void setUp() { + editCommand = new EditCommand(); + player = mock(Player.class); + inventory = mock(PlayerInventory.class); + item = mock(ItemStack.class); + itemMeta = mock(ItemMeta.class); + + when(player.getInventory()).thenReturn(inventory); + when(inventory.getItemInMainHand()).thenReturn(item); + when(item.getType()).thenReturn(Material.DIAMOND_SWORD); + when(item.getItemMeta()).thenReturn(itemMeta); + } + + @Test + public void execute_editsFirstLoreLineUsing1BasedIndex() { + List lore = new ArrayList<>(Arrays.asList("original line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = editCommand.execute(player, new String[]{"1", "\"new line\""}); + + assertTrue(result); + verify(itemMeta).setLore(Arrays.asList(ChatColor.WHITE + "new line")); + } + + @Test + public void execute_rejectsIndexBelow1() { + List lore = new ArrayList<>(Arrays.asList("original line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = editCommand.execute(player, new String[]{"0", "\"new line\""}); + + assertFalse(result); + verify(player).sendMessage(anyString()); + } + + @Test + public void execute_rejectsIndexBeyondLoreSize() { + List lore = new ArrayList<>(Arrays.asList("original line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = editCommand.execute(player, new String[]{"2", "\"new line\""}); + + assertFalse(result); + } + + @Test + public void execute_rejectsNonNumericIndex() { + boolean result = editCommand.execute(player, new String[]{"abc", "\"new line\""}); + + assertFalse(result); + } + + @Test + public void execute_rejectsMissingArgs() { + boolean result = editCommand.execute(player, new String[]{}); + + assertFalse(result); + } +} diff --git a/src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java b/src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java new file mode 100644 index 0000000..55d89b9 --- /dev/null +++ b/src/test/java/dansplugins/playerlore/commands/RemoveCommandTest.java @@ -0,0 +1,87 @@ +package dansplugins.playerlore.commands; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +import org.bukkit.Material; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.PlayerInventory; +import org.bukkit.inventory.meta.ItemMeta; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class RemoveCommandTest { + + private RemoveCommand removeCommand; + private Player player; + private PlayerInventory inventory; + private ItemStack item; + private ItemMeta itemMeta; + + @BeforeEach + public void setUp() { + removeCommand = new RemoveCommand(); + player = mock(Player.class); + inventory = mock(PlayerInventory.class); + item = mock(ItemStack.class); + itemMeta = mock(ItemMeta.class); + + when(player.getInventory()).thenReturn(inventory); + when(inventory.getItemInMainHand()).thenReturn(item); + when(item.getType()).thenReturn(Material.DIAMOND_SWORD); + when(item.getItemMeta()).thenReturn(itemMeta); + } + + @Test + public void execute_removesFirstLoreLineUsing1BasedIndex() { + List lore = new ArrayList<>(Arrays.asList("first line", "second line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = removeCommand.execute(player, new String[]{"1"}); + + assertTrue(result); + verify(itemMeta).setLore(Arrays.asList("second line")); + } + + @Test + public void execute_rejectsIndexBelow1() { + List lore = new ArrayList<>(Arrays.asList("first line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = removeCommand.execute(player, new String[]{"0"}); + + assertFalse(result); + } + + @Test + public void execute_rejectsIndexBeyondLoreSize() { + List lore = new ArrayList<>(Arrays.asList("first line")); + when(itemMeta.getLore()).thenReturn(lore); + + boolean result = removeCommand.execute(player, new String[]{"2"}); + + assertFalse(result); + } + + @Test + public void execute_rejectsNonNumericIndex() { + boolean result = removeCommand.execute(player, new String[]{"abc"}); + + assertFalse(result); + } + + @Test + public void execute_rejectsMissingArgs() { + boolean result = removeCommand.execute(player, new String[]{}); + + assertFalse(result); + } +}