Help with NBT based Item creation[Reopened]

[UberAffe]

This is class structure I am using

IDraftable
Abstract DraftingItemCore extends Item implements IDraftable
MeleeCore extends DraftingItemCore

 

Here is the relevant code from MeleeCore

        public static final MeleeCore meleeCore = new MeleeCore();

public MeleeCore(String name, CoreType type, DraftableMap map) {
	partMap = map;
	coreType = type;
	this.name = name;
	setProperties();
}

private MeleeCore(){
	partMap = new DraftableMap();
	coreType = CoreType.MELEECORE;
	name = "defaultmelee";
}

@Override // this is from DraftingItemCore
public ItemStack CreateItem() {
	LogHelper.info("creating " + name); // this prints out the name I expect
	return new ItemStack(meleeCore, 1, 0, SaveAsNBT());
}
        
        @Override // this is from IDraftable
        public NBTTagCompound SaveAsNBT() {
	NBTTagCompound toReturn = new NBTTagCompound();
	toReturn.setString("name", name);
	toReturn.setString("coreType", coreType.toString());
	toReturn.setTag("partMap", partMap.SaveAsNBT());
	return toReturn;
}

 

This is my registration with minecraft which happens in CommonProxy#preInit

GameRegistry.registerItem(MeleeCore.meleeCore, Refs.MODID + "_" + MeleeCore.meleeCore.GetName());

 

I have my own registry for all available IDraftable instances which looks like this:

public final class DraftableReg {

private static HashMap<String, IDraftable> knowledgeBase = new HashMap<String, IDraftable>();

public static void RegisterDraftable(IDraftable draftable, World world){
	if(!world.isRemote)
	{
		LogHelper.info("Registering Draftable :" + draftable.GetName());
		knowledgeBase.put(draftable.GetName(), draftable);
	}
}

        public static IDraftable GetDraftable(String name){
	return knowledgeBase.containsKey(name)? knowledgeBase.get(name): null;
}
}

 

This is the code that is getting run when the player attempts to create an IDraftable

public class DraftingManager {

public static void PerformDrafting(EntityPlayer player) {
	IDrafter drafter = DraftingProvider.get(player);
	if(drafter == null) return;
	IDraftable toDraft = DraftableReg.GetDraftable(drafter.GetSelectedDraftable());
	if(toDraft == null) return;
	ItemStack held = player.getHeldItem();
	LogHelper.info("attempting draft of " + toDraft.GetName()); // this uses the name I expect
	if(held == null)
	{
		if(drafter.AttemptDrafting(toDraft.GetCost()))
		{
			ItemStack drafted = toDraft.CreateItem();
			LogHelper.info(((IDraftable)drafted.getItem()).GetName() + " Created"); // this uses defaultmelee as the name
			player.setCurrentItemOrArmor(player.inventory.currentItem, drafted);
		}
		try{
			drafter.dataChanged(player); // updates client with capability changes
		}
		catch(Exception e) {
			LogHelper.warn("Drafting errored");
			System.out.println(e.getMessage());
		}
	}
}
}

 

I am probably doing some step wrong in the creating of nbt based items but I'm missing what that is. Any help is appreciated.

[BusyBeever]

so. whats the problem?

[UberAffe]

I guess I kinda hid the problem in the comments.

This method isn't returning the "item" i am expecting

@Override // this is from DraftingItemCore
public ItemStack CreateItem() {
	LogHelper.info("creating " + name); // this prints out the name I expect
	return new ItemStack(meleeCore, 1, 0, SaveAsNBT());
}

 

The name that I print to the log for my test item is "Bluex32" and that gets printed out here but

ItemStack drafted = toDraft.CreateItem();
LogHelper.info(((IDraftable)drafted.getItem()).GetName() + " Created");

when I run this code in my draftingmanager it prints out "defaultmelee" which tells me that I am creating the default item and it isn't getting the information from the nbt I create.

[UberAffe]

So should I be using something like

meleeCore.updateItemStackNBT(SaveAsNBT());
return new ItemStack(meleeCore, 1, 0);

That seems like it would mess with all meleeCore items but maybe I'm wrong.

[UberAffe]

Alright so I was thinking too hard about this and over complicating things.

In my #CreateItem() all I needed to do was return new ItemStack(this, 1, 0) because the object this method is getting called on already has everything it needs.

 

Sorry for my dumbs and thanks for the help!

[UberAffe]

New problem, the items seem to not be getting saved/loaded between logins.

updated code

        @Override
