From 279e0f01ea582f06515cba9f6f5133d3131ac720 Mon Sep 17 00:00:00 2001 From: KennyTV Date: Wed, 3 Mar 2021 13:53:15 +0100 Subject: [PATCH] Fix some (previously) existing issues with name/lore saving, cleanup --- .../viabackwards/api/data/MappedItem.java | 2 +- .../api/data/MappedLegacyBlockItem.java | 2 +- .../api/rewriters/EnchantmentRewriter.java | 106 ++++++++---------- .../api/rewriters/ItemRewriter.java | 10 +- .../api/rewriters/ItemRewriterBase.java | 49 ++++++-- .../rewriters/LegacyBlockItemRewriter.java | 2 +- .../packets/BlockItemPackets1_13.java | 8 +- .../packets/BlockItemPackets1_14.java | 66 +++++------ .../packets/BlockItemPackets1_16.java | 2 +- 9 files changed, 126 insertions(+), 121 deletions(-) diff --git a/common/src/main/java/nl/matsv/viabackwards/api/data/MappedItem.java b/common/src/main/java/nl/matsv/viabackwards/api/data/MappedItem.java index af34c63f..5dc58cce 100644 --- a/common/src/main/java/nl/matsv/viabackwards/api/data/MappedItem.java +++ b/common/src/main/java/nl/matsv/viabackwards/api/data/MappedItem.java @@ -9,7 +9,7 @@ public class MappedItem { public MappedItem(int id, String name) { this.id = id; - this.jsonName = ChatRewriter.legacyTextToJsonString(name); + this.jsonName = ChatRewriter.legacyTextToJsonString("§f" + name); } public int getId() { diff --git a/common/src/main/java/nl/matsv/viabackwards/api/data/MappedLegacyBlockItem.java b/common/src/main/java/nl/matsv/viabackwards/api/data/MappedLegacyBlockItem.java index 48c1c193..8a72f952 100644 --- a/common/src/main/java/nl/matsv/viabackwards/api/data/MappedLegacyBlockItem.java +++ b/common/src/main/java/nl/matsv/viabackwards/api/data/MappedLegacyBlockItem.java @@ -15,7 +15,7 @@ public class MappedLegacyBlockItem { public MappedLegacyBlockItem(int id, short data, @Nullable String name, boolean block) { this.id = id; this.data = data; - this.name = name != null ? "§r" + name : null; + this.name = name != null ? "§f" + name : null; this.block = block ? new Block(id, data) : null; } diff --git a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/EnchantmentRewriter.java b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/EnchantmentRewriter.java index cf5e3574..fe784c5a 100644 --- a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/EnchantmentRewriter.java +++ b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/EnchantmentRewriter.java @@ -2,7 +2,6 @@ package nl.matsv.viabackwards.api.rewriters; import us.myles.ViaVersion.api.minecraft.item.Item; import us.myles.ViaVersion.protocols.protocol1_13to1_12_2.ChatRewriter; -import us.myles.viaversion.libs.opennbt.tag.builtin.ByteTag; import us.myles.viaversion.libs.opennbt.tag.builtin.CompoundTag; import us.myles.viaversion.libs.opennbt.tag.builtin.ListTag; import us.myles.viaversion.libs.opennbt.tag.builtin.ShortTag; @@ -11,22 +10,26 @@ import us.myles.viaversion.libs.opennbt.tag.builtin.Tag; import java.util.ArrayList; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; +/** + * Rewriter to handle the addition of new enchantments. + */ public class EnchantmentRewriter { private final Map enchantmentMappings = new HashMap<>(); - private final String nbtTagName; + private final ItemRewriter itemRewriter; private final boolean jsonFormat; - public EnchantmentRewriter(String nbtTagName, boolean jsonFormat) { - this.nbtTagName = nbtTagName; + public EnchantmentRewriter(ItemRewriter itemRewriter, boolean jsonFormat) { + this.itemRewriter = itemRewriter; this.jsonFormat = jsonFormat; } - public EnchantmentRewriter(String nbtTagName) { - this(nbtTagName, true); + public EnchantmentRewriter(ItemRewriter itemRewriter) { + this(itemRewriter, true); } public void registerEnchantment(String key, String replacementLore) { @@ -49,10 +52,10 @@ public class EnchantmentRewriter { CompoundTag tag = item.getTag(); if (tag == null) return; - if (tag.contains(nbtTagName + "|Enchantments")) { + if (tag.contains(itemRewriter.getNbtTagName() + "|Enchantments")) { rewriteEnchantmentsToServer(tag, false); } - if (tag.contains(nbtTagName + "|StoredEnchantments")) { + if (tag.contains(itemRewriter.getNbtTagName() + "|StoredEnchantments")) { rewriteEnchantmentsToServer(tag, true); } } @@ -60,86 +63,67 @@ public class EnchantmentRewriter { public void rewriteEnchantmentsToClient(CompoundTag tag, boolean storedEnchant) { String key = storedEnchant ? "StoredEnchantments" : "Enchantments"; ListTag enchantments = tag.get(key); - ListTag remappedEnchantments = new ListTag(nbtTagName + "|" + key, CompoundTag.class); - List lore = new ArrayList<>(); - for (Tag enchantmentEntry : enchantments.clone()) { - Tag idTag = ((CompoundTag) enchantmentEntry).get("id"); + List loreToAdd = new ArrayList<>(); + boolean changed = false; + + Iterator iterator = enchantments.iterator(); + while (iterator.hasNext()) { + CompoundTag enchantmentEntry = (CompoundTag) iterator.next(); + StringTag idTag = enchantmentEntry.get("id"); if (idTag == null) continue; - String newId = (String) idTag.getValue(); - String enchantmentName = enchantmentMappings.get(newId); - if (enchantmentName != null) { - enchantments.remove(enchantmentEntry); - Number level = (Number) ((CompoundTag) enchantmentEntry).get("lvl").getValue(); - String loreValue = enchantmentName + " " + getRomanNumber(level.intValue()); - if (jsonFormat) { - loreValue = ChatRewriter.legacyTextToJson(loreValue).toString(); + String enchantmentId = idTag.getValue(); + String remappedName = enchantmentMappings.get(enchantmentId); + if (remappedName != null) { + if (!changed) { + // Backup original before doing modifications + itemRewriter.saveListTag(tag, enchantments); + changed = true; } - lore.add(new StringTag("", loreValue)); - remappedEnchantments.add(enchantmentEntry); + iterator.remove(); + + Number level = (Number) enchantmentEntry.get("lvl").getValue(); + String loreValue = remappedName + " " + getRomanNumber(level.intValue()); + if (jsonFormat) { + loreValue = ChatRewriter.legacyTextToJsonString(loreValue); + } + + loreToAdd.add(new StringTag("", loreValue)); } } - if (!lore.isEmpty()) { + + if (!loreToAdd.isEmpty()) { + // Add dummy enchant for the glow effect if there are no actual enchantments left if (!storedEnchant && enchantments.size() == 0) { CompoundTag dummyEnchantment = new CompoundTag(""); dummyEnchantment.put(new StringTag("id", "")); dummyEnchantment.put(new ShortTag("lvl", (short) 0)); enchantments.add(dummyEnchantment); - - tag.put(new ByteTag(nbtTagName + "|dummyEnchant")); } - tag.put(remappedEnchantments); - CompoundTag display = tag.get("display"); if (display == null) { tag.put(display = new CompoundTag("display")); } + ListTag loreTag = display.get("Lore"); if (loreTag == null) { display.put(loreTag = new ListTag("Lore", StringTag.class)); + } else { + // Save original lore + itemRewriter.saveListTag(display, loreTag); } - lore.addAll(loreTag.getValue()); - loreTag.setValue(lore); + loreToAdd.addAll(loreTag.getValue()); + loreTag.setValue(loreToAdd); } } public void rewriteEnchantmentsToServer(CompoundTag tag, boolean storedEnchant) { + // Just restore the original tag ig present (lore is always restored in the item rewriter) String key = storedEnchant ? "StoredEnchantments" : "Enchantments"; - ListTag remappedEnchantments = tag.get(nbtTagName + "|" + key); - ListTag enchantments = tag.get(key); - if (enchantments == null) { - enchantments = new ListTag(key, CompoundTag.class); - } - - if (!storedEnchant && tag.remove(nbtTagName + "|dummyEnchant") != null) { - for (Tag enchantment : enchantments.clone()) { - String id = (String) ((CompoundTag) enchantment).get("id").getValue(); - if (id.isEmpty()) { - enchantments.remove(enchantment); - } - } - } - - CompoundTag display = tag.get("display"); - // A few null checks just to be safe, though they shouldn't actually be - ListTag lore = display != null ? display.get("Lore") : null; - for (Tag enchantment : remappedEnchantments.clone()) { - enchantments.add(enchantment); - if (lore != null && lore.size() != 0) { - lore.remove(lore.get(0)); - } - } - if (lore != null && lore.size() == 0) { - display.remove("Lore"); - if (display.isEmpty()) { - tag.remove("display"); - } - } - tag.put(enchantments); - tag.remove(remappedEnchantments.getName()); + itemRewriter.restoreListTag(tag, key); } public static String getRomanNumber(int number) { diff --git a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriter.java b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriter.java index 418c8b5a..f58f4f96 100644 --- a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriter.java +++ b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriter.java @@ -32,7 +32,7 @@ public abstract class ItemRewriter extends ItemRewr if (name != null) { String newValue = translatableRewriter.processText(name.getValue()).toString(); if (!newValue.equals(name.getValue())) { - saveNameTag(display, name); + saveStringTag(display, name); } name.setValue(newValue); @@ -40,7 +40,6 @@ public abstract class ItemRewriter extends ItemRewr ListTag lore = display.get("Lore"); if (lore != null) { - ListTag original = null; boolean changed = false; for (Tag loreEntryTag : lore) { if (!(loreEntryTag instanceof StringTag)) continue; @@ -48,16 +47,13 @@ public abstract class ItemRewriter extends ItemRewr StringTag loreEntry = (StringTag) loreEntryTag; String newValue = translatableRewriter.processText(loreEntry.getValue()).toString(); if (!changed && !newValue.equals(loreEntry.getValue())) { + // Backup original lore before doing any modifications changed = true; - original = lore.clone(); + saveListTag(display, lore); } loreEntry.setValue(newValue); } - - if (changed) { - saveLoreTag(display, original); - } } } diff --git a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriterBase.java b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriterBase.java index 8761c355..9ecf58ec 100644 --- a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriterBase.java +++ b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/ItemRewriterBase.java @@ -3,7 +3,6 @@ package nl.matsv.viabackwards.api.rewriters; import nl.matsv.viabackwards.api.BackwardsProtocol; import org.jetbrains.annotations.Nullable; import us.myles.ViaVersion.api.minecraft.item.Item; -import us.myles.viaversion.libs.opennbt.conversion.builtin.CompoundTagConverter; import us.myles.viaversion.libs.opennbt.tag.builtin.CompoundTag; import us.myles.viaversion.libs.opennbt.tag.builtin.ListTag; import us.myles.viaversion.libs.opennbt.tag.builtin.StringTag; @@ -11,7 +10,6 @@ import us.myles.viaversion.libs.opennbt.tag.builtin.Tag; public abstract class ItemRewriterBase extends Rewriter { - protected static final CompoundTagConverter CONVERTER = new CompoundTagConverter(); protected final String nbtTagName; protected final boolean jsonNameFormat; @@ -40,12 +38,30 @@ public abstract class ItemRewriterBase extends Rewr return item; } - protected void saveNameTag(CompoundTag displayTag, StringTag original) { - displayTag.put(new StringTag(nbtTagName + "|o" + original.getName(), original.getValue())); + protected boolean hasBackupTag(CompoundTag displayTag, String tagName) { + return displayTag.contains(nbtTagName + "|o" + tagName); } - protected void saveLoreTag(CompoundTag displayTag, ListTag original) { - displayTag.put(new ListTag(nbtTagName + "|o" + original.getName(), original.getValue())); + protected void saveStringTag(CompoundTag displayTag, StringTag original) { + // Multiple places might try to backup data + String name = nbtTagName + "|o" + original.getName(); + if (!displayTag.contains(name)) { + displayTag.put(new StringTag(name, original.getValue())); + } + } + + protected void saveListTag(CompoundTag displayTag, ListTag original) { + // Multiple places might try to backup data + String name = nbtTagName + "|o" + original.getName(); + if (!displayTag.contains(name)) { + // Clone all tag entries + ListTag listTag = new ListTag(name); + for (Tag tag : original.getValue()) { + listTag.add(tag.clone()); + } + + displayTag.put(listTag); + } } protected void restoreDisplayTag(Item item) { @@ -57,18 +73,29 @@ public abstract class ItemRewriterBase extends Rewr if (display.remove(nbtTagName + "|customName") != null) { display.remove("Name"); } else { - restoreDisplayTag(display, "Name"); + restoreStringTag(display, "Name"); } // Restore lore - restoreDisplayTag(display, "Lore"); + restoreListTag(display, "Lore"); } } - protected void restoreDisplayTag(CompoundTag displayTag, String tagName) { - Tag original = displayTag.remove(nbtTagName + "|o" + tagName); + protected void restoreStringTag(CompoundTag tag, String tagName) { + StringTag original = tag.remove(nbtTagName + "|o" + tagName); if (original != null) { - displayTag.put(original); + tag.put(new StringTag(tagName, original.getValue())); } } + + protected void restoreListTag(CompoundTag tag, String tagName) { + ListTag original = tag.remove(nbtTagName + "|o" + tagName); + if (original != null) { + tag.put(new ListTag(tagName, original.getValue())); + } + } + + public String getNbtTagName() { + return nbtTagName; + } } diff --git a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/LegacyBlockItemRewriter.java b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/LegacyBlockItemRewriter.java index 7c7c0a22..c3011037 100644 --- a/common/src/main/java/nl/matsv/viabackwards/api/rewriters/LegacyBlockItemRewriter.java +++ b/common/src/main/java/nl/matsv/viabackwards/api/rewriters/LegacyBlockItemRewriter.java @@ -240,7 +240,7 @@ public abstract class LegacyBlockItemRewriter exten CompoundTag tag = new CompoundTag(""); tag.put(new CompoundTag("display")); text = "§r" + text; - ((CompoundTag) tag.get("display")).put(new StringTag("Name", jsonNameFormat ? ChatRewriter.legacyTextToJson(text).toString() : text)); + ((CompoundTag) tag.get("display")).put(new StringTag("Name", jsonNameFormat ? ChatRewriter.legacyTextToJsonString(text) : text)); return tag; } diff --git a/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java b/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java index 4ccc2cda..76dc9e5b 100644 --- a/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java +++ b/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_12_2to1_13/packets/BlockItemPackets1_13.java @@ -574,7 +574,7 @@ public class BlockItemPackets1_13 extends nl.matsv.viabackwards.api.rewriters.It CompoundTag display = tag.get("display"); if (display != null) { StringTag name = display.get("Name"); - if (name instanceof StringTag) { + if (name != null) { display.put(new StringTag(extraNbtTag + "|Name", name.getValue())); name.setValue(ChatRewriter.jsonToLegacyText(name.getValue())); } @@ -630,6 +630,7 @@ public class BlockItemPackets1_13 extends nl.matsv.viabackwards.api.rewriters.It tag.put(newCanPlaceOn); } + //TODO un-ugly all of this private void rewriteEnchantmentsToClient(CompoundTag tag, boolean storedEnch) { String key = storedEnch ? "StoredEnchantments" : "Enchantments"; ListTag enchantments = tag.get(key); @@ -775,7 +776,7 @@ public class BlockItemPackets1_13 extends nl.matsv.viabackwards.api.rewriters.It if (display instanceof CompoundTag) { CompoundTag displayTag = (CompoundTag) display; StringTag name = displayTag.get("Name"); - if (name instanceof StringTag) { + if (name != null) { StringTag via = displayTag.remove(extraNbtTag + "|Name"); name.setValue(via != null ? via.getValue() : ChatRewriter.legacyTextToJsonString(name.getValue())); } @@ -863,7 +864,8 @@ public class BlockItemPackets1_13 extends nl.matsv.viabackwards.api.rewriters.It for (Tag oldTag : blockTag) { Object value = oldTag.getValue(); String oldId = value.toString().replace("minecraft:", ""); - String numberConverted = BlockIdData.numberIdToString.get(Ints.tryParse(oldId)); + int key = Ints.tryParse(oldId); + String numberConverted = BlockIdData.numberIdToString.get(key); if (numberConverted != null) { oldId = numberConverted; } diff --git a/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_13_2to1_14/packets/BlockItemPackets1_14.java b/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_13_2to1_14/packets/BlockItemPackets1_14.java index d207a252..fa48fb78 100644 --- a/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_13_2to1_14/packets/BlockItemPackets1_14.java +++ b/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_13_2to1_14/packets/BlockItemPackets1_14.java @@ -35,7 +35,6 @@ import us.myles.ViaVersion.protocols.protocol1_14to1_13_2.types.Chunk1_14Type; import us.myles.ViaVersion.protocols.protocol1_9_3to1_9_1_2.storage.ClientWorld; import us.myles.viaversion.libs.gson.JsonElement; import us.myles.viaversion.libs.gson.JsonObject; -import us.myles.viaversion.libs.opennbt.conversion.ConverterRegistry; import us.myles.viaversion.libs.opennbt.tag.builtin.CompoundTag; import us.myles.viaversion.libs.opennbt.tag.builtin.ListTag; import us.myles.viaversion.libs.opennbt.tag.builtin.StringTag; @@ -518,7 +517,7 @@ public class BlockItemPackets1_14 extends nl.matsv.viabackwards.api.rewriters.It @Override protected void registerRewrites() { - enchantmentRewriter = new EnchantmentRewriter(nbtTagName, false); + enchantmentRewriter = new EnchantmentRewriter(this, false); enchantmentRewriter.registerEnchantment("minecraft:multishot", "§7Multishot"); enchantmentRewriter.registerEnchantment("minecraft:quick_charge", "§7Quick Charge"); enchantmentRewriter.registerEnchantment("minecraft:piercing", "§7Piercing"); @@ -529,57 +528,54 @@ public class BlockItemPackets1_14 extends nl.matsv.viabackwards.api.rewriters.It if (item == null) return null; super.handleItemToClient(item); + // Lore now uses JSON CompoundTag tag = item.getTag(); - if (tag != null) { - // Lore now uses JSON - if (tag.get("display") instanceof CompoundTag) { - CompoundTag display = tag.get("display"); - if (((CompoundTag) tag.get("display")).get("Lore") instanceof ListTag) { - ListTag lore = display.get("Lore"); - ListTag via = display.remove(nbtTagName + "|Lore"); - if (via != null) { - display.put(ConverterRegistry.convertToTag("Lore", ConverterRegistry.convertToValue(via))); - } else { - for (Tag loreEntry : lore) { - if (!(loreEntry instanceof StringTag)) continue; + CompoundTag display; + if (tag != null && (display = tag.get("display")) != null) { + ListTag lore = display.get("Lore"); + if (lore != null) { + saveListTag(display, lore); - String value = ((StringTag) loreEntry).getValue(); - if (value != null && !value.isEmpty()) { - ((StringTag) loreEntry).setValue(ChatRewriter.jsonToLegacyText(value)); - } - } + for (Tag loreEntry : lore) { + if (!(loreEntry instanceof StringTag)) continue; + + StringTag loreEntryTag = (StringTag) loreEntry; + String value = loreEntryTag.getValue(); + if (value != null && !value.isEmpty()) { + loreEntryTag.setValue(ChatRewriter.jsonToLegacyText(value)); } } } - - enchantmentRewriter.handleToClient(item); } + + enchantmentRewriter.handleToClient(item); return item; } @Override public Item handleItemToServer(Item item) { if (item == null) return null; - super.handleItemToServer(item); + // Lore now uses JSON CompoundTag tag = item.getTag(); - if (tag != null) { - // Display Lore now uses JSON - if (tag.get("display") instanceof CompoundTag) { - CompoundTag display = tag.get("display"); - if (display.get("Lore") instanceof ListTag) { - ListTag lore = display.get("Lore"); - display.put(ConverterRegistry.convertToTag(nbtTagName + "|Lore", ConverterRegistry.convertToValue(lore))); - for (Tag loreEntry : lore) { - if (loreEntry instanceof StringTag) { - ((StringTag) loreEntry).setValue(ChatRewriter.legacyTextToJson(((StringTag) loreEntry).getValue()).toString()); - } + CompoundTag display; + if (tag != null && (display = tag.get("display")) != null) { + // Transform to json if no backup tag is found (else process that in the super method) + ListTag lore = display.get("Lore"); + if (lore != null && !hasBackupTag(display, "Lore")) { + for (Tag loreEntry : lore) { + if (loreEntry instanceof StringTag) { + StringTag loreEntryTag = (StringTag) loreEntry; + loreEntryTag.setValue(ChatRewriter.legacyTextToJsonString(loreEntryTag.getValue())); } } } - - enchantmentRewriter.handleToServer(item); } + + enchantmentRewriter.handleToServer(item); + + // Call this last to check for the backup lore above + super.handleItemToServer(item); return item; } } diff --git a/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_15_2to1_16/packets/BlockItemPackets1_16.java b/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_15_2to1_16/packets/BlockItemPackets1_16.java index eb7e25bb..7fdf8e44 100644 --- a/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_15_2to1_16/packets/BlockItemPackets1_16.java +++ b/common/src/main/java/nl/matsv/viabackwards/protocol/protocol1_15_2to1_16/packets/BlockItemPackets1_16.java @@ -293,7 +293,7 @@ public class BlockItemPackets1_16 extends nl.matsv.viabackwards.api.rewriters.It @Override protected void registerRewrites() { - enchantmentRewriter = new EnchantmentRewriter(nbtTagName); + enchantmentRewriter = new EnchantmentRewriter(this); enchantmentRewriter.registerEnchantment("minecraft:soul_speed", "§7Soul Speed"); }