Skip to content

Commit 09eb3e9

Browse files
EarthcomputerEarthcomputer
committed
Fix network desync causing ItemThrowTask to usually overcompensate by throwing one extra item
Suggestions are now handled on-thread by MinecraftServer.execute, rather than via the PacketUtils queue, causing it to be handled earlier than other packets, making it inappropriate for a round trip fence. We now use a stats request for a round trip request.
1 parent 4dcbb70 commit 09eb3e9

10 files changed

Lines changed: 194 additions & 16 deletions

File tree

‎src/main/java/net/earthcomputer/clientcommands/command/ListenCommand.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import com.mojang.logging.LogUtils;
1010
import it.unimi.dsi.fastutil.objects.ReferenceOpenHashSet;
1111
import net.earthcomputer.clientcommands.Configs;
12+
import net.earthcomputer.clientcommands.event.ClientConnectionEvents;
1213
import net.earthcomputer.clientcommands.features.PacketDumper;
1314
import net.earthcomputer.clientcommands.util.ReflectionUtils;
1415
import net.earthcomputer.clientcommands.util.UnsafeUtils;
@@ -66,6 +67,10 @@ public class ListenCommand {
6667

6768
private static @UnknownNullability PacketCallback callback;
6869

70+
static {
71+
ClientConnectionEvents.SEND_PACKET_NO_UNPACK_BUNDLES.register((_, packet) -> onPacket(packet, PacketFlow.SERVERBOUND));
72+
}
73+
6974
public static void register(CommandDispatcher<FabricClientCommandSource> dispatcher) {
7075
dispatcher.register(literal("clisten")
7176
.then(literal("add")

‎src/main/java/net/earthcomputer/clientcommands/event/ClientConnectionEvents.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import net.fabricmc.fabric.api.event.Event;
44
import net.fabricmc.fabric.api.event.EventFactory;
5+
import net.minecraft.client.multiplayer.ClientPacketListener;
6+
import net.minecraft.network.protocol.Packet;
57

68
public final class ClientConnectionEvents {
79
public static final Event<Disconnect> DISCONNECT = EventFactory.createArrayBacked(Disconnect.class, listeners -> () -> {
@@ -10,8 +12,25 @@ public final class ClientConnectionEvents {
1012
}
1113
});
1214

15+
public static final Event<SendPacket> SEND_PACKET = EventFactory.createArrayBacked(SendPacket.class, listeners -> (connection, packet) -> {
16+
for (SendPacket listener : listeners) {
17+
listener.onSendPacket(connection, packet);
18+
}
19+
});
20+
21+
public static final Event<SendPacket> SEND_PACKET_NO_UNPACK_BUNDLES = EventFactory.createArrayBacked(SendPacket.class, listeners -> (connection, packet) -> {
22+
for (SendPacket listener : listeners) {
23+
listener.onSendPacket(connection, packet);
24+
}
25+
});
26+
1327
@FunctionalInterface
1428
public interface Disconnect {
1529
void onDisconnect();
1630
}
31+
32+
@FunctionalInterface
33+
public interface SendPacket {
34+
void onSendPacket(ClientPacketListener connection, Packet<?> packet);
35+
}
1736
}

‎src/main/java/net/earthcomputer/clientcommands/features/SuggestionsHook.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
55
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
66
import net.earthcomputer.clientcommands.event.ClientConnectionEvents;
7+
import net.earthcomputer.clientcommands.util.RoundTripFence;
78
import net.minecraft.client.Minecraft;
89
import net.minecraft.client.multiplayer.ClientPacketListener;
910
import net.minecraft.network.protocol.game.ClientboundCommandSuggestionsPacket;
@@ -23,10 +24,6 @@ private SuggestionsHook() {
2324
private static int currentSuggestionId = MAGIC_SUGGESTION_ID;
2425
private static final Int2ObjectMap<CompletableFuture<Suggestions>> pendingSuggestions = new Int2ObjectOpenHashMap<>();
2526

26-
public static CompletableFuture<Void> fence() {
27-
return request("").thenAccept(suggestions -> {});
28-
}
29-
3027
public static CompletableFuture<Suggestions> request(String command) {
3128
ClientPacketListener connection = Minecraft.getInstance().getConnection();
3229
if (connection == null) {
@@ -36,10 +33,24 @@ public static CompletableFuture<Suggestions> request(String command) {
3633
currentSuggestionId--;
3734
CompletableFuture<Suggestions> future = new CompletableFuture<>();
3835
pendingSuggestions.put(currentSuggestionId, future);
39-
connection.send(new ServerboundCommandSuggestionPacket(currentSuggestionId, command));
36+
doRequest(connection, currentSuggestionId, command);
4037
return future;
4138
}
4239

40+
private static void doRequest(ClientPacketListener connection, int suggestionId, String command) {
41+
if (!connection.isAcceptingMessages()) {
42+
return;
43+
}
44+
45+
connection.send(new ServerboundCommandSuggestionPacket(suggestionId, command));
46+
// Server isn't guaranteed to respond, keep retrying
47+
RoundTripFence.getInstance(connection).fence().thenRun(() -> {
48+
if (pendingSuggestions.containsKey(suggestionId)) {
49+
doRequest(connection, suggestionId, command);
50+
}
51+
});
52+
}
53+
4354
public static boolean onCompletions(ClientboundCommandSuggestionsPacket packet) {
4455
CompletableFuture<Suggestions> future = pendingSuggestions.remove(packet.id());
4556
if (future == null) {

‎src/main/java/net/earthcomputer/clientcommands/interfaces/IClientPacketListener.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package net.earthcomputer.clientcommands.interfaces;
2+
3+
import net.earthcomputer.clientcommands.util.RoundTripFence;
4+
5+
public interface IClientPacketListener {
6+
RoundTripFence clientcommands_getRoundTripFence();
7+
}

‎src/main/java/net/earthcomputer/clientcommands/mixin/commands/listen/ConnectionMixin.java‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
package net.earthcomputer.clientcommands.mixin.commands.listen;
22

3-
import io.netty.channel.ChannelFutureListener;
43
import io.netty.channel.ChannelHandlerContext;
54
import net.earthcomputer.clientcommands.command.ListenCommand;
65
import net.minecraft.network.Connection;
76
import net.minecraft.network.protocol.Packet;
87
import net.minecraft.network.protocol.PacketFlow;
9-
import org.jspecify.annotations.Nullable;
108
import org.spongepowered.asm.mixin.Final;
119
import org.spongepowered.asm.mixin.Mixin;
1210
import org.spongepowered.asm.mixin.Shadow;
@@ -24,11 +22,4 @@ private void onPacketReceive(ChannelHandlerContext ctx, Packet<?> packet, Callba
2422
ListenCommand.onPacket(packet, ListenCommand.PacketFlow.CLIENTBOUND);
2523
}
2624
}
27-
28-
@Inject(method = "doSendPacket", at = @At("HEAD"))
29-
private void onPacketSend(Packet<?> packet, @Nullable ChannelFutureListener listener, boolean flush, CallbackInfo ci) {
30-
if (this.receiving == PacketFlow.CLIENTBOUND) {
31-
ListenCommand.onPacket(packet, ListenCommand.PacketFlow.SERVERBOUND);
32-
}
33-
}
3425
}

‎src/main/java/net/earthcomputer/clientcommands/mixin/events/ConnectionMixin.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package net.earthcomputer.clientcommands.mixin.events;
2+
3+
import io.netty.channel.ChannelFutureListener;
4+
import net.earthcomputer.clientcommands.event.ClientConnectionEvents;
5+
import net.minecraft.client.multiplayer.ClientPacketListener;
6+
import net.minecraft.network.Connection;
7+
import net.minecraft.network.PacketListener;
8+
import net.minecraft.network.protocol.BundlePacket;
9+
import net.minecraft.network.protocol.Packet;
10+
import net.minecraft.network.protocol.PacketFlow;
11+
import org.jspecify.annotations.Nullable;
12+
import org.spongepowered.asm.mixin.Final;
13+
import org.spongepowered.asm.mixin.Mixin;
14+
import org.spongepowered.asm.mixin.Shadow;
15+
import org.spongepowered.asm.mixin.Unique;
16+
import org.spongepowered.asm.mixin.injection.At;
17+
import org.spongepowered.asm.mixin.injection.Inject;
18+
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
19+
20+
import java.util.function.Consumer;
21+
22+
@Mixin(Connection.class)
23+
public class ConnectionMixin {
24+
@Shadow
25+
private volatile @Nullable PacketListener packetListener;
26+
27+
@Shadow
28+
@Final
29+
private PacketFlow receiving;
30+
31+
@Inject(method = "sendPacket", at = @At("HEAD"))
32+
private void onSendPacket(Packet<?> packet, @Nullable ChannelFutureListener listener, boolean flush, CallbackInfo ci) {
33+
if (this.receiving == PacketFlow.CLIENTBOUND && this.packetListener instanceof ClientPacketListener connection) {
34+
ClientConnectionEvents.SEND_PACKET_NO_UNPACK_BUNDLES.invoker().onSendPacket(connection, packet);
35+
unpackBundle(packet, subPacket -> ClientConnectionEvents.SEND_PACKET.invoker().onSendPacket(connection, subPacket));
36+
}
37+
}
38+
39+
@Unique
40+
private static void unpackBundle(Packet<?> packet, Consumer<Packet<?>> action) {
41+
if (packet instanceof BundlePacket<?> bundle) {
42+
bundle.subPackets().forEach(subPacket -> unpackBundle(subPacket, action));
43+
} else {
44+
action.accept(packet);
45+
}
46+
}
47+
}

‎src/main/java/net/earthcomputer/clientcommands/mixin/roundtripfence/ClientPacketListenerMixin.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package net.earthcomputer.clientcommands.mixin.roundtripfence;
2+
3+
import net.earthcomputer.clientcommands.interfaces.IClientPacketListener;
4+
import net.earthcomputer.clientcommands.util.RoundTripFence;
5+
import net.minecraft.client.multiplayer.ClientPacketListener;
6+
import org.spongepowered.asm.mixin.Mixin;
7+
import org.spongepowered.asm.mixin.Unique;
8+
import org.spongepowered.asm.mixin.injection.At;
9+
import org.spongepowered.asm.mixin.injection.Inject;
10+
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
11+
12+
@Mixin(ClientPacketListener.class)
13+
public class ClientPacketListenerMixin implements IClientPacketListener {
14+
@Unique
15+
private final RoundTripFence roundTripFence = new RoundTripFence((ClientPacketListener) (Object) this);
16+
17+
@Override
18+
public RoundTripFence clientcommands_getRoundTripFence() {
19+
return roundTripFence;
20+
}
21+
22+
@Inject(method = "handleAwardStats", at = @At(value = "INVOKE", target = "Lnet/minecraft/network/protocol/PacketUtils;ensureRunningOnSameThread(Lnet/minecraft/network/protocol/Packet;Lnet/minecraft/network/PacketListener;Lnet/minecraft/network/PacketProcessor;)V", shift = At.Shift.AFTER))
23+
private void onAwardStats(CallbackInfo ci) {
24+
roundTripFence.onReceiveStats();
25+
}
26+
}

‎src/main/java/net/earthcomputer/clientcommands/task/ItemThrowTask.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,13 @@
44
import net.earthcomputer.clientcommands.Configs;
55
import net.earthcomputer.clientcommands.event.MoreClientEntityEvents;
66
import net.earthcomputer.clientcommands.features.PlayerRandCracker;
7-
import net.earthcomputer.clientcommands.features.SuggestionsHook;
7+
import net.earthcomputer.clientcommands.util.RoundTripFence;
88
import net.minecraft.client.Minecraft;
9+
import net.minecraft.client.multiplayer.ClientPacketListener;
910
import net.minecraft.client.player.LocalPlayer;
1011
import net.minecraft.network.protocol.game.ClientboundAddEntityPacket;
1112
import net.minecraft.world.entity.EntityTypes;
13+
import org.jspecify.annotations.Nullable;
1214
import org.slf4j.Logger;
1315

1416
import java.lang.ref.WeakReference;
@@ -23,6 +25,7 @@ public abstract class ItemThrowTask extends SimpleTask {
2325
public static final int FLAG_URGENT = 1;
2426
public static final int FLAG_WAIT_FOR_ITEMS = 2;
2527

28+
@Nullable
2629
private static WeakReference<ItemThrowTask> currentThrowTask = null;
2730

2831
static {
@@ -58,6 +61,11 @@ public boolean condition() {
5861

5962
@Override
6063
protected void onTick() {
64+
ClientPacketListener connection = Minecraft.getInstance().getConnection();
65+
if (connection == null) {
66+
return;
67+
}
68+
6169
itemThrowsAllowedThisTick += Configs.itemThrowsPerTick;
6270

6371
while (((flags & FLAG_URGENT) != 0 || itemThrowsAllowedThisTick >= 1) && sentItemThrows < totalItemsToThrow) {
@@ -82,7 +90,7 @@ protected void onTick() {
8290

8391
if (!waitingFence && sentItemThrows == totalItemsToThrow && confirmedItemThrows < sentItemThrows) {
8492
waitingFence = true;
85-
SuggestionsHook.fence().thenAccept(v -> {
93+
RoundTripFence.getInstance(connection).fence().thenRun(() -> {
8694
if (sentItemThrows > confirmedItemThrows) {
8795
LOGGER.info("Server rejected {} item throws. Rethrowing them.", sentItemThrows - confirmedItemThrows);
8896
while (sentItemThrows > confirmedItemThrows) {

‎src/main/java/net/earthcomputer/clientcommands/util/RoundTripFence.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package net.earthcomputer.clientcommands.util;
2+
3+
import com.mojang.logging.LogUtils;
4+
import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
5+
import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
6+
import net.earthcomputer.clientcommands.event.ClientConnectionEvents;
7+
import net.earthcomputer.clientcommands.interfaces.IClientPacketListener;
8+
import net.minecraft.client.multiplayer.ClientPacketListener;
9+
import net.minecraft.network.protocol.game.ServerboundClientCommandPacket;
10+
import org.slf4j.Logger;
11+
12+
import java.util.concurrent.CompletableFuture;
13+
14+
public final class RoundTripFence {
15+
private static final Logger LOGGER = LogUtils.getLogger();
16+
17+
private final ClientPacketListener connection;
18+
private long nextSendRequestId = 0;
19+
private long nextReceiveRequestId = 0;
20+
private final Long2ObjectMap<CompletableFuture<Void>> fences = new Long2ObjectOpenHashMap<>();
21+
22+
static {
23+
ClientConnectionEvents.SEND_PACKET.register((connection, packet) -> {
24+
if (packet instanceof ServerboundClientCommandPacket clientCommandPacket && clientCommandPacket.getAction() == ServerboundClientCommandPacket.Action.REQUEST_STATS) {
25+
getInstance(connection).onSendRequestStats();
26+
}
27+
});
28+
}
29+
30+
public RoundTripFence(ClientPacketListener connection) {
31+
this.connection = connection;
32+
}
33+
34+
public static RoundTripFence getInstance(ClientPacketListener connection) {
35+
return ((IClientPacketListener) connection).clientcommands_getRoundTripFence();
36+
}
37+
38+
public CompletableFuture<Void> fence() {
39+
CompletableFuture<Void> future = new CompletableFuture<>();
40+
fences.put(nextSendRequestId, future);
41+
connection.send(new ServerboundClientCommandPacket(ServerboundClientCommandPacket.Action.REQUEST_STATS));
42+
return future;
43+
}
44+
45+
public void onSendRequestStats() {
46+
nextSendRequestId++;
47+
}
48+
49+
public void onReceiveStats() {
50+
if (nextReceiveRequestId == nextSendRequestId) {
51+
LOGGER.warn("Received stats without request");
52+
return;
53+
}
54+
55+
CompletableFuture<Void> fence = fences.remove(nextReceiveRequestId);
56+
if (fence != null) {
57+
fence.complete(null);
58+
}
59+
60+
nextReceiveRequestId++;
61+
}
62+
}

‎src/main/resources/mixins.clientcommands.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
"debug.EntityMixin",
2323
"debug.LevelMixin",
2424
"debug.ServerLevelMixin",
25+
"events.ConnectionMixin",
2526
"events.EntityMixin",
2627
"events.ExperienceOrbMixin",
2728
"events.MinecraftMixin",
@@ -85,6 +86,7 @@
8586
"events.GuiMixin",
8687
"lengthextender.ChatScreenMixin",
8788
"rngevents.ClientLevelMixin",
89+
"roundtripfence.ClientPacketListenerMixin",
8890
"scrambletitle.MinecraftMixin",
8991
"screen.GuiMixin",
9092
"screen.WindowMixin",

0 commit comments

Comments
 (0)