public ItemStack CreateItem() {
	LogHelper.info("creating " + name);
	return new ItemStack(this, 1, 0);	
        }

All the other code should still be the same, and I am currently getting the correct version of the item when I call this method. Can you see any reason why this item would disappear between logout and login?

[UberAffe]

Update:

if i create an itemstack with the variable meleeCore instead of this it will persist between logout and login.

meleeCore is the instance of this class that gets registered as an item.

[UberAffe]

Doesn't minecraft handle saving the players inventory though?

 

This is how I am creating the ItemStack to give to the player

        public ItemStack CreateItem() {
	LogHelper.info("creating " + name);
	ItemStack toReturn = new ItemStack(this, 1, 0);
	NBTTagCompound tag = new NBTTagCompound();
	tag.setString(Refs.DRAFTABLE, name);
	toReturn.setTagCompound(tag);
	return toReturn;
}

[UberAffe]

IDraftable

 

public interface IDraftable {

public ItemStack CreateItem();

public HashMap<String, Float> GetCost();

public int DecayRate();

public DraftableMap GetPartArray();

public CoreType GetCoreType();

public String GetName();

public int GetPartsHigh();
public int GetPartsWide();
public int GetPartsDeep();

public NBTTagCompound SaveAsNBT();
public void LoadFrom(NBTTagCompound nbt);
}

 

But I think DraftingItemCore is the one you actually want to see

 

 

I have it as Serializable for when I save/load my DraftableReg

public abstract class DraftingItemCore extends Item implements IDraftable, Serializable{

/**
 * This number should be incremented any time a change to compatibility happens
 */
private static final long serialVersionUID = 1L;

/*
 * Draftable fields
 */
protected DraftableMap partMap;
protected CoreType coreType;
protected String name;
protected float weight;

/*
 * Item properties
 */
protected float damage;
protected int miningLevel;
protected float swingSpeed;

@Override
public DraftableMap GetPartArray() {return partMap;}

@Override
public CoreType GetCoreType() {return coreType;}

@Override
public String GetName() {return name;}

@Override
public int GetPartsHigh() {return partMap.GetHeight();}
@Override
public int GetPartsWide() {return partMap.GetWidth();}
@Override
public int GetPartsDeep() {return partMap.GetDepth();}

public void OnLeftClick(EntityPlayer player)
{
	if(player.worldObj.isRemote)
	{
		LogHelper.info("attacking from client side");
		ItemStack held = Minecraft.getMinecraft().thePlayer.getHeldItem();
		if(held != null && held.getItem() instanceof DraftingItemCore)
		{
			LogHelper.info("send attack to server");
			CommonProxy.network.sendToServer(new MessageInteractEvent());
		}
	}
}
}

 

For completeness here is MeleeCore

 

