NBT-API

NBT-API

98.9k Downloads

Why not access the nms ItemStack of CraftItemStack directly and modify the stack ?!

yannicklamprecht opened this issue ยท 9 comments

commented

https://github.com/tr7zw/Item-NBT-API/blob/master/src/main/java/de/tr7zw/itemnbtapi/NBTReflectionUtil.java#L143-L154

Currently you're invoking the method that delivers a copy of the nms ItemStack. Why not accessing the field directly?

private static Field nmsHandle;

    static {
        try {
            nmsHandle = CraftItemStack.class.getDeclaredField("handle");
            nmsHandle.setAccessible(true);
        } catch (NoSuchFieldException e) {
            e.printStackTrace();
        }
    }

private net.minecraft.server.v1_12_R1.ItemStack handle;

    private ItemStackNBTTagManager nbtTagManager;

    public CSItemstack(org.bukkit.inventory.ItemStack itemStack) {
        try {
            this.handle = (net.minecraft.server.v1_12_R1.ItemStack) nmsHandle.get(itemStack);
        } catch (IllegalAccessException e) {
            e.printStackTrace();
        }
        this.nbtTagManager = new ItemStackNBTTagManager(this.handle);
    }

This is currently implemented version dependent.

commented

ok, I done it your way in the past but a disadvantage is that you've to set the new itemStack into the inventory each time you want to modify it.

eg. saving ammo count that is changing each shot. I't takes long to getItemStack -> modify it -> set it in inventory -> update inventory. That caused some updating and displaying bugs.

commented

Huh thats a good point. Maybe adding a setto method to set the data to an itemstack could fix that.

commented

Huh, I think back then I thought it was better to implement it like that, to work with a copy of the Item stack. But since the Tiles and Entities are made to work with the given Tile/Entity, changing Items to work the same way sounds good. I will change that late :).

commented

Ok, I took a look at it: This creates more problems than it's worth. "This is currently implemented version dependent." booth ways are version dependent^^. One uses the nms-api to get the NMS ItemStack, the other one just grabs it from where the getter would get it. So no, not going to change that right now.

commented

Ok problem: ItemStacks may be not an Instance of CraftItemStack, so getting the handle doesn't work. I could make it ignore the save in that case. Not sure how to handle it the best way :/

commented
package org.bukkit.craftbukkit.v1_12_R1.inventory;

import org.bukkit.inventory.*;
import org.bukkit.configuration.serialization.*;
import org.bukkit.craftbukkit.v1_12_R1.util.*;
import org.bukkit.*;
import org.bukkit.inventory.meta.*;
import org.bukkit.material.*;
import org.bukkit.enchantments.*;
import org.apache.commons.lang.*;
import org.bukkit.craftbukkit.v1_12_R1.enchantments.*;
import net.minecraft.server.v1_12_R1.*;
import java.util.*;
import com.google.common.collect.*;

@DelegateDeserialization(ItemStack.class)
public final class CraftItemStack extends ItemStack
{
    net.minecraft.server.v1_12_R1.ItemStack handle;
.
.
.

I found it there. Do you mean that this field isn't available in other forks of spigot?

commented

No, as you can see CraftItemStack extends ItemStack. If you just create ItemStack item = new ItemStack(Material.Stone, 1);, this won't be an instance of CraftItemStack. As far as I can tell, CraftItemStacks get generated internally, when you pass "normal" ItemStacks.

commented

You are correct. I used to do this manually myself, back before i discovered the nbtapi. After reading the native spigot code, I found that reflection to native methods was the only way.

commented

Also sry for just letting this issue "sit" here, since I'm not too sure about what to do with it. But I guess the way it currently is should be fine for most people.