Why not access the nms ItemStack of CraftItemStack directly and modify the stack ?!
yannicklamprecht opened this issue ยท 9 comments
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.
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.
Huh thats a good point. Maybe adding a setto method to set the data to an itemstack could fix that.
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 :).
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.
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 :/
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?
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.
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.