From 30a3a4fb89c01cd3237a9f16fcb8ce226ac3b85a Mon Sep 17 00:00:00 2001 From: Quantum64 Date: Fri, 18 Mar 2022 06:01:46 -0400 Subject: [PATCH] Prevent 1.16 -> 1.17 cursor desync with dragging (mostly fixes #389) (#445) --- .../Protocol1_16_4To1_17.java | 2 + .../packets/BlockItemPackets1_17.java | 82 +++++++++++++++++-- .../storage/PlayerLastCursorItem.java | 52 ++++++++++++ .../Protocol1_17To1_17_1.java | 13 ++- 4 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/storage/PlayerLastCursorItem.java diff --git a/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/Protocol1_16_4To1_17.java b/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/Protocol1_16_4To1_17.java index 64e3fe9c..1e9122e6 100644 --- a/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/Protocol1_16_4To1_17.java +++ b/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/Protocol1_16_4To1_17.java @@ -25,6 +25,7 @@ import com.viaversion.viabackwards.api.rewriters.TranslatableRewriter; import com.viaversion.viabackwards.protocol.protocol1_16_4to1_17.packets.BlockItemPackets1_17; import com.viaversion.viabackwards.protocol.protocol1_16_4to1_17.packets.EntityPackets1_17; import com.viaversion.viabackwards.protocol.protocol1_16_4to1_17.storage.PingRequests; +import com.viaversion.viabackwards.protocol.protocol1_16_4to1_17.storage.PlayerLastCursorItem; import com.viaversion.viaversion.api.connection.UserConnection; import com.viaversion.viaversion.api.minecraft.RegistryType; import com.viaversion.viaversion.api.minecraft.TagData; @@ -249,6 +250,7 @@ public final class Protocol1_16_4To1_17 extends BackwardsProtocol { + short slot = wrapper.passthrough(Type.SHORT); // Slot + byte button = wrapper.passthrough(Type.BYTE); // Button + wrapper.read(Type.SHORT); // Action id - removed + int mode = wrapper.passthrough(Type.VAR_INT); // Mode + Item clicked = handleItemToServer(wrapper.read(Type.FLAT_VAR_INT_ITEM)); // Clicked item + // The 1.17 client would check the entire inventory for changes before -> after a click and send the changed slots here wrapper.write(Type.VAR_INT, 0); // Empty array of slot+item - // Expected is the carried item after clicking, old clients send the clicked one (*mostly* being the same) - handleItemToServer(wrapper.passthrough(Type.FLAT_VAR_INT_ITEM)); + PlayerLastCursorItem state = wrapper.user().get(PlayerLastCursorItem.class); + if (mode == 0 && button == 0 && clicked != null) { + // Left click PICKUP + // Carried item will (usually) be the entire clicked stack + state.setLastCursorItem(clicked); + } else if (mode == 0 && button == 1 && clicked != null) { + // Right click PICKUP + // Carried item will (usually) be half of the clicked stack (rounding up) + // if the clicked slot is empty, otherwise it will (usually) be the whole clicked stack + if (state.isSet()) { + state.setLastCursorItem(clicked); + } else { + state.setLastCursorItem(clicked, (clicked.amount() + 1) / 2); + } + } else if (mode == 5 && slot == -999 && (button == 0 || button == 4)) { + // Start drag (left or right click) + // Preserve guessed carried item and forward to server + // This mostly fixes the click and drag ghost item issue + + // no-op + } else { + // Carried item unknown (TODO) + state.setLastCursorItem(null); + } + + Item carried = state.getLastCursorItem(); + if (carried == null) { + // Expected is the carried item after clicking, old clients send the clicked one (*mostly* being the same) + wrapper.write(Type.FLAT_VAR_INT_ITEM, clicked); + } else { + wrapper.write(Type.FLAT_VAR_INT_ITEM, carried); + } }); } }); + + protocol.registerClientbound(ClientboundPackets1_17.SET_SLOT, new PacketRemapper() { + @Override + public void registerMap() { + handler(wrapper -> { + short windowId = wrapper.passthrough(Type.UNSIGNED_BYTE); + short slot = wrapper.passthrough(Type.SHORT); + + Item carried = wrapper.read(Type.FLAT_VAR_INT_ITEM); + if (carried != null && windowId == -1 && slot == -1) { + // This is related to the hack to fix click and drag ghost items above. + // After a completed drag, we have no idea how many items remain on the cursor, + // and vanilla logic replication would be required to calculate the value. + // When the click drag complete packet is sent, we will send an incorrect + // carried item, and the server will helpfully send this packet allowing us + // to update the internal state. This is necessary for fixing multiple sequential + // click drag actions without intermittent pickup actions. + wrapper.user().get(PlayerLastCursorItem.class).setLastCursorItem(carried); + } + + wrapper.write(Type.FLAT_VAR_INT_ITEM, handleItemToClient(carried)); + }); + } + }); + protocol.registerServerbound(ServerboundPackets1_16_2.WINDOW_CONFIRMATION, null, new PacketRemapper() { @Override public void registerMap() { diff --git a/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/storage/PlayerLastCursorItem.java b/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/storage/PlayerLastCursorItem.java new file mode 100644 index 00000000..feff1a2c --- /dev/null +++ b/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_16_4to1_17/storage/PlayerLastCursorItem.java @@ -0,0 +1,52 @@ +/* + * This file is part of ViaBackwards - https://github.com/ViaVersion/ViaBackwards + * Copyright (C) 2016-2022 ViaVersion and contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package com.viaversion.viabackwards.protocol.protocol1_16_4to1_17.storage; + +import com.viaversion.viaversion.api.connection.StorableObject; +import com.viaversion.viaversion.api.minecraft.item.DataItem; +import com.viaversion.viaversion.api.minecraft.item.Item; + +public class PlayerLastCursorItem implements StorableObject { + private Item lastCursorItem; + + public Item getLastCursorItem() { + return copyItem(lastCursorItem); + } + + public void setLastCursorItem(Item item) { + this.lastCursorItem = copyItem(item); + } + + public void setLastCursorItem(Item item, int amount) { + this.lastCursorItem = copyItem(item); + this.lastCursorItem.setAmount(amount); + } + + public boolean isSet() { + return lastCursorItem != null; + } + + private static Item copyItem(Item item) { + if (item == null) { + return null; + } + Item copy = new DataItem(item); + copy.setTag(copy.tag() == null ? null : copy.tag().clone()); + return copy; + } +} diff --git a/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_17to1_17_1/Protocol1_17To1_17_1.java b/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_17to1_17_1/Protocol1_17To1_17_1.java index a5f2df60..bdced6c6 100644 --- a/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_17to1_17_1/Protocol1_17To1_17_1.java +++ b/common/src/main/java/com/viaversion/viabackwards/protocol/protocol1_17to1_17_1/Protocol1_17To1_17_1.java @@ -18,6 +18,7 @@ package com.viaversion.viabackwards.protocol.protocol1_17to1_17_1; import com.viaversion.viabackwards.api.BackwardsProtocol; +import com.viaversion.viabackwards.protocol.protocol1_16_4to1_17.storage.PlayerLastCursorItem; import com.viaversion.viabackwards.protocol.protocol1_17to1_17_1.storage.InventoryStateIds; import com.viaversion.viaversion.api.connection.UserConnection; import com.viaversion.viaversion.api.minecraft.item.Item; @@ -91,7 +92,17 @@ public final class Protocol1_17To1_17_1 extends BackwardsProtocol