From 0f6c2ca62ef001a52907b22d42923596632f19ec Mon Sep 17 00:00:00 2001 From: willkroboth <46540330+willkroboth@users.noreply.github.com> Date: Tue, 30 Apr 2024 10:46:09 -0400 Subject: [PATCH] Fix & update `Previewable` arguments - Removed `CommandAPIHandler#previewableArguments` and related methods - Added `PreviewableCommandNode` for storing `Previewable` information directly in Brigadier's tree - Tweak `NMS_1_19_Common_ChatPreviewHandler` to build previews from the node tree rather than by the node path TODO: Should probably test these changes on a real server to verify. Also, another example of https://github.com/Mojang/brigadier/pull/144 being annoying. --- .../jorel/commandapi/CommandAPIHandler.java | 75 ------------ .../arguments/AbstractArgument.java | 33 +++--- .../PreviewableArgumentBuilder.java | 80 +++++++++++++ .../commandnodes/PreviewableCommandNode.java | 108 ++++++++++++++++++ .../UnnamedArgumentCommandNode.java | 2 +- .../NMS_1_19_Common_ChatPreviewHandler.java | 39 ++++--- 6 files changed, 229 insertions(+), 108 deletions(-) create mode 100644 commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableArgumentBuilder.java create mode 100644 commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableCommandNode.java diff --git a/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java b/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java index 8bd4179e5f..ab939cabd1 100644 --- a/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java +++ b/commandapi-core/src/main/java/dev/jorel/commandapi/CommandAPIHandler.java @@ -49,7 +49,6 @@ import dev.jorel.commandapi.executors.CommandArguments; import dev.jorel.commandapi.executors.ExecutionInfo; import dev.jorel.commandapi.preprocessor.RequireField; -import dev.jorel.commandapi.wrappers.PreviewableFunction; import java.util.concurrent.CompletableFuture; @@ -93,7 +92,6 @@ public class CommandAPIHandler platform; final Map registeredCommands; // Keep track of what has been registered for type checking - final Map, Previewable> previewableArguments; // Arguments with previewable chat static final Pattern NAMESPACE_PATTERN = Pattern.compile("[0-9a-z_.-]+"); private static CommandAPIHandler instance; @@ -105,7 +103,6 @@ public class CommandAPIHandler platform) { this.platform = platform; this.registeredCommands = new LinkedHashMap<>(); // This should be a LinkedHashMap to preserve insertion order - this.previewableArguments = new HashMap<>(); CommandAPIHandler.instance = this; } @@ -629,78 +626,6 @@ public Object parseArgument(CommandContext cmdCtx, String key, Argument } } - //////////////////////////////////// - // SECTION: Previewable Arguments // - //////////////////////////////////// - - /** - * Handles a previewable argument. This stores the path to the previewable argument - * in {@link CommandAPIHandler#previewableArguments} for runtime resolving - * - * @param previousArguments The list of arguments that came before this argument - * @param previewableArgument The {@link Previewable} argument - */ - public void addPreviewableArgument(List previousArguments, Argument previewableArgument) { - if (!(previewableArgument instanceof Previewable previewable)) { - throw new IllegalArgumentException("An argument must implement Previewable to be added as previewable argument"); - } - - // Generate all paths to the argument - List> paths = new ArrayList<>(); - paths.add(new ArrayList<>()); - - // TODO: Fix this, the `appendToCommandPaths` method was removed - // A smarter way to get this information should exist - // It probably makes sense to make a custom CommandNode for PreviewableArgument - if(true) throw new IllegalStateException("TODO: Fix this method"); - - // for (Argument argument : previousArguments) { - // argument.appendToCommandPaths(paths); - // } - // previewableArgument.appendToCommandPaths(paths); - - // Insert paths to our map - for (List path : paths) { - previewableArguments.put(path, previewable); - } - } - - /** - * Looks up the function to generate a chat preview for a path of nodes in the - * command tree. This is a method internal to the CommandAPI and isn't expected - * to be used by plugin developers (but you're more than welcome to use it as - * you see fit). - * - * @param path a list of Strings representing the path (names of command nodes) - * to (and including) the previewable argument - * @return a {@link PreviewableFunction} that takes in a {@link PreviewInfo} and returns a - * text Component. If such a function is not available, this will - * return a function that always returns null. - */ - @SuppressWarnings("unchecked") - public Optional> lookupPreviewable(List path) { - final Previewable previewable = previewableArguments.get(path); - if (previewable != null) { - return (Optional>) (Optional) previewable.getPreview(); - } else { - return Optional.empty(); - } - } - - /** - * @param path a list of Strings representing the path (names of command nodes) - * to (and including) the previewable argument - * @return Whether a previewable is legacy (non-Adventure) or not - */ - public boolean lookupPreviewableLegacyStatus(List path) { - final Previewable previewable = previewableArguments.get(path); - if (previewable != null && previewable.getPreview().isPresent()) { - return previewable.isLegacy(); - } else { - return true; - } - } - ///////////////////////// // SECTION: Reflection // ///////////////////////// diff --git a/commandapi-core/src/main/java/dev/jorel/commandapi/arguments/AbstractArgument.java b/commandapi-core/src/main/java/dev/jorel/commandapi/arguments/AbstractArgument.java index c8b4e05d86..705d317c33 100644 --- a/commandapi-core/src/main/java/dev/jorel/commandapi/arguments/AbstractArgument.java +++ b/commandapi-core/src/main/java/dev/jorel/commandapi/arguments/AbstractArgument.java @@ -26,11 +26,13 @@ import com.mojang.brigadier.builder.RequiredArgumentBuilder; import com.mojang.brigadier.context.CommandContext; import com.mojang.brigadier.exceptions.CommandSyntaxException; +import com.mojang.brigadier.suggestion.SuggestionProvider; import com.mojang.brigadier.tree.CommandNode; import dev.jorel.commandapi.AbstractArgumentTree; import dev.jorel.commandapi.CommandAPIHandler; import dev.jorel.commandapi.CommandPermission; import dev.jorel.commandapi.RegisteredCommand; +import dev.jorel.commandapi.commandnodes.PreviewableArgumentBuilder; import dev.jorel.commandapi.commandnodes.UnnamedRequiredArgumentBuilder; import dev.jorel.commandapi.exceptions.DuplicateNodeNameException; import dev.jorel.commandapi.exceptions.GreedyArgumentException; @@ -371,16 +373,9 @@ public NodeInformation addArgumentNodes( List previousArguments, List previousArgumentNames, Function, Command> terminalExecutorCreator ) { - CommandAPIHandler handler = CommandAPIHandler.getInstance(); - // Check preconditions checkPreconditions(previousNodeInformation, previousArguments, previousArgumentNames); - // Handle previewable argument - if (this instanceof Previewable) { - handler.addPreviewableArgument(previousArguments, (Argument) this); - } - // Create node ArgumentBuilder rootBuilder = createArgumentBuilder(previousArguments, previousArgumentNames); @@ -426,19 +421,29 @@ public void checkPreconditions( CommandAPIHandler handler = CommandAPIHandler.getInstance(); // Create node and add suggestions - // Note: I would like to combine these two `build.suggests(...)` calls, but they are actually two unrelated - // methods since UnnamedRequiredArgumentBuilder does not extend RequiredArgumentBuilder (see - // UnnamedRequiredArgumentBuilder for why). If UnnamedRequiredArgumentBuilder *does* extend - // RequiredArgumentBuilder, please simplify this if statement, like what Literal#createArgumentBuilder does. + // Note: I would like to combine these `builder.suggests(...)` calls, but they are actually unrelated + // methods since UnnamedRequiredArgumentBuilder and PreviewableArgumentBuilder do not extend RequiredArgumentBuilder + // (see those classes for why). If this has been fixed and they do extend RequiredArgumentBuilder, please simplify + // this if statement, like what Literal#createArgumentBuilder does. + SuggestionProvider suggestions = handler.generateBrigadierSuggestions(previousArguments, (Argument) this); ArgumentBuilder rootBuilder; - if(isListed) { + if (this instanceof Previewable previewable) { + // Handle previewable argument + PreviewableArgumentBuilder builder = PreviewableArgumentBuilder.previewableArgument( + nodeName, rawType, + previewable.getPreview().orElse(null), previewable.isLegacy(), isListed + ); + builder.suggests(suggestions); + + rootBuilder = builder; + } else if (isListed) { RequiredArgumentBuilder builder = RequiredArgumentBuilder.argument(nodeName, rawType); - builder.suggests(handler.generateBrigadierSuggestions(previousArguments, (Argument) this)); + builder.suggests(suggestions); rootBuilder = builder; } else { UnnamedRequiredArgumentBuilder builder = UnnamedRequiredArgumentBuilder.unnamedArgument(nodeName, rawType); - builder.suggests(handler.generateBrigadierSuggestions(previousArguments, (Argument) this)); + builder.suggests(suggestions); rootBuilder = builder; } diff --git a/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableArgumentBuilder.java b/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableArgumentBuilder.java new file mode 100644 index 0000000000..ad6d11f950 --- /dev/null +++ b/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableArgumentBuilder.java @@ -0,0 +1,80 @@ +package dev.jorel.commandapi.commandnodes; + +import com.mojang.brigadier.arguments.ArgumentType; +import com.mojang.brigadier.builder.ArgumentBuilder; +import com.mojang.brigadier.builder.RequiredArgumentBuilder; +import com.mojang.brigadier.suggestion.SuggestionProvider; +import com.mojang.brigadier.tree.CommandNode; + +import dev.jorel.commandapi.arguments.Previewable; +import dev.jorel.commandapi.wrappers.PreviewableFunction; + +/** + * A special type of {@link RequiredArgumentBuilder} for {@link Previewable} Arguments. Compared to the + * {@link RequiredArgumentBuilder}, this class builds a {@link PreviewableCommandNode} + * + * @param The Brigadier Source object for running commands. + * @param The type returned when this argument is parsed. + */ +// We can't actually extend RequiredArgumentBuilder since its only constructor is private :( +// See https://github.com/Mojang/brigadier/pull/144 +public class PreviewableArgumentBuilder extends ArgumentBuilder> { + // Everything here is copied from RequiredArgumentBuilder, which is why it would be nice to extend that directly + private final String name; + private final ArgumentType type; + private SuggestionProvider suggestionsProvider = null; + + // `Previewable` information + private final PreviewableFunction previewableFunction; + private final boolean legacy; + private final boolean isListed; + + private PreviewableArgumentBuilder(String name, ArgumentType type, PreviewableFunction previewableFunction, boolean legacy, boolean isListed) { + this.name = name; + this.type = type; + + this.previewableFunction = previewableFunction; + this.legacy = legacy; + this.isListed = isListed; + } + + public static PreviewableArgumentBuilder previewableArgument(String name, ArgumentType type, PreviewableFunction previewableFunction, boolean legacy, boolean isListed) { + return new PreviewableArgumentBuilder<>(name, type, previewableFunction, legacy, isListed); + } + + public PreviewableArgumentBuilder suggests(final SuggestionProvider provider) { + this.suggestionsProvider = provider; + return getThis(); + } + + public SuggestionProvider getSuggestionsProvider() { + return suggestionsProvider; + } + + @Override + protected PreviewableArgumentBuilder getThis() { + return this; + } + + public ArgumentType getType() { + return type; + } + + public String getName() { + return name; + } + + public PreviewableCommandNode build() { + final PreviewableCommandNode result = new PreviewableCommandNode( + previewableFunction, legacy, isListed, + getName(), getType(), + getCommand(), getRequirement(), getRedirect(), getRedirectModifier(), isFork(), getSuggestionsProvider() + ); + + for (final CommandNode argument : getArguments()) { + result.addChild(argument); + } + + return result; + } +} diff --git a/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableCommandNode.java b/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableCommandNode.java new file mode 100644 index 0000000000..4093c54a4f --- /dev/null +++ b/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/PreviewableCommandNode.java @@ -0,0 +1,108 @@ +package dev.jorel.commandapi.commandnodes; + +import java.util.Objects; +import java.util.Optional; +import java.util.function.Predicate; + +import com.mojang.brigadier.Command; +import com.mojang.brigadier.RedirectModifier; +import com.mojang.brigadier.StringReader; +import com.mojang.brigadier.arguments.ArgumentType; +import com.mojang.brigadier.context.CommandContextBuilder; +import com.mojang.brigadier.context.ParsedArgument; +import com.mojang.brigadier.exceptions.CommandSyntaxException; +import com.mojang.brigadier.suggestion.SuggestionProvider; +import com.mojang.brigadier.tree.ArgumentCommandNode; +import com.mojang.brigadier.tree.CommandNode; + +import dev.jorel.commandapi.arguments.Previewable; +import dev.jorel.commandapi.wrappers.PreviewableFunction; + +/** + * A special type of {@link ArgumentCommandNode} for {@link Previewable} arguments. Compared to the + * {@link ArgumentCommandNode}, this class also has the methods {@link #getPreview()} and {@link #isLegacy()}, + * which are used when players try to use the chat preview feature. + * + * @param The Brigadier Source object for running commands. + * @param The type returned when this argument is parsed. + */ +public class PreviewableCommandNode extends ArgumentCommandNode { + private final PreviewableFunction preview; + private final boolean legacy; + + // Instead of having a listed and unlisted copy of this class, we can just handle this with this boolean + private final boolean isListed; + + public PreviewableCommandNode( + PreviewableFunction preview, boolean legacy, boolean isListed, + String name, ArgumentType type, + Command command, Predicate requirement, CommandNode redirect, RedirectModifier modifier, boolean forks, SuggestionProvider customSuggestions + ) { + super(name, type, command, requirement, redirect, modifier, forks, customSuggestions); + this.preview = preview; + this.legacy = legacy; + this.isListed = isListed; + } + + // Methods needed to generate a preview + public Optional> getPreview() { + return Optional.ofNullable(preview); + } + + public boolean isLegacy() { + return legacy; + } + + // If we are unlisted, then when parsed, don't add the argument result to the CommandContext + public boolean isListed() { + return isListed; + } + + @Override + public void parse(StringReader reader, CommandContextBuilder contextBuilder) throws CommandSyntaxException { + // Copied from `super#parse`, but with listability added + int start = reader.getCursor(); + + T result = this.getType().parse(reader); + ParsedArgument parsed = new ParsedArgument<>(start, reader.getCursor(), result); + + if(isListed) contextBuilder.withArgument(this.getName(), parsed); + + contextBuilder.withNode(this, parsed.getRange()); + } + + // Typical ArgumentCommandNode methods, but make it our classes + // Mostly copied and inspired by the implementations for these methods in ArgumentCommandNode + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (!(obj instanceof PreviewableCommandNode other)) return false; + + if (!Objects.equals(this.preview, other.preview)) return false; + if (this.legacy != other.legacy) return false; + if (this.isListed != other.isListed) return false; + return super.equals(other); + } + + @Override + public int hashCode() { + int result = Objects.hash(this.preview, this.legacy, this.isListed); + result = 31*result + super.hashCode(); + return result; + } + + // TODO: Um, this currently doesn't work since PreviewableArgumentBuilder does not extend RequiredArgumentBuilder + // See PreviewableArgumentBuilder for why + // I hope no one tries to use this method! + // @Override + // public PreviewableArgumentBuilder createBuilder() { + // PreviewableArgumentBuilder builder = PreviewableArgumentBuilder.previewableArgument(getName(), getType(), preview, legacy, isListed); + + // builder.requires(getRequirement()); + // builder.forward(getRedirect(), getRedirectModifier(), isFork()); + // if (getCommand() != null) { + // builder.executes(getCommand()); + // } + // return builder; + // } +} diff --git a/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/UnnamedArgumentCommandNode.java b/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/UnnamedArgumentCommandNode.java index ae2cf6105b..c8a5ecac82 100644 --- a/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/UnnamedArgumentCommandNode.java +++ b/commandapi-core/src/main/java/dev/jorel/commandapi/commandnodes/UnnamedArgumentCommandNode.java @@ -26,7 +26,7 @@ public UnnamedArgumentCommandNode(String name, ArgumentType type, Command contextBuilder) throws CommandSyntaxException { final int start = reader.getCursor(); diff --git a/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common_ChatPreviewHandler.java b/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common_ChatPreviewHandler.java index ec7402c1af..ebbd69b31f 100644 --- a/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common_ChatPreviewHandler.java +++ b/commandapi-platforms/commandapi-bukkit/commandapi-bukkit-nms/commandapi-bukkit-1.19-common/src/main/java/dev/jorel/commandapi/nms/NMS_1_19_Common_ChatPreviewHandler.java @@ -3,10 +3,9 @@ import com.mojang.brigadier.ParseResults; import com.mojang.brigadier.context.ParsedCommandNode; import com.mojang.brigadier.exceptions.CommandSyntaxException; - -import dev.jorel.commandapi.CommandAPIHandler; import dev.jorel.commandapi.CommandAPIBukkit; import dev.jorel.commandapi.arguments.PreviewInfo; +import dev.jorel.commandapi.commandnodes.PreviewableCommandNode; import dev.jorel.commandapi.commandsenders.BukkitPlayer; import dev.jorel.commandapi.exceptions.WrapperCommandSyntaxException; import dev.jorel.commandapi.wrappers.PreviewableFunction; @@ -26,7 +25,6 @@ import org.bukkit.entity.Player; import org.bukkit.plugin.Plugin; -import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -51,7 +49,7 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception // Is command if (!chatPreview.query().isEmpty() && chatPreview.query().charAt(0) == '/') { // Is previewable argument - if(InitialParse.processChatPreviewQuery(chatPreview.query(), platform, player).preview.isPresent()){ + if(InitialParse.processChatPreviewQuery(chatPreview.query(), platform, player).previewableNode.getPreview().isPresent()){ handleChatPreviewPacket(chatPreview); return; } @@ -65,28 +63,32 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception protected abstract void handleChatPreviewPacket(ServerboundChatPreviewPacket chatPreview); public MutableComponent parseChatPreviewQuery(String chatPreviewQuery) { + // Parse the string final InitialParse ip = InitialParse.processChatPreviewQuery(chatPreviewQuery, platform, player); - final Optional> preview = ip.preview; + + // Return early if the node is not previewable + final PreviewableCommandNode previewableNode = ip.previewableNode; + final Optional> preview = previewableNode.getPreview(); if (preview.isEmpty()) { return null; } final String fullInput = ip.fullInput; final ParseResults results = ip.results; - final List path = ip.path; + final ParsedCommandNode parsedNode = ip.parsedNode; // Calculate the (argument) input and generate the component to send - String input = results.getContext().getNodes().get(results.getContext().getNodes().size() - 1).getRange().get(fullInput); + String input = parsedNode.getRange().get(fullInput); final String jsonToSend; Object component; try { @SuppressWarnings("rawtypes") final PreviewInfo previewInfo; - if (CommandAPIHandler.getInstance().lookupPreviewableLegacyStatus(path)) { + if (previewableNode.isLegacy()) { BaseComponent[] parsedInput; try { - parsedInput = platform.getChat(results.getContext().build(fullInput), path.get(path.size() - 1)); + parsedInput = platform.getChat(results.getContext().build(fullInput), previewableNode.getName()); } catch (CommandSyntaxException e) { throw new WrapperCommandSyntaxException(e); } @@ -94,7 +96,7 @@ public MutableComponent parseChatPreviewQuery(String chatPreviewQuery) { } else { Component parsedInput; try { - parsedInput = platform.getAdventureChat(results.getContext().build(fullInput), path.get(path.size() - 1)); + parsedInput = platform.getAdventureChat(results.getContext().build(fullInput), previewableNode.getName()); } catch (CommandSyntaxException e) { throw new WrapperCommandSyntaxException(e); } @@ -125,7 +127,7 @@ public MutableComponent parseChatPreviewQuery(String chatPreviewQuery) { return Serializer.fromJson(jsonToSend); } - private record InitialParse(String fullInput, ParseResults results, List path, Optional> preview){ + private record InitialParse(String fullInput, ParseResults results, ParsedCommandNode parsedNode, PreviewableCommandNode previewableNode){ private static InitialParse cachedResult = null; public static InitialParse processChatPreviewQuery(String chatPreviewQuery, CommandAPIBukkit platform, Player player){ // Substring 1 to get rid of the leading / @@ -136,14 +138,15 @@ public static InitialParse processChatPreviewQuery(String chatPreviewQuery, Comm ParseResults results = platform.getBrigadierDispatcher() .parse(fullInput, platform.getBrigadierSourceFromCommandSender(new BukkitPlayer(player))); - // Generate the path for lookup - List path = new ArrayList<>(); - for (ParsedCommandNode commandNode : results.getContext().getNodes()) { - path.add(commandNode.getNode().getName()); - } - Optional> preview = CommandAPIHandler.getInstance().lookupPreviewable(path); + // Get the last node + List> nodes = results.getContext().getNodes(); + ParsedCommandNode parsedNode = nodes.get(nodes.size()-1); + + // Get the parsed node, if it exists + PreviewableCommandNode previewableNode = parsedNode.getNode() instanceof PreviewableCommandNode pn ? pn : null; - cachedResult = new InitialParse(fullInput, results, path, preview); + // Cache the result and return + cachedResult = new InitialParse(fullInput, results, parsedNode, previewableNode); return cachedResult; } }