From 3a450ee1759b3df38d698daabdfafd0447d8223a Mon Sep 17 00:00:00 2001 From: Trumeet Date: Wed, 27 Jul 2022 11:04:45 -0700 Subject: feat(mod): enforce cmd response ordering and termination --- mod/README.md | 11 +++--- .../java/moe/ymc/acron/cmd/CmdResConsumer.java | 18 ++++++++- mod/src/main/java/moe/ymc/acron/cmd/CmdSrc.java | 45 +++++++++++++++++++++- .../moe/ymc/acron/mixin/CommandManagerMixin.java | 15 ++++++++ 4 files changed, 81 insertions(+), 8 deletions(-) diff --git a/mod/README.md b/mod/README.md index d84a718..1e23817 100644 --- a/mod/README.md +++ b/mod/README.md @@ -212,7 +212,7 @@ Parameters: **Command result:** -When the command finishes without issues (?), Acron will send the following response: +When the command finishes, Acron will send the following response: ```json { @@ -225,11 +225,12 @@ When the command finishes without issues (?), Acron will send the following resp All parameters always present. -> **Note** +> **Important:** > -> The result completely depends on Minecraft server's response. -> It may not be reliable, and the values of `.result` and `.success` are -> undocumented. +> For `cmd` requests, the response will be either a single `error`, or a series of: +> single `ok` (Successfully scheduled) -> zero or more `cmd_out` (Output) -> single `cmd_result` (Final result) +> The response will always end with either error or cmd_result. +> After that, the server should not return anything with the same ID. ### Receiving Messages diff --git a/mod/src/main/java/moe/ymc/acron/cmd/CmdResConsumer.java b/mod/src/main/java/moe/ymc/acron/cmd/CmdResConsumer.java index d22b77e..36f7605 100644 --- a/mod/src/main/java/moe/ymc/acron/cmd/CmdResConsumer.java +++ b/mod/src/main/java/moe/ymc/acron/cmd/CmdResConsumer.java @@ -16,6 +16,8 @@ public class CmdResConsumer implements ResultConsumer { private final @NotNull Channel channel; private final int id; + private boolean hasResult; + public CmdResConsumer(@NotNull Channel channel, int id) { this.channel = channel; @@ -28,6 +30,20 @@ public class CmdResConsumer implements ResultConsumer { id, success, result); - channel.writeAndFlush(Serializer.serialize(new EventCmdRes(id, success, result))); + // Ignore any failed results because CommandDispatcher#execute does not reliably send error + // results to the consumer. + // For example, fail results are either not sent at all (pre-processing errors), or + // sent before writing the result, which both violates the spec. + // We will send the failed results in a custom mixin. + if (success) { + hasResult = true; + channel.writeAndFlush(Serializer.serialize(new EventCmdRes(id, success, result))); + } + } + + public void sendResultIfNot() { + if (!hasResult) { + channel.writeAndFlush(Serializer.serialize(new EventCmdRes(id, false, -1))); + } } } diff --git a/mod/src/main/java/moe/ymc/acron/cmd/CmdSrc.java b/mod/src/main/java/moe/ymc/acron/cmd/CmdSrc.java index 983b4ed..9fde8a4 100644 --- a/mod/src/main/java/moe/ymc/acron/cmd/CmdSrc.java +++ b/mod/src/main/java/moe/ymc/acron/cmd/CmdSrc.java @@ -1,24 +1,34 @@ package moe.ymc.acron.cmd; +import com.mojang.brigadier.ResultConsumer; import io.netty.channel.Channel; import moe.ymc.acron.net.ClientConfiguration; import net.minecraft.command.argument.EntityAnchorArgumentType; +import net.minecraft.entity.Entity; import net.minecraft.server.MinecraftServer; +import net.minecraft.server.command.CommandOutput; import net.minecraft.server.command.ServerCommandSource; +import net.minecraft.server.world.ServerWorld; import net.minecraft.text.LiteralText; +import net.minecraft.text.Text; +import net.minecraft.util.math.Vec2f; +import net.minecraft.util.math.Vec3d; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; -class CmdSrc extends ServerCommandSource { +public class CmdSrc extends ServerCommandSource { private static final Logger LOGGER = LogManager.getLogger(); + private final CmdResConsumer resConsumer; + public CmdSrc(@NotNull Channel channel, int id, boolean display, @NotNull ClientConfiguration configuration, @NotNull MinecraftServer server) { - super(new CmdOut(channel, id, display), + this(new CmdOut(channel, id, display), configuration.pos(), configuration.rot(), configuration.world(), @@ -31,4 +41,35 @@ class CmdSrc extends ServerCommandSource { new CmdResConsumer(channel, id), EntityAnchorArgumentType.EntityAnchor.FEET); } + + public CmdSrc(CommandOutput output, + Vec3d pos, + Vec2f rot, + ServerWorld world, + int level, + String name, + Text displayName, + MinecraftServer server, + @Nullable Entity entity, + boolean silent, + CmdResConsumer consumer, + EntityAnchorArgumentType.EntityAnchor entityAnchor) { + super(output, + pos, + rot, + world, + level, + name, + displayName, + server, + entity, + silent, + consumer, + entityAnchor); + this.resConsumer = consumer; + } + + public void sendResultIfNot() { + resConsumer.sendResultIfNot(); + } } diff --git a/mod/src/main/java/moe/ymc/acron/mixin/CommandManagerMixin.java b/mod/src/main/java/moe/ymc/acron/mixin/CommandManagerMixin.java index 9aaed2e..ccde6f0 100644 --- a/mod/src/main/java/moe/ymc/acron/mixin/CommandManagerMixin.java +++ b/mod/src/main/java/moe/ymc/acron/mixin/CommandManagerMixin.java @@ -1,6 +1,7 @@ package moe.ymc.acron.mixin; import com.mojang.brigadier.CommandDispatcher; +import moe.ymc.acron.cmd.CmdSrc; import moe.ymc.acron.config.ConfigReloadCmd; import net.minecraft.server.command.CommandManager; import net.minecraft.server.command.ServerCommandSource; @@ -12,6 +13,7 @@ import org.spongepowered.asm.mixin.Shadow; import org.spongepowered.asm.mixin.injection.At; import org.spongepowered.asm.mixin.injection.Inject; import org.spongepowered.asm.mixin.injection.callback.CallbackInfo; +import org.spongepowered.asm.mixin.injection.callback.CallbackInfoReturnable; import static net.minecraft.server.command.CommandManager.literal; @@ -35,4 +37,17 @@ public abstract class CommandManagerMixin { ) ); } + + @Inject(at = @At(value = "RETURN"), + method = "execute") + // Unfortunately we cannot inject into the catch blocks, so + // a cursed approach would be always sending the result if we + // haven't. + public void execute(ServerCommandSource commandSource, + String command, + CallbackInfoReturnable cir) { + if (commandSource instanceof CmdSrc src) { + src.sendResultIfNot(); + } + } } -- cgit v1.2.3