Clean up StructuredItemRewriter, also handle sound events

api/src/main/java/com/viaversion/viaversion/api/data/MappingData.java

@@ -83,6 +83,10 @@ public interface MappingData {
 
     int getNewAttributeId(int id);
 
+    int getNewSoundId(int id);
+
+    int getOldSoundId(int i);
+
     /**
      * Returns a list of tags to send if present.
      *

api/src/main/java/com/viaversion/viaversion/api/data/MappingDataBase.java

@@ -203,6 +203,16 @@ public class MappingDataBase implements MappingData {
         return checkValidity(id, attributeMappings.getNewId(id), "attributes");
     }
 
+    @Override
+    public int getNewSoundId(final int id) {
+        return checkValidity(id, soundMappings.getNewId(id), "sound");
+    }
+
+    @Override
+    public int getOldSoundId(final int i) {
+        return soundMappings.getNewIdOrDefault(i, 0);
+    }
+
     @Override
     public @Nullable List<TagData> getTags(final RegistryType type) {
         return tags != null ? tags.get(type) : null;

api/src/main/java/com/viaversion/viaversion/api/minecraft/Holder.java

@@ -22,6 +22,8 @@
  */
 package com.viaversion.viaversion.api.minecraft;
 
+import it.unimi.dsi.fastutil.ints.Int2IntFunction;
+
 public interface Holder<T> {
 
     /**
@@ -79,4 +81,12 @@ public interface Holder<T> {
      * @see #hasId()
      */
     int id();
+
+    /**
+     * Returns a new holder with the id rewritten using the given function, or self if this is a direct holder or the id did not change.
+     *
+     * @param rewriteFunction the function to rewrite the id
+     * @return a new holder with the id rewritten, or self
+     */
+    Holder<T> updateId(final Int2IntFunction rewriteFunction);
 }

api/src/main/java/com/viaversion/viaversion/api/minecraft/HolderImpl.java

@@ -23,6 +23,7 @@
 package com.viaversion.viaversion.api.minecraft;
 
 import com.google.common.base.Preconditions;
+import it.unimi.dsi.fastutil.ints.Int2IntFunction;
 
 final class HolderImpl<T> implements Holder<T> {
 
@@ -61,6 +62,22 @@ final class HolderImpl<T> implements Holder<T> {
         return id;
     }
 
+    @Override
+    public Holder<T> updateId(final Int2IntFunction rewriteFunction) {
+        if (isDirect()) {
+            return this;
+        }
+
+        final int rewrittenId = rewriteFunction.applyAsInt(id);
+        if (rewrittenId == id) {
+            return this;
+        }
+        if (rewrittenId == -1) {
+            throw new IllegalArgumentException("Received invalid id in updateId");
+        }
+        return Holder.of(rewrittenId);
+    }
+
     @Override
     public String toString() {
         return "HolderImpl{" +

api/src/main/java/com/viaversion/viaversion/api/minecraft/HolderSetImpl.java

@@ -64,7 +64,7 @@ final class HolderSetImpl extends EitherImpl<String, int[]> implements HolderSet
         final int[] ids = ids();
         final int[] mappedIds = new int[ids.length];
         for (int i = 0; i < mappedIds.length; i++) {
-            mappedIds[i] = idRewriter.apply(ids[i]);
+            mappedIds[i] = idRewriter.applyAsInt(ids[i]);
         }
         return new HolderSetImpl(mappedIds);
     }

api/src/main/java/com/viaversion/viaversion/api/minecraft/item/data/AttributeModifiers1_21.java

@@ -26,6 +26,7 @@ import com.viaversion.viaversion.api.type.Type;
 import com.viaversion.viaversion.api.type.Types;
 import com.viaversion.viaversion.api.type.types.ArrayType;
 import io.netty.buffer.ByteBuf;
+import it.unimi.dsi.fastutil.ints.Int2IntFunction;
 
 public record AttributeModifiers1_21(AttributeModifier[] modifiers, boolean showInTooltip) {
 
@@ -44,6 +45,15 @@ public record AttributeModifiers1_21(AttributeModifier[] modifiers, boolean show
         }
     };
 
+    public AttributeModifiers1_21 rewrite(final Int2IntFunction rewriteFunction) {
+        final AttributeModifier[] modifiers = new AttributeModifier[this.modifiers.length];
+        for (int i = 0; i < this.modifiers.length; i++) {
+            final AttributeModifier modifier = this.modifiers[i];
+            modifiers[i] = new AttributeModifier(rewriteFunction.applyAsInt(modifier.attribute()), modifier.modifier(), modifier.slotType());
+        }
+        return new AttributeModifiers1_21(modifiers, showInTooltip);
+    }
+
     public record AttributeModifier(int attribute, ModifierData modifier, int slotType) {
 
         public static final Type<AttributeModifier> TYPE = new Type<>(AttributeModifier.class) {

api/src/main/java/com/viaversion/viaversion/api/minecraft/item/data/Instrument.java

@@ -27,6 +27,7 @@ import com.viaversion.viaversion.api.minecraft.SoundEvent;
 import com.viaversion.viaversion.api.type.Types;
 import com.viaversion.viaversion.api.type.types.misc.HolderType;
 import io.netty.buffer.ByteBuf;
+import it.unimi.dsi.fastutil.ints.Int2IntFunction;
 
 public record Instrument(Holder<SoundEvent> soundEvent, int useDuration, float range) {
 
@@ -47,4 +48,8 @@ public record Instrument(Holder<SoundEvent> soundEvent, int useDuration, float r
         }
     };
 
+    public Instrument rewrite(final Int2IntFunction soundIdRewriteFunction) {
+        final Holder<SoundEvent> soundEvent = this.soundEvent.updateId(soundIdRewriteFunction);
+        return soundEvent == this.soundEvent ? this : new Instrument(soundEvent, useDuration, range);
+    }
 }

api/src/main/java/com/viaversion/viaversion/api/minecraft/item/data/JukeboxPlayable.java

@@ -30,6 +30,7 @@ import com.viaversion.viaversion.api.type.Types;
 import com.viaversion.viaversion.api.type.types.misc.HolderType;
 import com.viaversion.viaversion.util.Either;
 import io.netty.buffer.ByteBuf;
+import it.unimi.dsi.fastutil.ints.Int2IntFunction;
 
 public record JukeboxPlayable(Either<Holder<JukeboxSong>, String> song, boolean showInTooltip) {
 
@@ -56,6 +57,20 @@ public record JukeboxPlayable(Either<Holder<JukeboxSong>, String> song, boolean
         }
     };
 
+    public JukeboxPlayable rewrite(final Int2IntFunction soundIdRewriteFunction) {
+        if (song.isRight()) {
+            return this;
+        }
+
+        final Holder<JukeboxSong> songHolder = this.song.left();
+        if (songHolder.hasId()) {
+            return this;
+        }
+
+        final JukeboxSong rewrittenSong = songHolder.value().rewrite(soundIdRewriteFunction);
+        return rewrittenSong == songHolder.value() ? this : new JukeboxPlayable(Holder.of(rewrittenSong), showInTooltip);
+    }
+
     public record JukeboxSong(Holder<SoundEvent> soundEvent, Tag description,
                               float lengthInSeconds, int comparatorOutput) {
 
@@ -77,5 +92,10 @@ public record JukeboxPlayable(Either<Holder<JukeboxSong>, String> song, boolean
                 Types.VAR_INT.writePrimitive(buffer, value.comparatorOutput);
             }
         };
+
+        public JukeboxSong rewrite(final Int2IntFunction soundIdRewriteFunction) {
+            final Holder<SoundEvent> soundEvent = this.soundEvent.updateId(soundIdRewriteFunction);
+            return soundEvent == this.soundEvent ? this : new JukeboxSong(soundEvent, description, lengthInSeconds, comparatorOutput);
+        }
     }
 }

api/src/main/java/com/viaversion/viaversion/api/minecraft/item/data/MapDecorations.java

@@ -1,56 +0,0 @@
-/*
- * This file is part of ViaVersion - https://github.com/ViaVersion/ViaVersion
- * Copyright (C) 2016-2024 ViaVersion and contributors
- *
- * Permission is hereby granted, free of charge, to any person obtaining a copy
- * of this software and associated documentation files (the "Software"), to deal
- * in the Software without restriction, including without limitation the rights
- * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
- * copies of the Software, and to permit persons to whom the Software is
- * furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in all
- * copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
- * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
- * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
- * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
- * SOFTWARE.
- */
-package com.viaversion.viaversion.api.minecraft.item.data;
-
-import com.viaversion.viaversion.api.type.Type;
-import com.viaversion.viaversion.api.type.Types;
-import io.netty.buffer.ByteBuf;
-import it.unimi.dsi.fastutil.objects.Object2ObjectMap;
-import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
-import java.util.Map;
-
-public record MapDecorations(Map<String, MapDecoration> decorations) {
-
-    public static final Type<MapDecorations> TYPE = new Type<>(MapDecorations.class) {
-        @Override
-        public MapDecorations read(final ByteBuf buffer) {
-            final Object2ObjectMap<String, MapDecoration> decorations = new Object2ObjectOpenHashMap<>();
-            final int size = Types.VAR_INT.readPrimitive(buffer);
-            for (int i = 0; i < size; i++) {
-                final String id = Types.STRING.read(buffer);
-                final MapDecoration decoration = MapDecoration.TYPE.read(buffer);
-                decorations.put(id, decoration);
-            }
-            return new MapDecorations(decorations);
-        }
-
-        @Override
-        public void write(final ByteBuf buffer, final MapDecorations value) {
-            Types.VAR_INT.writePrimitive(buffer, value.decorations.size());
-            for (final Map.Entry<String, MapDecoration> entry : value.decorations.entrySet()) {
-                Types.STRING.write(buffer, entry.getKey());
-                MapDecoration.TYPE.write(buffer, entry.getValue());
-            }
-        }
-    };
-}

common/src/main/java/com/viaversion/viaversion/rewriter/StructuredItemRewriter.java

@@ -22,6 +22,7 @@ import com.viaversion.nbt.tag.Tag;
 import com.viaversion.viaversion.api.connection.UserConnection;
 import com.viaversion.viaversion.api.data.FullMappings;
 import com.viaversion.viaversion.api.data.MappingData;
+import com.viaversion.viaversion.api.minecraft.Holder;
 import com.viaversion.viaversion.api.minecraft.Particle;
 import com.viaversion.viaversion.api.minecraft.data.StructuredData;
 import com.viaversion.viaversion.api.minecraft.data.StructuredDataContainer;
@@ -30,7 +31,6 @@ import com.viaversion.viaversion.api.minecraft.item.Item;
 import com.viaversion.viaversion.api.protocol.Protocol;
 import com.viaversion.viaversion.api.protocol.packet.ClientboundPacketType;
 import com.viaversion.viaversion.api.protocol.packet.ServerboundPacketType;
-import com.viaversion.viaversion.api.rewriter.ComponentRewriter;
 import com.viaversion.viaversion.api.type.Type;
 import it.unimi.dsi.fastutil.ints.Int2IntFunction;
 import java.util.Map;
@@ -63,40 +63,12 @@ public class StructuredItemRewriter<C extends ClientboundPacketType, S extends S
         }
 
         final MappingData mappingData = protocol.getMappingData();