public class MeleeCore extends DraftingItemCore{

/**
 * This number should be incremented any time a change to compatibility happens
 */
private static final long serialVersionUID = 1L;

public static final MeleeCore meleeCore = new MeleeCore();

public MeleeCore(String name, CoreType type, DraftableMap map) {
	partMap = map;
	coreType = type;
	this.name = name;
	setProperties();
}

private MeleeCore(){
	partMap = new DraftableMap();
	coreType = CoreType.MELEECORE;
	name = "defaultmelee";
}

@Override
public int DecayRate() {
	// TODO calculate based on luxin used
	return 0;
}

@Override
public ItemStack CreateItem() {
	LogHelper.info("creating " + name);
	ItemStack toReturn = new ItemStack(this, 1, 0);
	NBTTagCompound tag = new NBTTagCompound();
	tag.setString(Refs.DRAFTABLE, name);
	toReturn.setTagCompound(tag);
	return toReturn;
}

@Override
public HashMap<String, Float> GetCost() {
	return partMap.GetCost();
}

@Override
public NBTTagCompound SaveAsNBT() {
	NBTTagCompound toReturn = new NBTTagCompound();
	toReturn.setString("name", name);
	toReturn.setString("coreType", coreType.toString());
	toReturn.setTag("partMap", partMap.SaveAsNBT());
	return toReturn;
}

@Override
public void LoadFrom(NBTTagCompound nbt) {
	name = nbt.getString("name");
	coreType = CoreType.valueOf(nbt.getString("coreType"));
	partMap.LoadFrom((NBTTagCompound) nbt.getTag("partMap"));
	setProperties();
}

private void setProperties(){
	switch(coreType){
	case SWORD:
		analyzeSword();
		break;
	default:
		break;
	}
}

private void analyzeSword() {
	float dam = 0.0f;
	float speed = 0.0f;
	float timing = .5f;
	//TODO analyze partMap
	float centerX = ((float)partMap.GetWidth()) / 2;
	float centerZ = ((float)partMap.GetDepth()) / 2;
	float centerY = ((float)partMap.GetHeight()) / 2;
	float weight = 0.0f;
	float xBal = 0.0f;
	float zBal = 0.0f;
	float yBal = 0.0f;
	for(int relX = 0; relX <= partMap.GetWidth(); relX++)
	{
		for(int relZ = 0; relZ <= partMap.GetDepth(); relZ++)
		{
			for(int relY = 0; relY <= partMap.GetHeight(); relY++)
			{
				IPartType part = partMap.GetPart(relX + "," + relZ + "," + relY);
				if(part != null)
				{
					float pW = part.WeightModifier();
					weight += pW;
					//Section for determining balance
					xBal += pW * (centerX - relX);
					zBal += pW * (centerZ - relZ);
					yBal += pW * (centerY - relY);
					//TODO Identify effective part and add effect
					if(part.IsEffectiveIn(CoreType.SWORD))
						dam += part.DamageModifier();
				}
			}
		}
	}
	//Apply weight affect on properties
	dam = (weight + 1) * dam;
	speed = (weight + 1) * speed;
	timing = 1 - Math.min(1, 1/(weight + 1));
	//apply balance affect on properties
	float xAB = Math.abs(xBal);
	float zAB = Math.abs(zBal);
	float yAB =  Math.abs(yBal);
	if(xBal > 0) // front balance
		timing = Math.min(1, timing * (1 / (xAB + 1)));
	else if(xBal < 0) // back balance
		timing = Math.min(1, timing * (2 - (1 / (xAB + 1))));
	if(zBal > 0) // left balance
		;//TODO what happens in this case?
	else if(zBal < 0) // right balance
		;//TODO what happens in this case?
	if(yBal > 0) // bottom balance
	{
		dam = dam * (1-(1/(yAB + 1)));
		speed = speed * (1-(1/(yAB + 1)));
	}
	else if(yBal < 0) // top balance
	{
		dam = dam * (1+(1/(yAB + 1)));
		speed = speed * (1+(1/(yAB + 1)));
	}
	damage = dam;
	miningLevel = 0;
	swingSpeed = speed;
	LogHelper.info("damage for " + name + " is " + damage);
}

/*
 * TODO set all overriden methods of item
 */
@Override
public void onUpdate(ItemStack stack, World worldIn, Entity entityIn, int itemSlot, boolean isSelected)
    {
	//TODO useful for decay, possibly other things too.
    }

    @Override
    public void onCreated(ItemStack stack, World worldIn, EntityPlayer playerIn)
    {
    	//TODO check when this gets called, might be useless for my purposes
    }
    
    @Override
    public EnumAction getItemUseAction(ItemStack stack)
    {
    	//TODO check if this is the basic swing animation
        return EnumAction.NONE;
    }
    
    @Override
    public int getMaxItemUseDuration(ItemStack stack)
    {
    	//TODO un-needed for MeleeCores?
        return 0;
    }
    
    @Override
    @SideOnly(Side.CLIENT)
    public void addInformation(ItemStack stack, EntityPlayer playerIn, List<String> tooltip, boolean advanced)
    {
    	//TODO add cost info, add time left(decay rate)
    }
    
    @Override
    /*
     * All MeleeCore's are tools
     */
    public boolean isItemTool(ItemStack stack)
    {
        return true;
    }
    
    @Override
    public boolean onLeftClickEntity(ItemStack stack, EntityPlayer player, Entity entity)
    {
    	//TODO set interaction for different CoreTypes
        return true;
    }
    
    @Override
    public int getEntityLifespan(ItemStack itemStack, World world)
    {
    	//TODO base on decayrate?
        return 6000;
    }

@Override
public void OnLeftClick(EntityPlayer player) {
	super.OnLeftClick(player);
	if(!player.worldObj.isRemote)
	{
		LogHelper.info("left click on server");
		ItemStack held = player.getHeldItem();
		if(held != null && held.getItem() instanceof MeleeCore)
		{
			LogHelper.info("left click with MeleeCore");
			MeleeCore item = (MeleeCore)held.getItem();
			int range = item.partMap.GetHeight() + 16;
			BlockPos pPos = player.getPosition();
			Vec3 look = player.getLookVec();
			BlockPos tPos = pPos.add(new Vec3i(pPos.getX() + look.xCoord*range, pPos.getY()+ (look.yCoord*range), pPos.getZ() + (look.zCoord*range)));
			List<Entity> targets = player.worldObj.getEntitiesWithinAABBExcludingEntity(player, new AxisAlignedBB(pPos, tPos));
			for(Entity ent : targets)
			{
				LogHelper.info(ent.getName() + " was attacked by " + player.getName() + " using " + item.name + " for " + item.damage + " damage.");
				ent.attackEntityFrom(DamageSource.causePlayerDamage(player), item.damage);
			}
		}
	}
}
}

 

