Improve 1.21 client enchantments on legacy servers (#4255)

Signed-off-by: Pablo Herrera <pabloherrerapalacio@gmail.com>

bukkit/src/main/java/com/viaversion/viaversion/bukkit/listeners/v1_20_5to1_21/LegacyChangeItemListener.java

@@ -0,0 +1,72 @@
+/*
+ * This file is part of ViaVersion - https://github.com/ViaVersion/ViaVersion
+ * Copyright (C) 2016-2024 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 <http://www.gnu.org/licenses/>.
+ */
+package com.viaversion.viaversion.bukkit.listeners.v1_20_5to1_21;
+
+import com.viaversion.viaversion.ViaVersionPlugin;
+import com.viaversion.viaversion.api.connection.UserConnection;
+import com.viaversion.viaversion.protocols.v1_20_5to1_21.storage.EfficiencyAttributeStorage;
+import org.bukkit.entity.Player;
+import org.bukkit.event.EventHandler;
+import org.bukkit.event.EventPriority;
+import org.bukkit.event.block.BlockDamageEvent;
+import org.bukkit.event.inventory.InventoryCloseEvent;
+import org.bukkit.event.inventory.InventoryType;
+import org.bukkit.inventory.ItemStack;
+import org.bukkit.inventory.PlayerInventory;
+
+public class LegacyChangeItemListener extends PlayerChangeItemListener {
+
+    public LegacyChangeItemListener(final ViaVersionPlugin plugin) {
+        super(plugin);
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
+    public void onBlockDamageEvent(final BlockDamageEvent event) {
+        final Player player = event.getPlayer();
+        final ItemStack item = event.getItemInHand();
+        sendAttributeUpdate(player, item, Slot.HAND);
+    }
+
+    @EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
+    public void onInventoryClose(final InventoryCloseEvent event) {
+        if (event.getPlayer() instanceof Player player &&
+            (event.getInventory().getType() == InventoryType.CRAFTING ||
+                event.getInventory().getType() == InventoryType.PLAYER)) {
+            sendArmorUpdate(player);
+        }
+    }
+
+    private void sendArmorUpdate(final Player player) {
+        final UserConnection connection = getUserConnection(player);
+        final EfficiencyAttributeStorage storage = getEfficiencyStorage(connection);
+        if (storage == null) {
+            return;
+        }
+
+        final PlayerInventory inventory = player.getInventory();
+        final ItemStack helmet = inventory.getHelmet();
+        final ItemStack leggings = swiftSneak != null ? inventory.getLeggings() : null;
+        final ItemStack boots = depthStrider != null ? inventory.getBoots() : null;
+
+        storage.setEnchants(player.getEntityId(), connection, storage.activeEnchants()
+            .aquaAffinity(helmet != null ? helmet.getEnchantmentLevel(aquaAffinity) : 0)
+            .swiftSneak(leggings != null ? leggings.getEnchantmentLevel(swiftSneak) : 0)
+            .depthStrider(boots != null ? boots.getEnchantmentLevel(depthStrider) : 0));
+    }
+
+}

bukkit/src/main/java/com/viaversion/viaversion/bukkit/listeners/v1_20_5to1_21/PlayerChangeItemListener.java

@@ -18,7 +18,6 @@
 package com.viaversion.viaversion.bukkit.listeners.v1_20_5to1_21;
 
 import com.viaversion.viaversion.ViaVersionPlugin;
-import com.viaversion.viaversion.api.Via;
 import com.viaversion.viaversion.api.connection.UserConnection;
 import com.viaversion.viaversion.bukkit.listeners.ViaBukkitListener;
 import com.viaversion.viaversion.protocols.v1_20_5to1_21.Protocol1_20_5To1_21;
@@ -38,11 +37,11 @@ import org.checkerframework.checker.nullness.qual.Nullable;
 public class PlayerChangeItemListener extends ViaBukkitListener {
 
     // Use legacy function and names here to support all versions
-    private final Enchantment efficiency = getByName("efficiency", "DIG_SPEED");
+    protected final Enchantment efficiency = getByName("efficiency", "DIG_SPEED");
-    private final Enchantment aquaAffinity = getByName("aqua_affinity", "WATER_WORKER");
+    protected final Enchantment aquaAffinity = getByName("aqua_affinity", "WATER_WORKER");
-    private final Enchantment depthStrider = getByName("depth_strider", "DEPTH_STRIDER");
+    protected final Enchantment depthStrider = getByName("depth_strider", "DEPTH_STRIDER");
-    private final Enchantment soulSpeed = getByName("soul_speed", "SOUL_SPEED");
+    protected final Enchantment soulSpeed = getByName("soul_speed", "SOUL_SPEED");
-    private final Enchantment swiftSneak = getByName("swift_sneak", "SWIFT_SNEAK");
+    protected final Enchantment swiftSneak = getByName("swift_sneak", "SWIFT_SNEAK");
 
     public PlayerChangeItemListener(final ViaVersionPlugin plugin) {
         super(plugin, Protocol1_20_5To1_21.class);
@@ -55,35 +54,26 @@ public class PlayerChangeItemListener extends ViaBukkitListener {
         sendAttributeUpdate(player, item, Slot.HAND);
     }
 
+    protected EfficiencyAttributeStorage getEfficiencyStorage(final UserConnection connection) {
+        return connection != null ? connection.get(EfficiencyAttributeStorage.class) : null;
+    }
+
     void sendAttributeUpdate(final Player player, @Nullable final ItemStack item, final Slot slot) {
-        final UserConnection connection = Via.getAPI().getConnection(player.getUniqueId());
+        final UserConnection connection = getUserConnection(player);
-        if (connection == null || !isOnPipe(player)) {
+        final EfficiencyAttributeStorage storage = getEfficiencyStorage(connection);
-            return;
+        if (storage == null) return;
-        }
 
-        final EfficiencyAttributeStorage storage = connection.get(EfficiencyAttributeStorage.class);
+        EfficiencyAttributeStorage.ActiveEnchants enchants = storage.activeEnchants();
-        if (storage == null) {
+        enchants = switch (slot) {
-            return;
+            case HAND -> enchants.efficiency(item != null ? item.getEnchantmentLevel(efficiency) : 0);
-        }
+            case HELMET -> enchants.aquaAffinity(item != null ? item.getEnchantmentLevel(aquaAffinity) : 0);
-
+            case LEGGINGS -> enchants.swiftSneak(item != null && swiftSneak != null ? item.getEnchantmentLevel(swiftSneak) : 0);
-        final EfficiencyAttributeStorage.ActiveEnchants activeEnchants = storage.activeEnchants();
+            case BOOTS -> enchants.depthStrider(item != null && depthStrider != null ? item.getEnchantmentLevel(depthStrider) : 0);
-        int efficiencyLevel = activeEnchants.efficiency().level();
-        int aquaAffinityLevel = activeEnchants.aquaAffinity().level();
-        int soulSpeedLevel = activeEnchants.soulSpeed().level();
-        int swiftSneakLevel = activeEnchants.swiftSneak().level();
-        int depthStriderLevel = activeEnchants.depthStrider().level();
-        switch (slot) {
-            case HAND -> efficiencyLevel = item != null ? item.getEnchantmentLevel(efficiency) : 0;
-            case HELMET -> aquaAffinityLevel = item != null ? item.getEnchantmentLevel(aquaAffinity) : 0;
-            case LEGGINGS -> swiftSneakLevel = item != null && swiftSneak != null ? item.getEnchantmentLevel(swiftSneak) : 0;
-            case BOOTS -> {
-                depthStriderLevel = item != null && depthStrider != null ? item.getEnchantmentLevel(depthStrider) : 0;
             // TODO This needs continuous ticking for the supporting block as a conditional effect
             //  and is even more prone to desync from high ping than the other attributes
             //soulSpeedLevel = item != null && soulSpeed != null ? item.getEnchantmentLevel(soulSpeed) : 0;
-            }
+        };
-        }
+        storage.setEnchants(player.getEntityId(), connection, enchants);
-        storage.setEnchants(player.getEntityId(), connection, efficiencyLevel, soulSpeedLevel, swiftSneakLevel, aquaAffinityLevel, depthStriderLevel);
     }
 
     enum Slot {

bukkit/src/main/java/com/viaversion/viaversion/bukkit/platform/BukkitViaLoader.java

@@ -28,6 +28,7 @@ import com.viaversion.viaversion.bukkit.listeners.multiversion.PlayerSneakListen
 import com.viaversion.viaversion.bukkit.listeners.v1_14_4to1_15.EntityToggleGlideListener;
 import com.viaversion.viaversion.bukkit.listeners.v1_19_3to1_19_4.ArmorToggleListener;
 import com.viaversion.viaversion.bukkit.listeners.v1_18_2to1_19.BlockBreakListener;
+import com.viaversion.viaversion.bukkit.listeners.v1_20_5to1_21.LegacyChangeItemListener;
 import com.viaversion.viaversion.bukkit.listeners.v1_8to1_9.ArmorListener;
 import com.viaversion.viaversion.bukkit.listeners.v1_8to1_9.BlockListener;
 import com.viaversion.viaversion.bukkit.listeners.v1_8to1_9.DeathListener;
@@ -184,7 +185,7 @@ public class BukkitViaLoader implements ViaPlatformLoader {
             if (PaperViaInjector.hasClass("io.papermc.paper.event.player.PlayerInventorySlotChangeEvent")) {
                 new PaperPlayerChangeItemListener(plugin).register();
             } else {
-                new PlayerChangeItemListener(plugin).register();
+                new LegacyChangeItemListener(plugin).register();
             }
         }
     }

common/src/main/java/com/viaversion/viaversion/ViaListener.java

@@ -48,7 +48,10 @@ public abstract class ViaListener {
      * @return True if on pipe
      */
     protected boolean isOnPipe(UUID uuid) {
-        UserConnection userConnection = getUserConnection(uuid);
+        return isOnPipe(getUserConnection(uuid));
+    }
+
+    protected boolean isOnPipe(UserConnection userConnection) {
         return userConnection != null &&
             (requiredPipeline == null || userConnection.getProtocolInfo().getPipeline().contains(requiredPipeline));
     }

common/src/main/java/com/viaversion/viaversion/protocols/v1_20_5to1_21/rewriter/BlockItemPacketRewriter1_21.java

@@ -26,6 +26,7 @@ import com.viaversion.viaversion.api.minecraft.data.StructuredDataKey;
 import com.viaversion.viaversion.api.minecraft.item.Item;
 import com.viaversion.viaversion.api.minecraft.item.data.AttributeModifiers1_20_5;
 import com.viaversion.viaversion.api.minecraft.item.data.AttributeModifiers1_21;
+import com.viaversion.viaversion.api.minecraft.item.data.Enchantments;
 import com.viaversion.viaversion.api.protocol.packet.PacketWrapper;
 import com.viaversion.viaversion.api.type.Types;
 import com.viaversion.viaversion.api.type.types.chunk.ChunkType1_20_2;
@@ -38,6 +39,7 @@ import com.viaversion.viaversion.protocols.v1_20_3to1_20_5.packet.ServerboundPac
 import com.viaversion.viaversion.protocols.v1_20_3to1_20_5.packet.ServerboundPackets1_20_5;
 import com.viaversion.viaversion.protocols.v1_20_5to1_21.Protocol1_20_5To1_21;
 import com.viaversion.viaversion.protocols.v1_20_5to1_21.data.AttributeModifierMappings1_21;
+import com.viaversion.viaversion.protocols.v1_20_5to1_21.storage.EfficiencyAttributeStorage;
 import com.viaversion.viaversion.protocols.v1_20_5to1_21.storage.OnGroundTracker;
 import com.viaversion.viaversion.rewriter.BlockRewriter;
 import com.viaversion.viaversion.rewriter.StructuredItemRewriter;
@@ -48,6 +50,14 @@ import java.util.Objects;
 public final class BlockItemPacketRewriter1_21 extends StructuredItemRewriter<ClientboundPacket1_20_5, ServerboundPacket1_20_5, Protocol1_20_5To1_21> {
 
     private static final List<String> DISCS = List.of("11", "13", "5", "blocks", "cat", "chirp", "far", "mall", "mellohi", "otherside", "pigstep", "relic", "stal", "strad", "wait", "ward");
+    private static final int HELMET_SLOT = 5;
+    private static final int CHESTPLATE_SLOT = 6;
+    private static final int LEGGINGS_SLOT = 7;
+    private static final int BOOTS_SLOT = 8;
+
+    private static final int AQUA_AFFINITY_ID = 6;
+    private static final int DEPTH_STRIDER_ID = 8;
+    private static final int SWIFT_SNEAK_ID = 12;
 
     public BlockItemPacketRewriter1_21(final Protocol1_20_5To1_21 protocol) {
         super(protocol,
@@ -67,7 +77,29 @@ public final class BlockItemPacketRewriter1_21 extends StructuredItemRewriter<Cl
 
         registerCooldown(ClientboundPackets1_20_5.COOLDOWN);
         registerSetContent1_17_1(ClientboundPackets1_20_5.CONTAINER_SET_CONTENT);
-        registerSetSlot1_17_1(ClientboundPackets1_20_5.CONTAINER_SET_SLOT);
+        protocol.registerClientbound(ClientboundPackets1_20_5.CONTAINER_SET_SLOT, wrapper -> {
+            final short containerId = wrapper.passthrough(Types.UNSIGNED_BYTE); // Container id
+            wrapper.passthrough(Types.VAR_INT); // State id
+            final short slotId = wrapper.passthrough(Types.SHORT); // Slot id
+            final Item item = handleItemToClient(wrapper.user(), wrapper.read(itemType()));
+            wrapper.write(mappedItemType(), item);
+
+            // When a players' armor is set, update their attributes
+            if (containerId != 0 || slotId > BOOTS_SLOT || slotId < HELMET_SLOT || slotId == CHESTPLATE_SLOT) {
+                return;
+            }
+
+            final EfficiencyAttributeStorage storage = wrapper.user().get(EfficiencyAttributeStorage.class);
+            Enchantments enchants = item.dataContainer().get(StructuredDataKey.ENCHANTMENTS);
+            EfficiencyAttributeStorage.ActiveEnchants active = storage.activeEnchants();
+            active = switch (slotId) {
+                case HELMET_SLOT -> active.aquaAffinity(enchants == null ? 0 : enchants.getLevel(AQUA_AFFINITY_ID));
+                case LEGGINGS_SLOT -> active.swiftSneak(enchants == null ? 0 : enchants.getLevel(SWIFT_SNEAK_ID));
+                case BOOTS_SLOT -> active.depthStrider(enchants == null ? 0 : enchants.getLevel(DEPTH_STRIDER_ID));
+                default -> active;
+            };
+            storage.setEnchants(-1, wrapper.user(), active);
+        });
         registerAdvancements1_20_3(ClientboundPackets1_20_5.UPDATE_ADVANCEMENTS);
         registerSetEquipment(ClientboundPackets1_20_5.SET_EQUIPMENT);
         registerContainerClick1_17_1(ServerboundPackets1_20_5.CONTAINER_CLICK);

common/src/main/java/com/viaversion/viaversion/protocols/v1_20_5to1_21/rewriter/EntityPacketRewriter1_21.java

@@ -111,7 +111,8 @@ public final class EntityPacketRewriter1_21 extends EntityRewriter<ClientboundPa
                 map(Types.STRING); // World
                 handler(worldDataTrackerHandlerByKey1_20_5(3));
                 handler(playerTrackerHandler());
-                handler(wrapper -> wrapper.user().get(EfficiencyAttributeStorage.class).onLoginSent(wrapper.user()));
+                handler(wrapper -> wrapper.user().get(EfficiencyAttributeStorage.class)
+                    .onLoginSent(wrapper.get(Types.INT, 0), wrapper.user()));
             }
         });
 

common/src/main/java/com/viaversion/viaversion/protocols/v1_20_5to1_21/storage/EfficiencyAttributeStorage.java

@@ -45,25 +45,10 @@ public final class EfficiencyAttributeStorage implements StorableObject {
     private volatile boolean loginSent;
     private ActiveEnchants activeEnchants = DEFAULT;
 
-    public void setEnchants(final int entityId, final UserConnection connection, final int efficiency, final int soulSpeed,
+    public void setEnchants(final int entityId, final UserConnection connection, final ActiveEnchants enchants) {
-                            final int swiftSneak, final int aquaAffinity, final int depthStrider) {
+        if (activeEnchants == enchants) return;
-        // Always called from the main thread
-        if (efficiency == activeEnchants.efficiency.level
-            && soulSpeed == activeEnchants.soulSpeed.level
-            && swiftSneak == activeEnchants.swiftSneak.level
-            && aquaAffinity == activeEnchants.aquaAffinity.level
-            && depthStrider == activeEnchants.depthStrider.level) {
-            return;
-        }
-
         synchronized (lock) {
-            this.activeEnchants = new ActiveEnchants(entityId,
+            this.activeEnchants = entityId == -1 ? enchants : enchants.withEntityId(entityId);
-                new ActiveEnchant(activeEnchants.efficiency, efficiency),
-                new ActiveEnchant(activeEnchants.soulSpeed, soulSpeed),
-                new ActiveEnchant(activeEnchants.swiftSneak, swiftSneak),
-                new ActiveEnchant(activeEnchants.aquaAffinity, aquaAffinity),
-                new ActiveEnchant(activeEnchants.depthStrider, depthStrider)
-            );
             this.attributesSent = false;
         }
         sendAttributesPacket(connection, false);
@@ -73,7 +58,10 @@ public final class EfficiencyAttributeStorage implements StorableObject {
         return activeEnchants;
     }
 
-    public void onLoginSent(final UserConnection connection) {
+    public void onLoginSent(final int entityId, final UserConnection connection) {
+        synchronized (lock) {
+            activeEnchants = activeEnchants.withEntityId(entityId);
+        }
         // Always called from the netty thread
         this.loginSent = true;
         sendAttributesPacket(connection, false);
@@ -128,6 +116,59 @@ public final class EfficiencyAttributeStorage implements StorableObject {
 
     public record ActiveEnchants(int entityId, ActiveEnchant efficiency, ActiveEnchant soulSpeed,
                                  ActiveEnchant swiftSneak, ActiveEnchant aquaAffinity, ActiveEnchant depthStrider) {
+        private ActiveEnchants withEntityId(int entityId) {
+            return this.entityId == entityId ? this : new ActiveEnchants(entityId,
+                efficiency,
+                soulSpeed,
+                swiftSneak,
+                aquaAffinity,
+                depthStrider);
+        }
+
+        public ActiveEnchants efficiency(int level) {
+            return efficiency.level == level ? this : new ActiveEnchants(entityId,
+                new ActiveEnchant(efficiency, level),
+                soulSpeed,
+                swiftSneak,
+                aquaAffinity,
+                depthStrider);
+        }
+
+        public ActiveEnchants soulSpeed(int level) {
+            return soulSpeed.level == level ? this : new ActiveEnchants(entityId,
+                efficiency,
+                new ActiveEnchant(soulSpeed, level),
+                swiftSneak,
+                aquaAffinity,
+                depthStrider);
+        }
+
+        public ActiveEnchants swiftSneak(int level) {
+            return swiftSneak.level == level ? this : new ActiveEnchants(entityId,
+                efficiency,
+                soulSpeed,
+                new ActiveEnchant(swiftSneak, level),
+                aquaAffinity,
+                depthStrider);
+        }
+
+        public ActiveEnchants aquaAffinity(int level) {
+            return aquaAffinity.level == level ? this : new ActiveEnchants(entityId,
+                efficiency,
+                soulSpeed,
+                swiftSneak,
+                new ActiveEnchant(aquaAffinity, level),
+                depthStrider);
+        }
+
+        public ActiveEnchants depthStrider(int level) {
+            return depthStrider.level == level ? this : new ActiveEnchants(entityId,
+                efficiency,
+                soulSpeed,
+                swiftSneak,
+                aquaAffinity,
+                new ActiveEnchant(depthStrider, level));
+        }
     }
 
     public record ActiveEnchant(EnchantAttributeModifier modifier, int previousLevel, int level) {