A bunch of fixes and some cleanup

common/src/main/java/com/viaversion/viabackwards/api/rewriters/BackwardsStructuredItemRewriter.java

@@ -18,13 +18,17 @@
 package com.viaversion.viabackwards.api.rewriters;
 
 import com.viaversion.nbt.tag.CompoundTag;
+import com.viaversion.nbt.tag.IntArrayTag;
 import com.viaversion.nbt.tag.IntTag;
 import com.viaversion.nbt.tag.ListTag;
+import com.viaversion.nbt.tag.StringTag;
 import com.viaversion.nbt.tag.Tag;
 import com.viaversion.viabackwards.api.BackwardsProtocol;
 import com.viaversion.viabackwards.api.data.BackwardsMappingData;
 import com.viaversion.viabackwards.api.data.MappedItem;
 import com.viaversion.viaversion.api.connection.UserConnection;
+import com.viaversion.viaversion.api.minecraft.Holder;
+import com.viaversion.viaversion.api.minecraft.HolderSet;
 import com.viaversion.viaversion.api.minecraft.data.StructuredDataContainer;
 import com.viaversion.viaversion.api.minecraft.data.StructuredDataKey;
 import com.viaversion.viaversion.api.minecraft.item.Item;
@@ -34,6 +38,8 @@ import com.viaversion.viaversion.api.type.Type;
 import com.viaversion.viaversion.rewriter.StructuredItemRewriter;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.function.BiConsumer;
+import java.util.function.Function;
 import org.checkerframework.checker.nullness.qual.Nullable;
 
 public class BackwardsStructuredItemRewriter<C extends ClientboundPacketType, S extends ServerboundPacketType,
@@ -165,6 +171,54 @@ public class BackwardsStructuredItemRewriter<C extends ClientboundPacketType, S
         return new ArrayList<>(data.values());
     }
 
+    protected Tag holderSetToTag(final HolderSet set) {
+        if (set.hasIds()) {
+            return new IntArrayTag(set.ids());
+        } else {
+            return new StringTag(set.tagKey());
+        }
+    }
+
+    protected HolderSet restoreHolderSet(final CompoundTag tag, final String key) {
+        final Tag savedTag = tag.get(key);
+        if (savedTag == null) {
+            return HolderSet.of(new int[0]);
+        }
+
+        if (savedTag instanceof StringTag tagKey) {
+            return HolderSet.of(tagKey.getValue());
+        } else if (savedTag instanceof IntArrayTag idsTag) {
+            return HolderSet.of(idsTag.getValue());
+        } else {
+            return HolderSet.of(new int[0]);
+        }
+    }
+
+    protected <V> Tag holderToTag(final Holder<V> holder, final BiConsumer<V, CompoundTag> valueSaveFunction) {
+        if (holder.hasId()) {
+            return new IntTag(holder.id());
+        } else {
+            final CompoundTag savedTag = new CompoundTag();
+            valueSaveFunction.accept(holder.value(), savedTag);
+            return savedTag;
+        }
+    }
+
+    protected <V> Holder<V> restoreHolder(final CompoundTag tag, final String key, final Function<CompoundTag, V> valueRestoreFunction) {
+        final Tag savedTag = tag.get(key);
+        if (savedTag == null) {
+            return Holder.of(0);
+        }
+
+        if (savedTag instanceof IntTag idTag) {
+            return Holder.of(idTag.asInt());
+        } else if (savedTag instanceof CompoundTag compoundTag) {
+            return Holder.of(valueRestoreFunction.apply(compoundTag));
+        } else {
+            return Holder.of(0);
+        }
+    }
+
     @Override
     public String nbtTagName() {
         return "VB|" + protocol.getClass().getSimpleName();

common/src/main/java/com/viaversion/viabackwards/protocol/v1_21_2to1_21/rewriter/BlockItemPacketRewriter1_21_2.java

@@ -19,7 +19,7 @@ package com.viaversion.viabackwards.protocol.v1_21_2to1_21.rewriter;
 
 import com.viaversion.nbt.tag.ByteTag;
 import com.viaversion.nbt.tag.CompoundTag;
-import com.viaversion.nbt.tag.IntArrayTag;
+import com.viaversion.nbt.tag.FloatTag;
 import com.viaversion.nbt.tag.IntTag;
 import com.viaversion.nbt.tag.ListTag;
 import com.viaversion.nbt.tag.Tag;
@@ -321,9 +321,7 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
 
         final HolderSet repairable = data.get(StructuredDataKey.REPAIRABLE);
         if (repairable != null) {
-            final CompoundTag tag = new CompoundTag();
+            backupTag.put("repairable", holderSetToTag(repairable));
-            convertHolderSet(tag, repairable);
-            backupTag.put("repairable", tag);
         }
 
         final Integer enchantable = data.get(StructuredDataKey.ENCHANTABLE);
@@ -361,10 +359,7 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
                 tag.putString("camera_overlay", cameraOverlay);
             }
             if (equippable.allowedEntities() != null) {
-                final CompoundTag allowedEntities = new CompoundTag();
+                tag.put("allowed_entities", holderSetToTag(equippable.allowedEntities()));
-                convertHolderSet(allowedEntities, equippable.allowedEntities());
-
-                tag.put("allowed_entities", allowedEntities);
             }
             tag.putBoolean("dispensable", equippable.dispensable());
             tag.putBoolean("swappable", equippable.swappable());