[UberAffe]

Maybe I don't need it, I still get confused with minecrafts single item instance for multiple ItemStack thing. So let me restate what I am trying to accomplish.

 

I am trying to allow players to design new Items during the game. Any player with a sufficiently high level will be able to create an ItemStack that uses whichever Item they selected. Each Item that gets designed will be a variant of one of the cores, the only core available right now is MeleeCore which is used for items that act like swords. Variants of Cores have the same functionality but use variant specific models/damage/durability values.

 

What is working with my current code:

In-game creation of new core variants

Save/Load of core variants

In-game creation of ItemStack based on specific variant (has correct damage values)

 

When I create an ItemStack using the default core it stays in the players inventory between logout and login. When I create an ItemStack using a variant of the core it gets lost between logout and login.

 

I believe this means that I will need to switch things up so that I always use the default core but with different NBTs that have all the information that would otherwise have been in the variant core.

[UberAffe]

Hurray for mass refactoring! Thanks for the help!

[Ernio]

Item - description of thing you are holding.

ItemStack - thing you are holding.

 

1. Item simply CAN'T hold any data. Everything item can hold must be kept in ItemStack's NBT/damage/size.

 

2. You should NEVER, ever make "new" instance of Item outside init phase. That is simply wrong and will turn back on you in near future.

 

3. How I'd do it:

interface IDraftable<T extends Item> which can be applied by you/users of API to make "special" Item singletons that  can be handled by your system.

Start implementing this interface to your base Item instances, e.g: ItemMeele.

This Item instance will serve as something that can act upon data held in ItemStack's using this Item, by that I mean:

- You hold ALL data in ItemStack which uses ItemMeele as definition, then in your ItemMeele you e.g: use Item#hitEntity and for-loop through list saved inside ItemStack's NBT looking for "special effects" - say - key "ignite" will mean that hit entity should be set on fire.

This example is obviously super simple.

 

As to rendering - you implement some (versions change a lot) IBakedModel that will take ItemStack and check its contends for proper NBT keys.

There are 2 ways to go about it:

1.

Define universal per-ItemStack format - meaning your IBakedModel will be able to handle any IDraftable:

- Say you have: IDraftable#getAllQuads(ItemStack)) that will be called by IBakedModel to return all cubicals that should be rendered as Item.

Those cubicals can be saved/read directly into/from ItemStack's NBT - so every stack can be different.

2.

Also make this kind of format BUT. instead of making IDraftable#getAllQuads(ItemStack)), you can make IDraftable#getDraftableName(ItemStack)), meaning you will be returning String saved in ItemStack that points at NAME of alredy-generated model that can be saved in server and synced to clients on demand.

 

Both ways have + and -: if you are saving every model into ItemStack it might get heavy on syncing, BUT - it will be always synced and saved and overally awesome. on the other hand if you would only make ItemStack save names that point at some registry of your alredy generated models you are light on packeting and data-saving,  BUT you have to handle saving of models and syncing on your own (meaning you have to send model to clients whenever they connect to server and save them on server using WorldSavedData).

 

Idk, how much of this "design" example you understand, but above is most flexible to go about it.

 

Note: A lot of times I see draco posting links to his Artifacts mod and from what I know it does a lot around customization of ItemStacks, might be worth looking at.

 

[Draco18s]

Oh.

 

