From c8231eb050c1e4635d1592b041d9f29b9ed9d843 Mon Sep 17 00:00:00 2001 From: tr7zw Date: Mon, 12 Jun 2023 18:17:37 +0200 Subject: [PATCH 1/4] Testing readOnly mode for NBT.get(ItemStack) --- .../java/de/tr7zw/changeme/nbtapi/NBT.java | 2 +- .../de/tr7zw/changeme/nbtapi/NBTItem.java | 32 +++++++++++++++++++ .../changeme/nbtapi/NBTReflectionUtil.java | 22 +++++++++++++ .../plugin/tests/items/DirectApplyTest.java | 2 ++ 4 files changed, 57 insertions(+), 1 deletion(-) diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java index 1aad85c61..4eb0b44e2 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java @@ -36,7 +36,7 @@ private NBT() { * @return The function is being returned. */ public static T get(ItemStack item, Function getter) { - return getter.apply(new NBTItem(item)); + return getter.apply(new NBTItem(item, false, true)); } /** diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTItem.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTItem.java index ece38cf0c..d609e601a 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTItem.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTItem.java @@ -9,6 +9,7 @@ import de.tr7zw.changeme.nbtapi.iface.ReadWriteItemNBT; import de.tr7zw.changeme.nbtapi.iface.ReadWriteNBT; import de.tr7zw.changeme.nbtapi.iface.ReadableNBT; +import de.tr7zw.changeme.nbtapi.utils.nmsmappings.ClassWrapper; import de.tr7zw.changeme.nbtapi.utils.nmsmappings.ReflectionMethod; /** @@ -22,6 +23,7 @@ public class NBTItem extends NBTCompound implements ReadWriteItemNBT { private ItemStack bukkitItem; private boolean directApply; + private boolean readOnly; private ItemStack originalSrcStack = null; /** @@ -33,6 +35,30 @@ public NBTItem(ItemStack item) { this(item, false); } + /** + * @param item + * @param directApply + * @param readOnly When turned on, no copy of the source item is created. + * Modifying the stack in that case is not valid! Also + * overwrites directApply + */ + protected NBTItem(ItemStack item, boolean directApply, boolean readOnly) { + super(null, null); + if (item == null || item.getType() == Material.AIR || item.getAmount() <= 0) { + throw new NullPointerException("ItemStack can't be null/air/amount of 0! This is not a NBTAPI bug!"); + } + if (readOnly) { + bukkitItem = item; + this.readOnly = true; + } else { + this.directApply = directApply; + bukkitItem = item.clone(); + if (directApply) { + this.originalSrcStack = item; + } + } + } + /** * Constructor for NBTItems. The ItemStack will be cloned! If directApply is * true, all changed will be mapped to the original item. Changes to the NBTItem @@ -55,11 +81,17 @@ public NBTItem(ItemStack item, boolean directApply) { @Override public Object getCompound() { + if (readOnly && ClassWrapper.CRAFT_ITEMSTACK.getClazz().isAssignableFrom(bukkitItem.getClass())) { + return NBTReflectionUtil.getItemRootNBTTagCompound(NBTReflectionUtil.getCraftItemHandle(bukkitItem)); + } return NBTReflectionUtil.getItemRootNBTTagCompound(ReflectionMethod.ITEMSTACK_NMSCOPY.run(null, bukkitItem)); } @Override protected void setCompound(Object compound) { + if (readOnly) { + throw new NbtApiException("Tried setting data in read only mode!"); + } Object stack = ReflectionMethod.ITEMSTACK_NMSCOPY.run(null, bukkitItem); ReflectionMethod.ITEMSTACK_SET_TAG.run(stack, compound); bukkitItem = (ItemStack) ReflectionMethod.ITEMSTACK_BUKKITMIRROR.run(null, stack); diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java index 3c8392260..2a225c641 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java @@ -13,6 +13,7 @@ import org.bukkit.block.BlockState; import org.bukkit.entity.Entity; +import org.bukkit.inventory.ItemStack; import org.bukkit.inventory.meta.ItemMeta; import de.tr7zw.changeme.nbtapi.utils.GsonWrapper; @@ -32,6 +33,7 @@ public class NBTReflectionUtil { private static Field field_unhandledTags = null; + private static Field field_handle = null; static { try { @@ -39,6 +41,12 @@ public class NBTReflectionUtil { field_unhandledTags.setAccessible(true); } catch (NoSuchFieldException e) { + } + try { + field_handle = ClassWrapper.CRAFT_ITEMSTACK.getClazz().getDeclaredField("handle"); + field_handle.setAccessible(true); + } catch (NoSuchFieldException e) { + } } @@ -96,6 +104,20 @@ public static Object writeNBT(Object nbt, OutputStream stream) { } } + /** + * Gets the nms handle ItemStack from a CraftItemStack. Passing Spigot ItemStacks will cause an error! + * + * @param item + * @return + */ + public static Object getCraftItemHandle(ItemStack item) { + try { + return field_handle.get(item); + } catch (IllegalArgumentException | IllegalAccessException e) { + throw new NbtApiException("Error getting handle from " + item.getClass(), e); + } + } + /** * Writes a Compound to an OutputStream * diff --git a/item-nbt-plugin/src/main/java/de/tr7zw/nbtapi/plugin/tests/items/DirectApplyTest.java b/item-nbt-plugin/src/main/java/de/tr7zw/nbtapi/plugin/tests/items/DirectApplyTest.java index 24eb03542..035fd54b1 100644 --- a/item-nbt-plugin/src/main/java/de/tr7zw/nbtapi/plugin/tests/items/DirectApplyTest.java +++ b/item-nbt-plugin/src/main/java/de/tr7zw/nbtapi/plugin/tests/items/DirectApplyTest.java @@ -23,6 +23,8 @@ public void test() throws Exception { nbt.setString("SomeKey", "SomeValue"); return nbt.getString("SomeKey"); }); + baseItem = NBT.itemStackFromNBT(NBT.itemStackToNBT(baseItem)); // trick to force the item to be "real", not a + // Spigot only item String outside = NBT.get(baseItem, nbt -> nbt.getString("SomeKey")); if (!new NBTItem(baseItem).hasTag("SomeKey")) { throw new NbtApiException("The data was not applied!"); From cfd04fffa6d35031034c171b60bef27e2a3f2e8c Mon Sep 17 00:00:00 2001 From: tr7zw Date: Mon, 12 Jun 2023 18:21:35 +0200 Subject: [PATCH 2/4] Format --- .../main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java index 2a225c641..172b6c12f 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTReflectionUtil.java @@ -105,7 +105,8 @@ public static Object writeNBT(Object nbt, OutputStream stream) { } /** - * Gets the nms handle ItemStack from a CraftItemStack. Passing Spigot ItemStacks will cause an error! + * Gets the nms handle ItemStack from a CraftItemStack. Passing Spigot + * ItemStacks will cause an error! * * @param item * @return From 2a80e8ff63db069dc808407b865dfb5c3befd5f4 Mon Sep 17 00:00:00 2001 From: tr7zw Date: Sun, 2 Jul 2023 20:48:53 +0200 Subject: [PATCH 3/4] Add readonly modes for Entity/TileEntity --- .../java/de/tr7zw/changeme/nbtapi/NBT.java | 4 +-- .../de/tr7zw/changeme/nbtapi/NBTEntity.java | 29 +++++++++++++++++++ .../tr7zw/changeme/nbtapi/NBTTileEntity.java | 29 +++++++++++++++++++ 3 files changed, 60 insertions(+), 2 deletions(-) diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java index 4eb0b44e2..40585797f 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java @@ -48,7 +48,7 @@ public static T get(ItemStack item, Function getter) { * @return The NBTEntity class is being returned. */ public static T get(Entity entity, Function getter) { - return getter.apply(new NBTEntity(entity)); + return getter.apply(new NBTEntity(entity, true)); } /** @@ -62,7 +62,7 @@ public static T get(Entity entity, Function getter) { * @return The return type is the same as the type of the getter function. */ public static T get(BlockState blockState, Function getter) { - return getter.apply(new NBTTileEntity(blockState)); + return getter.apply(new NBTTileEntity(blockState, true)); } /** diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTEntity.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTEntity.java index 58c1a0526..9b5edece0 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTEntity.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTEntity.java @@ -19,6 +19,26 @@ public class NBTEntity extends NBTCompound { private final Entity ent; + private final boolean readonly; + private final Object compound; + + /** + * @param entity Any valid Bukkit Entity + * @param readonly Readonly makes a copy at init, only reading from that copy + */ + protected NBTEntity(Entity entity, boolean readonly) { + super(null, null); + if (entity == null) { + throw new NullPointerException("Entity can't be null!"); + } + this.readonly = readonly; + ent = entity; + if (readonly) { + this.compound = getCompound(); + } else { + this.compound = null; + } + } /** * @param entity Any valid Bukkit Entity @@ -28,11 +48,17 @@ public NBTEntity(Entity entity) { if (entity == null) { throw new NullPointerException("Entity can't be null!"); } + this.readonly = false; + this.compound = null; ent = entity; } @Override public Object getCompound() { + // this runs before async check, since it's just a copy + if (readonly && compound != null) { + return compound; + } if (!Bukkit.isPrimaryThread()) throw new NbtApiException("Entity NBT needs to be accessed sync!"); return NBTReflectionUtil.getEntityNBTTagCompound(NBTReflectionUtil.getNMSEntity(ent)); @@ -40,6 +66,9 @@ public Object getCompound() { @Override protected void setCompound(Object compound) { + if (readonly) { + throw new NbtApiException("Tried setting data in read only mode!"); + } if (!Bukkit.isPrimaryThread()) throw new NbtApiException("Entity NBT needs to be accessed sync!"); NBTReflectionUtil.setEntityNBTTag(compound, NBTReflectionUtil.getNMSEntity(ent)); diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTTileEntity.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTTileEntity.java index c50758fa2..656063715 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTTileEntity.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBTTileEntity.java @@ -20,6 +20,26 @@ public class NBTTileEntity extends NBTCompound { private final BlockState tile; + private final boolean readonly; + private final Object compound; + + /** + * @param tile BlockState from any TileEntity + * @param readonly Readonly makes a copy at init, only reading from that copy + */ + public NBTTileEntity(BlockState tile, boolean readonly) { + super(null, null); + if (tile == null || (MinecraftVersion.isAtLeastVersion(MinecraftVersion.MC1_8_R3) && !tile.isPlaced())) { + throw new NullPointerException("Tile can't be null/not placed!"); + } + this.tile = tile; + this.readonly = readonly; + if (readonly) { + this.compound = getCompound(); + } else { + this.compound = null; + } + } /** * @param tile BlockState from any TileEntity @@ -29,11 +49,17 @@ public NBTTileEntity(BlockState tile) { if (tile == null || (MinecraftVersion.isAtLeastVersion(MinecraftVersion.MC1_8_R3) && !tile.isPlaced())) { throw new NullPointerException("Tile can't be null/not placed!"); } + this.readonly = false; + this.compound = null; this.tile = tile; } @Override public Object getCompound() { + // this runs before async check, since it's just a copy + if (readonly && compound != null) { + return compound; + } if (!Bukkit.isPrimaryThread()) throw new NbtApiException("BlockEntity NBT needs to be accessed sync!"); return NBTReflectionUtil.getTileEntityNBTTagCompound(tile); @@ -41,6 +67,9 @@ public Object getCompound() { @Override protected void setCompound(Object compound) { + if (readonly) { + throw new NbtApiException("Tried setting data in read only mode!"); + } if (!Bukkit.isPrimaryThread()) throw new NbtApiException("BlockEntity NBT needs to be accessed sync!"); NBTReflectionUtil.setTileEntityNBTTagCompound(tile, compound); From 0228b68a7c003d25be016e0359e43422249df11c Mon Sep 17 00:00:00 2001 From: tr7zw Date: Mon, 3 Jul 2023 23:47:43 +0200 Subject: [PATCH 4/4] Change NBT.modify calls on (Block-)Entities to be atomic --- .../java/de/tr7zw/changeme/nbtapi/NBT.java | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java index 40585797f..e6b008751 100644 --- a/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java +++ b/item-nbt-api/src/main/java/de/tr7zw/changeme/nbtapi/NBT.java @@ -130,7 +130,11 @@ public static void modify(ItemStack item, Consumer consumer) { * @return The return type is the same as the return type of the function. */ public static T modify(Entity entity, Function function) { - return function.apply(new NBTEntity(entity)); + NBTEntity nbtEnt = new NBTEntity(entity); + NBTContainer cont = new NBTContainer(nbtEnt.getCompound()); + T ret = function.apply(cont); + nbtEnt.setCompound(cont.getCompound()); + return ret; } /** @@ -141,7 +145,10 @@ public static T modify(Entity entity, Function function) { * @param consumer The consumer that will be called with the NBTEntity. */ public static void modify(Entity entity, Consumer consumer) { - consumer.accept(new NBTEntity(entity)); + NBTEntity nbtEnt = new NBTEntity(entity); + NBTContainer cont = new NBTContainer(nbtEnt.getCompound()); + consumer.accept(cont); + nbtEnt.setCompound(cont.getCompound()); } /** @@ -177,7 +184,11 @@ public static void modifyPersistentData(Entity entity, Consumer co * @return The return type is the same as the return type of the function. */ public static T modify(BlockState blockState, Function function) { - return function.apply(new NBTTileEntity(blockState)); + NBTTileEntity blockEnt = new NBTTileEntity(blockState); + NBTContainer cont = new NBTContainer(blockEnt.getCompound()); + T ret = function.apply(cont); + blockEnt.setCompound(cont); + return ret; } /** @@ -190,7 +201,10 @@ public static T modify(BlockState blockState, Function func * takes a ReadWriteNBT and does something with it. */ public static void modify(BlockState blockState, Consumer consumer) { - consumer.accept(new NBTTileEntity(blockState)); + NBTTileEntity blockEnt = new NBTTileEntity(blockState); + NBTContainer cont = new NBTContainer(blockEnt.getCompound()); + consumer.accept(cont); + blockEnt.setCompound(cont); } /**