-        final StructuredDataContainer dataContainer = item.dataContainer();
+        if (mappingData != null && mappingData.getItemMappings() != null) {
-        if (mappingData != null) {
-            if (mappingData.getItemMappings() != null) {
             item.setIdentifier(mappingData.getNewItemId(item.identifier()));
         }
 
-            final FullMappings dataComponentMappings = mappingData.getDataComponentSerializerMappings();
+        updateItemDataComponentTypeIds(item.dataContainer(), true);
-            if (dataComponentMappings != null) {
+        updateItemDataComponents(connection, item, true);
-                dataContainer.setIdLookup(protocol, true);
-                dataContainer.updateIds(protocol, dataComponentMappings::getNewId);
-            }
-        }
-
-        final ComponentRewriter componentRewriter = protocol.getComponentRewriter();
-        if (componentRewriter != null) {
-            // Handle name and lore components
-            updateComponent(connection, item, StructuredDataKey.ITEM_NAME, "item_name");
-            updateComponent(connection, item, StructuredDataKey.CUSTOM_NAME, "custom_name");
-
-            final StructuredData<Tag[]> loreData = dataContainer.getNonEmpty(StructuredDataKey.LORE);
-            if (loreData != null) {
-                for (final Tag tag : loreData.value()) {
-                    componentRewriter.processTag(connection, tag);
-                }
-            }
-        }
-
-        Int2IntFunction itemIdRewriter = null;
-        Int2IntFunction blockIdRewriter = null;
-        if (mappingData != null) {
-            itemIdRewriter = mappingData.getItemMappings() != null ? mappingData::getNewItemId : null;
-            blockIdRewriter = mappingData.getBlockMappings() != null ? mappingData::getNewBlockId : null;
-        }
-        updateItemComponents(connection, dataContainer, this::handleItemToClient, itemIdRewriter, blockIdRewriter);
         return item;
     }
 