I figured out the problem. (Ernio's post is more elaborate, but I had to go deal with surprise baby bunnies and was interrupted in posting)

 

public abstract class DraftingItemCore extends Item implements IDraftable, Serializable{
protected DraftableMap partMap;
protected CoreType coreType;
protected String name;
protected float weight;

 

You have item properties that are part of the singleton.  These aren't being stored in the NBT.

 

$10 says that if you create two different, non-default swords they will be identical.

[UberAffe]

Actually they weren't I was able to have multiple different MeleeCore variants and they would each use the correct damage values. Although I might have realized the problem sooner if what you said had been the case.

[UberAffe]

I ended up updating to 1.9 but I am getting an NPE on NBTTagCompound.copy and I believe it is happening when I give the player the ItemStack created here:

         public ItemStack CreateItem() {
	LogHelper.info("creating " + name);
	ItemStack toReturn = new ItemStack(meleeCore, 1, 0);
	toReturn.setTagInfo(Refs.MODID, SaveAsNBT());
	return toReturn;
}

I know that SaveAsNBT is returning a valid-non-NULL NBT because I use in other places with this object.

 

This is how I am giving the player the ItemStack

ItemStack drafted = toDraft.CreateItem();
LogHelper.info(((IDraftable)drafted.getItem()).GetName() + " Created");
player.setHeldItem(EnumHand.MAIN_HAND, drafted);

This only gets run serverside and there are checks to make sure that nothing is null.

 

here is the full crash

 

[16:03:52] [server thread/ERROR]: Encountered an unexpected exception
net.minecraft.util.ReportedException: Ticking player
at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:785) ~[MinecraftServer.class:?]
at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:683) ~[MinecraftServer.class:?]
at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:155) ~[integratedServer.class:?]
at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:532) [MinecraftServer.class:?]
at java.lang.Thread.run(Unknown Source) [?:1.8.0_45]
Caused by: java.lang.NullPointerException
at net.minecraft.nbt.NBTTagCompound.copy(NBTTagCompound.java:526) ~[NBTTagCompound.class:?]
at net.minecraft.nbt.NBTTagCompound.copy(NBTTagCompound.java:526) ~[NBTTagCompound.class:?]
at net.minecraft.nbt.NBTTagCompound.copy(NBTTagCompound.java:526) ~[NBTTagCompound.class:?]
at net.minecraft.item.ItemStack.copy(ItemStack.java:419) ~[itemStack.class:?]
at net.minecraft.inventory.Container.detectAndSendChanges(Container.java:89) ~[Container.class:?]
at net.minecraft.entity.player.EntityPlayerMP.onUpdate(EntityPlayerMP.java:290) ~[EntityPlayerMP.class:?]
at net.minecraft.world.World.updateEntityWithOptionalForce(World.java:2086) ~[World.class:?]
at net.minecraft.world.WorldServer.updateEntityWithOptionalForce(WorldServer.java:864) ~[WorldServer.class:?]
at net.minecraft.world.World.updateEntity(World.java:2051) ~[World.class:?]
at net.minecraft.world.WorldServer.tickPlayers(WorldServer.java:666) ~[WorldServer.class:?]
at net.minecraft.world.World.updateEntities(World.java:1858) ~[World.class:?]
at net.minecraft.world.WorldServer.updateEntities(WorldServer.java:637) ~[WorldServer.class:?]
at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:779) ~[MinecraftServer.class:?]
... 4 more

 

 

My guess is that something is wrong with how I am creating the ItemStack.

 

Update:

If I remove the setTagInfo line the crash goes away. Am I doing something wrong with how I am adding nbt information?

[Ernio]

You have:

toReturn.setTagInfo(Refs.MODID, SaveAsNBT());

Then:

@Override
public NBTTagCompound SaveAsNBT() {
	NBTTagCompound toReturn = new NBTTagCompound();
	toReturn.setString("name", name);
	toReturn.setString("coreType", coreType.toString());
	toReturn.setTag("partMap", partMap.SaveAsNBT());
	return toReturn;
}

What is this?

	toReturn.setTag("partMap", partMap.SaveAsNBT());

 

EDIT

 

Btw. WHY ON EARTH are you NOT using Java conventions? jesus christ...

Also: Consider using INBTSerializable<T>

[UberAffe]

It is from my DraftableMap class which is basically just a wrapper for a HashMap

         public NBTTagCompound SaveAsNBT() {
	NBTTagCompound toReturn = new NBTTagCompound();
	for(Entry<String, IPartType>  set : map.entrySet())
		toReturn.setTag(set.getKey(), set.getValue().SaveAsNBT());
	return toReturn;
}

Diving down the Rabit hole for IPartType#SaveAsNBT

         public NBTTagCompound SaveAsNBT() {
	NBTTagCompound toReturn = new NBTTagCompound();
	toReturn.setString("purity", purity.toString());
	toReturn.setString("luxin", luxin.GetName());
	toReturn.setString("partType", PartType.EDGE.toString());
	return toReturn;
}

 

... Well first I'm dumb. My IPartType implementation was return null instead of toReturn and the error is no longer happening ... but that raises a new question of why wasn't this making other things crash ...