@@ -417,11 +412,7 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
         } else if (effect.type() == Types.HOLDER_SET && effect.value() instanceof HolderSet set) {
             tag.putString("type", "remove_effects");
 
-            if (set.hasIds()) {
+            tag.put("remove_effects", holderSetToTag(set));
-                tag.put("ids", new IntArrayTag(set.ids()));
-            } else {
-                tag.putString("tag", set.tagKey());
-            }
         } else if (effect.type() == Types.EMPTY) {
             tag.putString("type", "clear_all_effects");
         } else if (effect.type() == Types.FLOAT) {
@@ -449,23 +440,12 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
     }
 
     private void convertSoundEventHolder(final CompoundTag tag, final Holder<SoundEvent> holder) {
-        if (holder.hasId()) {
+        tag.put("sound_event", holderToTag(holder, (event, soundEventTag) -> {
-            tag.putInt("sound", holder.id());
+            soundEventTag.putString("identifier", event.identifier());
-        } else {
-            final SoundEvent event = holder.value();
-            tag.putString("identifier", event.identifier());
             if (event.fixedRange() != null) {
-                tag.putFloat("fixed_range", event.fixedRange());
+                soundEventTag.putFloat("fixed_range", event.fixedRange());
             }
-        }
+        }));
-    }
-
-    private void convertHolderSet(final CompoundTag tag, final HolderSet set) {
-        if (set.hasIds()) {
-            tag.put("ids", new IntArrayTag(set.ids()));
-        } else {
-            tag.putString("tag", set.tagKey());
-        }
     }
 
     private Consumable1_21_2.ConsumeEffect<?> convertConsumableEffect(final CompoundTag tag) {
@@ -483,7 +463,7 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
             final float probability = tag.getFloat("probability");
             return new Consumable1_21_2.ConsumeEffect<>(id, Consumable1_21_2.ApplyStatusEffects.TYPE, new Consumable1_21_2.ApplyStatusEffects(potionEffects, probability));
         } else if ("remove_effects".equals(type)) {
-            final HolderSet set = convertHolderSet(tag);
+            final HolderSet set = restoreHolderSet(tag, "remove_effects");
             return new Consumable1_21_2.ConsumeEffect<>(id, Types.HOLDER_SET, set);
         } else if ("clear_all_effects".equals(type)) {
             return new Consumable1_21_2.ConsumeEffect<>(id, Types.EMPTY, Unit.INSTANCE);
@@ -508,41 +488,22 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
     }
 
     private Holder<SoundEvent> convertSoundEventHolder(final CompoundTag tag) {
-        final IntTag soundId = tag.getIntTag("sound");
+        return restoreHolder(tag, "sound_event", soundEventTag -> {
-        if (soundId != null) {
+            final String identifier = soundEventTag.getString("identifier");
-            return Holder.of(soundId.asInt());
+            final FloatTag fixedRange = soundEventTag.getFloatTag("fixed_range");
-        }
+            return new SoundEvent(identifier, fixedRange != null ? fixedRange.asFloat() : null);
-
+        });
-        final String identifier = tag.getString("identifier");
-        final Float fixedRange = tag.getFloat("fixed_range");
-        return Holder.of(new SoundEvent(identifier, fixedRange));
-    }
-
-    private HolderSet convertHolderSet(final CompoundTag tag) {
-        if (tag == null) {
-            return null;
-        }
-        final IntArrayTag ids = tag.getIntArrayTag("ids");
-        if (ids != null) {
-            return HolderSet.of(ids.getValue());
-        }
-        return HolderSet.of(tag.getString("tag"));
     }
 
     private void restoreInconvertibleData(final Item item) {
         final StructuredDataContainer data = item.dataContainer();
         final CompoundTag customData = data.get(StructuredDataKey.CUSTOM_DATA);
-        if (customData == null) {
+        if (customData == null || !(customData.remove(nbtTagName("inconvertible_data")) instanceof CompoundTag backupTag)) {
-            return;
-        }
-
-        final CompoundTag backupTag = customData.removeUnchecked(nbtTagName("inconvertible_data"));
-        if (backupTag == null) {
             return;
         }
 
         final Holder<Instrument1_21_2> instrument = data.get(StructuredDataKey.INSTRUMENT1_21_2);
-        if (instrument != null) {
+        if (instrument != null && instrument.isDirect()) {
             final Tag description = backupTag.get(nbtTagName("instrument_description"));
             if (description != null) {
                 final Instrument1_21_2 delegate = instrument.value();
@@ -550,10 +511,8 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
             }
         }
 
-        final IntArrayTag repairableIds = backupTag.getIntArrayTag("repairable_ids");
+        if (backupTag.contains("repairable")) {
-        final String repairableTag = backupTag.getString("repairable_tag");
+            data.set(StructuredDataKey.REPAIRABLE, restoreHolderSet(backupTag, "repairable"));
-        if (repairableIds != null || repairableTag != null) {
-            data.set(StructuredDataKey.REPAIRABLE, repairableIds != null ? HolderSet.of(repairableIds.getValue()) : HolderSet.of(repairableTag));
         }
 
         final IntTag enchantable = backupTag.getIntTag("enchantable");
@@ -564,7 +523,7 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
         final CompoundTag useCooldown = backupTag.getCompoundTag("use_cooldown");
         if (useCooldown != null) {
             final float seconds = useCooldown.getFloat("seconds");
-            final String cooldownGroup = useCooldown.getString("cooldown_group", null);
+            final String cooldownGroup = useCooldown.getString("cooldown_group");
             data.set(StructuredDataKey.USE_COOLDOWN, new UseCooldown(seconds, cooldownGroup));
         }
 
@@ -573,17 +532,16 @@ public final class BlockItemPacketRewriter1_21_2 extends BackwardsStructuredItem
             data.set(StructuredDataKey.ITEM_MODEL, itemModel);
         }
 
-        final CompoundTag equippable = backupTag.getCompoundTag("equippable");
+        final CompoundTag equippable = backupTag.getCompoundTag("equitable");
         if (equippable != null) {
             final int equipmentSlot = equippable.getInt("equipment_slot");
             final Holder<SoundEvent> soundEvent = convertSoundEventHolder(equippable);
-            final String model = equippable.getString("model", null);
+            final String model = equippable.getString("model");
-            final String cameraOverlay = equippable.getString("camera_overlay", null);
+            final String cameraOverlay = equippable.getString("camera_overlay");
-            final HolderSet allowedEntities = convertHolderSet(equippable.getCompoundTag("allowed_entities"));
+            final HolderSet allowedEntities = equippable.contains("allowed_entities") ? restoreHolderSet(equippable, "allowed_entities") : null;
             final boolean dispensable = equippable.getBoolean("dispensable");
             final boolean swappable = equippable.getBoolean("swappable");
             final boolean damageOnHurt = equippable.getBoolean("damage_on_hurt");
-
             data.set(StructuredDataKey.EQUIPPABLE, new Equippable(equipmentSlot, soundEvent, model, cameraOverlay, allowedEntities, dispensable, swappable, damageOnHurt));
         }
 

common/src/main/java/com/viaversion/viabackwards/protocol/v1_21to1_20_5/rewriter/BlockItemPacketRewriter1_21.java

@@ -279,25 +279,18 @@ public final class BlockItemPacketRewriter1_21 extends BackwardsStructuredItemRe
 
         final CompoundTag tag = new CompoundTag();
         if (jukeboxPlayable.song().isLeft()) {
-            final Holder<JukeboxPlayable.JukeboxSong> song = jukeboxPlayable.song().left();
+            final Holder<JukeboxPlayable.JukeboxSong> songHolder = jukeboxPlayable.song().left();
-            if (song.hasId()) {
+            tag.put("song", holderToTag(songHolder, (song, songTag) -> {
-                tag.putInt("song_id", song.id());
+                songTag.put("sound_event", holderToTag(song.soundEvent(), (soundEvent, soundEventTag) -> {
-            } else {
+                    soundEventTag.putString("identifier", soundEvent.identifier());
-                final JukeboxPlayable.JukeboxSong songData = song.value();
+                    if (soundEvent.fixedRange() != null) {
-                final Holder<SoundEvent> soundEvent = songData.soundEvent();
+                        soundEventTag.putFloat("fixed_range", soundEvent.fixedRange());
-                if (soundEvent.hasId()) {
-                    tag.putInt("sound", soundEvent.id());
-                } else {
-                    final SoundEvent event = soundEvent.value();
-                    tag.putString("identifier", event.identifier());
-                    if (event.fixedRange() != null) {
-                        tag.putFloat("fixed_range", event.fixedRange());
                     }
-                }
+                }));
-                tag.put("description", songData.description());
+                songTag.put("description", song.description());
-                tag.putFloat("length_in_seconds", songData.lengthInSeconds());
+                songTag.putFloat("length_in_seconds", song.lengthInSeconds());
-                tag.putInt("comparator_output", songData.comparatorOutput());
+                songTag.putInt("comparator_output", song.comparatorOutput());
-            }
+            }));
         } else {
             tag.putString("song_identifier", jukeboxPlayable.song().right());
         }
@@ -308,36 +301,26 @@ public final class BlockItemPacketRewriter1_21 extends BackwardsStructuredItemRe
     private void restoreInconvertibleData(final Item item) {
         final StructuredDataContainer data = item.dataContainer();
         final CompoundTag customData = data.get(StructuredDataKey.CUSTOM_DATA);
-        if (customData == null) {
+        if (customData == null || !(customData.remove(nbtTagName("jukebox_playable")) instanceof CompoundTag tag)) {
-            return;
-        }
-        final CompoundTag tag = customData.removeUnchecked(nbtTagName("jukebox_playable"));
-        if (tag == null) {
             return;
         }
 
         final Either<Holder<JukeboxPlayable.JukeboxSong>, String> song;
-        if (tag.contains("song_identifier")) {
+        final String songIdentifier = tag.getString("song_identifier");
-            song = Either.right(tag.getString("song_identifier"));
+        if (songIdentifier != null) {
+            song = Either.right(tag.getString(songIdentifier));
         } else {
-            final Holder<JukeboxPlayable.JukeboxSong> songData;
+            song = Either.left(restoreHolder(tag, "song", songTag -> {
-            if (tag.contains("song_id")) {
+                final Holder<SoundEvent> soundEvent = restoreHolder(songTag, "sound_event", soundTag -> {
-                songData = Holder.of(tag.getInt("song_id"));
+                    final String identifier = soundTag.getString("identifier");
-            } else {
+                    final Float fixedRange = soundTag.contains("fixed_range") ? soundTag.getFloat("fixed_range") : null;
-                final Holder<SoundEvent> soundEvent;
+                    return new SoundEvent(identifier, fixedRange);
-                if (tag.contains("sound")) {
+                });
-                    soundEvent = Holder.of(tag.getInt("sound"));
+                final Tag description = songTag.get("description");
-                } else {
+                final float lengthInSeconds = songTag.getFloat("length_in_seconds");
-                    final String identifier = tag.getString("identifier");
+                final int comparatorOutput = songTag.getInt("comparator_output");
-                    final Float fixedRange = tag.contains("fixed_range") ? tag.getFloat("fixed_range") : null;
+                return new JukeboxPlayable.JukeboxSong(soundEvent, description, lengthInSeconds, comparatorOutput);
-                    soundEvent = Holder.of(new SoundEvent(identifier, fixedRange));
+            }));
-                }
-                final Tag description = tag.get("description");
-                final float lengthInSeconds = tag.getFloat("length_in_seconds");
-                final int comparatorOutput = tag.getInt("comparator_output");
-                songData = Holder.of(new JukeboxPlayable.JukeboxSong(soundEvent, description, lengthInSeconds, comparatorOutput));
-            }
-            song = Either.left(songData);
         }
 
         final JukeboxPlayable jukeboxPlayable = new JukeboxPlayable(song, tag.getBoolean("show_in_tooltip"));