@@ -107,44 +79,69 @@ public class StructuredItemRewriter<C extends ClientboundPacketType, S extends S
         }
 
         final MappingData mappingData = protocol.getMappingData();
-        final StructuredDataContainer dataContainer = item.dataContainer();
+        if (mappingData != null && mappingData.getItemMappings() != null) {
-        if (mappingData != null) {
-            if (mappingData.getItemMappings() != null) {
             item.setIdentifier(mappingData.getOldItemId(item.identifier()));
         }
 
-            final FullMappings dataComponentMappings = mappingData.getDataComponentSerializerMappings();
+        updateItemDataComponentTypeIds(item.dataContainer(), false);
-            if (dataComponentMappings != null) {
+        updateItemDataComponents(connection, item, false);
-                dataContainer.setIdLookup(protocol, false);
-                dataContainer.updateIds(protocol, id -> dataComponentMappings.inverse().getNewId(id));
-            }
-        }
-
         restoreTextComponents(item);
-
-        Int2IntFunction itemIdRewriter = null;
-        Int2IntFunction blockIdRewriter = null;
-        if (mappingData != null) {
-            itemIdRewriter = mappingData.getItemMappings() != null ? mappingData::getOldItemId : null;
-            blockIdRewriter = mappingData.getBlockMappings() != null ? mappingData::getOldBlockId : null;
-        }
-        updateItemComponents(connection, dataContainer, this::handleItemToServer, itemIdRewriter, blockIdRewriter);
         return item;
     }
 
