diff --git a/CHANGELOG.md b/CHANGELOG.md index 961df080b4..3b480c31eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -251,6 +251,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - removed the hearts of the Armorstand in the Menu Conversation IO - journal entries now keep their leading whitespaces - exceptions in `sudo` and `command` events leading to broken conversations that could not be exited +- the NPC's head could be stolen from conversations with the `chest` style - Things that are also fixed in 1.12.X: - eating of items when entering the chest conversation io actually consumed the item - legacy `§x` HEX color format not working in some contexts diff --git a/src/main/java/org/betonquest/betonquest/conversation/Conversation.java b/src/main/java/org/betonquest/betonquest/conversation/Conversation.java index 3707e7c9f4..a99e655b79 100644 --- a/src/main/java/org/betonquest/betonquest/conversation/Conversation.java +++ b/src/main/java/org/betonquest/betonquest/conversation/Conversation.java @@ -119,6 +119,11 @@ public class Conversation implements Listener { @SuppressWarnings("PMD.AvoidUsingVolatile") protected volatile ConversationState state = ConversationState.CREATED; + /** + * The next NPC option that will be printed. Set by {@link #selectOption(List, boolean)}. + */ + protected ResolvedOption nextNPCOption; + /** * The conversation data that is currently being used. */ @@ -129,11 +134,6 @@ public class Conversation implements Listener { */ private ConversationIO inOut; - /** - * The next option that will be printed. Set by {@link #selectOption(List, boolean)}. - */ - private ResolvedOption option; - /** * The {@link Interceptor} used to hide unrelated messages while the player is in this conversation. */ @@ -232,7 +232,7 @@ public static Conversation getConversation(final Profile profile) { private void selectOption(final List options, final boolean force) { final List inputOptions = force ? List.of(options.get(0)) : options; - this.option = null; + nextNPCOption = null; for (final ResolvedOption option : inputOptions) { // If we refer to another conversation starting options the name is null @@ -240,14 +240,14 @@ private void selectOption(final List options, final boolean forc for (final String startingOptionName : option.conversationData().getStartingOptions()) { if (force || BetonQuest.conditions(onlineProfile, option.conversationData().getConditionIDs(startingOptionName, NPC))) { this.data = option.conversationData(); - this.option = new ResolvedOption(option.conversationData(), NPC, startingOptionName); + this.nextNPCOption = new ResolvedOption(option.conversationData(), NPC, startingOptionName); break; } } } else { if (force || BetonQuest.conditions(onlineProfile, option.conversationData().getConditionIDs(option.name(), NPC))) { this.data = option.conversationData(); - this.option = option; + this.nextNPCOption = option; break; } } @@ -263,12 +263,12 @@ private void selectOption(final List options, final boolean forc */ private void printNPCText() { // if there are no possible options, end conversation - if (option == null) { + if (nextNPCOption == null) { new ConversationEnder().runTask(BetonQuest.getInstance()); return; } - String text = data.getText(onlineProfile, language, option); + String text = data.getText(onlineProfile, language, nextNPCOption); // resolve variables for (final String variable : BetonQuest.resolveVariables(text)) { text = text.replace(variable, plugin.getVariableValue(data.getPack().getQuestPath(), variable, onlineProfile)); @@ -278,7 +278,7 @@ private void printNPCText() { // print option to the player inOut.setNpcResponse(data.getQuester(language), text); - new NPCEventRunner(option).runTask(BetonQuest.getInstance()); + new NPCEventRunner(nextNPCOption).runTask(BetonQuest.getInstance()); } /** @@ -519,7 +519,7 @@ public void suspend() { inOut.end(); // save the conversation to the database - final PlayerConversationState state = new PlayerConversationState(identifier, option.name(), center); + final PlayerConversationState state = new PlayerConversationState(identifier, nextNPCOption.name(), center); plugin.getSaver().add(new Record(UpdateType.UPDATE_CONVERSATION, state.toString(), onlineProfile.getProfileUUID().toString())); // End interceptor @@ -700,7 +700,7 @@ public void run() { selectOption(resolvedOptions, false); // check whether to add a prefix - final String prefix = data.getPrefix(language, option); + final String prefix = data.getPrefix(language, nextNPCOption); String prefixName = null; String[] prefixVariables = null; if (prefix != null) { @@ -722,7 +722,7 @@ public void run() { } printNPCText(); - final ConversationOptionEvent optionEvent = new ConversationOptionEvent(PlayerConverter.getID(player), conv, option, conv.option); + final ConversationOptionEvent optionEvent = new ConversationOptionEvent(PlayerConverter.getID(player), conv, nextNPCOption, conv.nextNPCOption); new BukkitRunnable() { @@ -755,26 +755,26 @@ private List resolveOptions(final List startingOptions) } /** - * Fires events from the option. Should be called in the main thread. + * Fires events from an NPC option. Should be called on the main thread. */ private class NPCEventRunner extends BukkitRunnable { /** - * The option that has been selected and should be printed. + * The NPC option that has been selected and should be printed. */ - private final ResolvedOption option; + private final ResolvedOption npcOption; - public NPCEventRunner(final ResolvedOption option) { + public NPCEventRunner(final ResolvedOption npcOption) { super(); - this.option = option; + this.npcOption = npcOption; } @Override public void run() { - for (final EventID event : data.getEventIDs(onlineProfile, option, NPC)) { + for (final EventID event : data.getEventIDs(onlineProfile, npcOption, NPC)) { BetonQuest.event(onlineProfile, event); } - new OptionPrinter(option).runTaskAsynchronously(BetonQuest.getInstance()); + new OptionPrinter(npcOption).runTaskAsynchronously(BetonQuest.getInstance()); } } @@ -784,26 +784,26 @@ public void run() { private class PlayerEventRunner extends BukkitRunnable { /** - * The option that has been selected. + * The option that has been selected by the player. */ - private final ResolvedOption option; + private final ResolvedOption playerOption; /** * Creates a new PlayerEventRunner with the option that has been selected by the player. * - * @param option the option that has been selected by the player + * @param playerOption the option that has been selected by the player */ - public PlayerEventRunner(final ResolvedOption option) { + public PlayerEventRunner(final ResolvedOption playerOption) { super(); - this.option = option; + this.playerOption = playerOption; } @Override public void run() { - for (final EventID event : data.getEventIDs(onlineProfile, option, PLAYER)) { + for (final EventID event : data.getEventIDs(onlineProfile, playerOption, PLAYER)) { BetonQuest.event(onlineProfile, event); } - new ResponsePrinter(option).runTaskAsynchronously(BetonQuest.getInstance()); + new ResponsePrinter(playerOption).runTaskAsynchronously(BetonQuest.getInstance()); } } @@ -813,13 +813,13 @@ public void run() { private class ResponsePrinter extends BukkitRunnable { /** - * The option that has been selected and should be printed. + * The option that has been selected by the player and should be printed. */ - private final ResolvedOption option; + private final ResolvedOption playerOption; - public ResponsePrinter(final ResolvedOption option) { + public ResponsePrinter(final ResolvedOption playerOption) { super(); - this.option = option; + this.playerOption = playerOption; } @Override @@ -833,10 +833,10 @@ public void run() { return; } - selectOption(resolvePointers(option), false); + selectOption(resolvePointers(playerOption), false); printNPCText(); - final ConversationOptionEvent event = new ConversationOptionEvent(PlayerConverter.getID(player), conv, option, conv.option); + final ConversationOptionEvent event = new ConversationOptionEvent(PlayerConverter.getID(player), conv, playerOption, conv.nextNPCOption); new BukkitRunnable() { @Override @@ -854,18 +854,18 @@ public void run() { } /** - * Prints the options to the player. Should be called asynchronously. + * Prints possible player options to a NPC option to the player. Should be called asynchronously. */ private class OptionPrinter extends BukkitRunnable { /** * The option that has been selected and should be printed. */ - private final ResolvedOption option; + private final ResolvedOption npcOption; - public OptionPrinter(final ResolvedOption option) { + public OptionPrinter(final ResolvedOption npcOption) { super(); - this.option = option; + this.npcOption = npcOption; } @Override @@ -878,7 +878,7 @@ public void run() { if (!state.isActive()) { return; } - printOptions(resolvePointers(option)); + printOptions(resolvePointers(npcOption)); } catch (final InstructionParseException | ObjectNotFoundException e) { log.reportException(pack, e); throw new IllegalStateException("Cannot ensure a valid conversation flow with unresolvable options.", e); diff --git a/src/main/java/org/betonquest/betonquest/conversation/InventoryConvIO.java b/src/main/java/org/betonquest/betonquest/conversation/InventoryConvIO.java index 9c0224ee0e..5416c46ab9 100644 --- a/src/main/java/org/betonquest/betonquest/conversation/InventoryConvIO.java +++ b/src/main/java/org/betonquest/betonquest/conversation/InventoryConvIO.java @@ -9,11 +9,7 @@ import org.betonquest.betonquest.utils.LocalChatPaginator; import org.betonquest.betonquest.utils.PlayerConverter; import org.betonquest.betonquest.utils.Utils; -import org.bukkit.Bukkit; -import org.bukkit.ChatColor; -import org.bukkit.GameMode; -import org.bukkit.Location; -import org.bukkit.Material; +import org.bukkit.*; import org.bukkit.configuration.ConfigurationSection; import org.bukkit.entity.Player; import org.bukkit.event.EventHandler; @@ -26,6 +22,7 @@ import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import org.bukkit.inventory.meta.SkullMeta; +import org.jetbrains.annotations.NotNull; import java.util.ArrayList; import java.util.Arrays; @@ -35,8 +32,9 @@ /** * Inventory GUI for conversations. */ -@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.TooManyFields", "PMD.CommentRequired", "PMD.AvoidFieldNameMatchingMethodName", "PMD.AvoidLiteralsInIfCondition", "PMD.NPathComplexity"}) +@SuppressWarnings({"PMD.CyclomaticComplexity", "PMD.TooManyFields", "PMD.TooManyMethods", "PMD.CommentRequired", "PMD.AvoidFieldNameMatchingMethodName", "PMD.AvoidLiteralsInIfCondition", "PMD.NPathComplexity"}) public class InventoryConvIO implements Listener, ConversationIO { + private static final Map SKULL_CACHE = new HashMap<>(); /** @@ -70,7 +68,7 @@ public class InventoryConvIO implements Listener, ConversationIO { protected boolean processingLastClick; - protected boolean allowClose; + protected boolean allowListenerUnregister; protected boolean switching; @@ -153,18 +151,11 @@ public void display() { } if (player.getGameMode() == GameMode.SPECTATOR) { conv.endConversation(); - player.closeInventory(); + Bukkit.getScheduler().runTask(BetonQuest.getInstance(), () -> player.closeInventory()); conv.getInterceptor().sendMessage(Config.getMessage(PlayerConverter.getID(player).getProfileUUID().toString(), "conversation_spectator")); return; } - if (response == null) { - end(); - player.closeInventory(); - return; - } - if (options.isEmpty()) { - end(); - } + // each row contains 7 options, so get amount of rows int rows = options.size() / 7; rows++; @@ -172,37 +163,24 @@ public void display() { inv = Bukkit.createInventory(null, 9 * rows, "NPC"); inv.setContents(new ItemStack[9 * rows]); final ItemStack[] buttons = new ItemStack[9 * rows]; - // set the NPC head - final ItemStack npc; - if (SKULL_CACHE.containsKey(npcName)) { - log.debug(conv.getPackage(), "skull cache hit"); - npc = SKULL_CACHE.get(npcName); - } else { - log.debug(conv.getPackage(), "skull cache miss"); - npc = new ItemStack(Material.PLAYER_HEAD); - npc.setDurability((short) 3); - final SkullMeta npcMeta = (SkullMeta) npc.getItemMeta(); - npcMeta.setDisplayName(npcNameColor + npcName); - npc.setItemMeta(npcMeta); - Bukkit.getScheduler().runTaskAsynchronously(BetonQuest.getInstance(), () -> { - try { - npc.setItemMeta(updateSkullMeta((SkullMeta) npc.getItemMeta())); - Bukkit.getScheduler().runTask(BetonQuest.getInstance(), () -> { - SKULL_CACHE.put(npcName, npc); - inv.setItem(0, npc); - }); - } catch (final IllegalArgumentException e) { - log.debug(conv.getPackage(), "Could not load skull for chest conversation!", e); - } - }); + buttons[0] = createNpcHead(); + generateRows(rows, buttons); + + if (printMessages) { + conv.sendMessage(npcNameColor + npcName + ChatColor.RESET + ": " + npcTextColor + response); } - final SkullMeta npcMeta = (SkullMeta) npc.getItemMeta(); - npcMeta.setLore(Arrays.asList(LocalChatPaginator.wordWrap( - Utils.replaceReset(response, npcTextColor), 45))); - npc.setItemMeta(npcMeta); + Bukkit.getScheduler().runTask(BetonQuest.getInstance(), () -> { + inv.setContents(buttons); + switching = true; + player.openInventory(inv); + switching = false; + processingLastClick = false; + }); + } - buttons[0] = npc; + @SuppressWarnings("PMD.CognitiveComplexity") + private void generateRows(final int rows, final ItemStack... buttons) { // this is the number of an option int next = 0; // now fill the slots @@ -282,16 +260,39 @@ public void display() { item.setItemMeta(meta); buttons[j] = item; } - if (printMessages) { - conv.sendMessage(npcNameColor + npcName + ChatColor.RESET + ": " + npcTextColor + response); + } + + @NotNull + private ItemStack createNpcHead() { + final ItemStack npcHead; + if (SKULL_CACHE.containsKey(npcName)) { + log.debug(conv.getPackage(), "skull cache hit"); + npcHead = SKULL_CACHE.get(npcName); + } else { + log.debug(conv.getPackage(), "skull cache miss"); + npcHead = new ItemStack(Material.PLAYER_HEAD); + npcHead.setDurability((short) 3); + final SkullMeta npcMeta = (SkullMeta) npcHead.getItemMeta(); + npcMeta.setDisplayName(npcNameColor + npcName); + npcHead.setItemMeta(npcMeta); + Bukkit.getScheduler().runTaskAsynchronously(BetonQuest.getInstance(), () -> { + try { + npcHead.setItemMeta(updateSkullMeta((SkullMeta) npcHead.getItemMeta())); + Bukkit.getScheduler().runTask(BetonQuest.getInstance(), () -> { + SKULL_CACHE.put(npcName, npcHead); + inv.setItem(0, npcHead); + }); + } catch (final IllegalArgumentException e) { + log.debug(conv.getPackage(), "Could not load skull for chest conversation!", e); + } + }); } - Bukkit.getScheduler().runTask(BetonQuest.getInstance(), () -> { - inv.setContents(buttons); - switching = true; - player.openInventory(inv); - switching = false; - processingLastClick = false; - }); + + final SkullMeta npcMeta = (SkullMeta) npcHead.getItemMeta(); + npcMeta.setLore(Arrays.asList(LocalChatPaginator.wordWrap( + Utils.replaceReset(response, npcTextColor), 45))); + npcHead.setItemMeta(npcMeta); + return npcHead; } @SuppressWarnings("deprecation") @@ -352,7 +353,7 @@ public void onClose(final InventoryCloseEvent event) { return; } // allow closing when the conversation has finished - if (allowClose) { + if (allowListenerUnregister) { HandlerList.unregisterAll(this); return; } @@ -383,12 +384,25 @@ public void clear() { @Override public void end() { - allowClose = true; - if (inv != null) { - player.closeInventory(); + allowListenerUnregister = true; + if (mustBeClosed()) { + Bukkit.getScheduler().runTask(BetonQuest.getInstance(), () -> player.closeInventory()); } } + /** + * Checks whether the inventory must be closed when the conversation ends. + *

+ * If a conversation's next option (this was actually it's previous / last option because this is called at the + * conversation's ending) is null, the previous option was a player's response. If the player ended the + * conversation we want to close the inventory. + * + * @return true if the inventory must be closed + */ + private boolean mustBeClosed() { + return inv != null && conv.nextNPCOption == null; + } + @Override public boolean printMessages() { return printMessages;