-    protected void updateItemComponents(UserConnection connection, StructuredDataContainer container, ItemHandler itemHandler, @Nullable Int2IntFunction idRewriter, @Nullable Int2IntFunction blockIdRewriter) {
+    protected void updateItemDataComponentTypeIds(final StructuredDataContainer container, final boolean mappedNames) {
-        // Specific types that need deep handling
+        final MappingData mappingData = protocol.getMappingData();
-        if (idRewriter != null) {
+        if (mappingData == null) {
-            container.updateIfPresent(StructuredDataKey.TRIM, value -> value.rewrite(idRewriter));
+            return;
-            container.updateIfPresent(StructuredDataKey.POT_DECORATIONS, value -> value.rewrite(idRewriter));
         }
-        if (blockIdRewriter != null) {
+
+        FullMappings dataComponentMappings = mappingData.getDataComponentSerializerMappings();
+        if (dataComponentMappings == null) {
+            return;
+        }
+
+        if (!mappedNames) {
+            dataComponentMappings = dataComponentMappings.inverse();
+        }
+
+        container.setIdLookup(protocol, mappedNames); // Necessary to be set before trying to add values to the container
+        container.updateIds(protocol, dataComponentMappings::getNewId);
+    }
+
+    protected void updateItemDataComponents(final UserConnection connection, final Item item, final boolean clientbound) {
+        // Specific types that need deep handling
+        final StructuredDataContainer container = item.dataContainer();
+        final MappingData mappingData = protocol.getMappingData();
+        if (mappingData.getItemMappings() != null) {
+            final Int2IntFunction itemIdRewriter = clientbound ? mappingData::getNewItemId : mappingData::getOldItemId;
+            container.updateIfPresent(StructuredDataKey.TRIM, value -> value.rewrite(itemIdRewriter));
+            container.updateIfPresent(StructuredDataKey.POT_DECORATIONS, value -> value.rewrite(itemIdRewriter));
+        }
+        if (mappingData.getBlockMappings() != null) {
+            final Int2IntFunction blockIdRewriter = clientbound ? mappingData::getNewBlockId : mappingData::getOldBlockId;
             container.updateIfPresent(StructuredDataKey.TOOL, value -> value.rewrite(blockIdRewriter));
             container.updateIfPresent(StructuredDataKey.CAN_PLACE_ON, value -> value.rewrite(blockIdRewriter));
             container.updateIfPresent(StructuredDataKey.CAN_BREAK, value -> value.rewrite(blockIdRewriter));
         }
+        if (mappingData.getSoundMappings() != null) {
+            final Int2IntFunction soundIdRewriter = clientbound ? mappingData::getNewSoundId : mappingData::getOldSoundId;
+            container.updateIfPresent(StructuredDataKey.INSTRUMENT, value -> value.isDirect() ? Holder.of(value.value().rewrite(soundIdRewriter)) : value);
+            container.updateIfPresent(StructuredDataKey.JUKEBOX_PLAYABLE, value -> value.rewrite(soundIdRewriter));
+        }
+        if (clientbound && protocol.getComponentRewriter() != null) {
+            updateComponent(connection, item, StructuredDataKey.ITEM_NAME, "item_name");
+            updateComponent(connection, item, StructuredDataKey.CUSTOM_NAME, "custom_name");
+
+            final StructuredData<Tag[]> loreData = container.getNonEmpty(StructuredDataKey.LORE);
+            if (loreData != null) {
+                for (final Tag tag : loreData.value()) {
+                    protocol.getComponentRewriter().processTag(connection, tag);
+                }
+            }
+        }
 
         // Look for item types
+        final ItemHandler itemHandler = clientbound ? this::handleItemToClient : this::handleItemToServer;
         for (final Map.Entry<StructuredDataKey<?>, StructuredData<?>> entry : container.data().entrySet()) {
             final StructuredData<?> data = entry.getValue();
             if (data.isEmpty()) {
@@ -228,7 +225,7 @@ public class StructuredItemRewriter<C extends ClientboundPacketType, S extends S
     }
 
     @FunctionalInterface
-    public interface ItemHandler {
+    private interface ItemHandler {
 
         Item rewrite(UserConnection connection, Item item);
     }