[1.9] Can't assign class or instance variable in custom Item

[nates1984]

Suspect it's a server vs client thing. Any ideas on why this isn't working? I've tried both static and non-static variables. The code is below. When I try to use pos in the hitEntity() method I get a NullPointerException.

 

EDIT: Also worth pointing out that the code in hitEntity() only executes if posSet is true, and it's set to false in the constructor. So I know posSet is working as intended.

 

public class CustomItem extends Item {

public static BlockPos pos;
public Random rand;
public static boolean posSet;

.......

 

@SuppressWarnings({ "unchecked", "rawtypes" })
@Override
public ActionResult<ItemStack> onItemRightClick(ItemStack itemStackIn, World worldIn, EntityPlayer playerIn, EnumHand hand) {
		CustomItem.pos = playerIn.playerLocation;
		CustomItem.posSet = true;
        return new ActionResult(EnumActionResult.PASS, itemStackIn);
    }

[Leviathan143]

Remove the static modifiers and make your fields private, create a getter if you need to access a field outside it's class, inside the class you can reference them directly or with this.<field name> . I also suggest you look up what the static modifier actually does,

[Ernio]

Remove the static modifiers and make your fields private, create a getter if you need to access a field outside it's class, inside the class you can reference them directly or with this.<field name> . I also suggest you look up what the static modifier actually does,

 

While this is true, but somehow pointless for this case, it is totally WRONG when talking about Item objects.

 

Item is a SINGLETON. It cannot hold any fields that are not supposed to be shared between ALL items in-game.

To hold per-item data you need to save it to ItemStack's NBT.

 

Item - description of what you are holding (what it is and how it should act)

ItemStack - thing that you are holding, that holds data and calls methods in Item singleton.

[Leviathan143]

Item is a SINGLETON. It cannot hold any fields that are not supposed to be shared between ALL items in-game.

I am aware of that, I advised him to remove them because it was bad practice. I was not aware that you couldn't use non-static fields, update them from NBT before you use them and write their values to NBT after you changed them. I never got that approach to work, but I thought I was just doing it wrong.

 

@nates1984

I revise my advice, remove the fields entirely, instead create helper functions to retrieve the values from NBT when you need them.

[nates1984]

I want the value of pos to be shared across all items, so no reason to use PropertyInteger, NBT, etc.

 

What is it about BlockPos that makes this not work?

 

 

[coolAlias]

It's not BlockPos that makes it not work, it's the fact that Item cannot hold independent values.

 

Say that two players both are holding an ItemStack containing your Item; player A right clicks, setting CustomItem fields appropriately. Well, now player B's item ALSO contains player A's position, because Item is a singleton as Ernio pointed out. If player B were to right click, player A's item values would also change.

 

The only way you can store independent values for an Item is by using its container, i.e. ItemStack. That's why item damage is stored in the ItemStack and not the Item. Same with NBT.

 

You can store the BlockPos in the ItemStack NBT, but not in the Item as a class field.

[Ernio]

I want the value of pos to be shared across all items

 

Only logical design I see here is that you want per-player saving (each player has his value, no matter what item is used), then you need:

@Capability: http://mcforge.readthedocs.org/en/latest/datastorage/capabilities/

And most likely SimpleNetworkWrapper to sync data.

 

You might ment something else, but if that woud be the case... You are simply doing everything wrong.

[Draco18s]

Item is a SINGLETON. It cannot hold any fields that are not supposed to be shared between ALL items in-game.

I am aware of that, I advised him to remove them because it was bad practice.

 

There is absolutely nothing wrong with the static keyword.  It is not bad practice.

 

It is merely the wrong tool for the job.

 

I use static all over the place in my code.

 

public class WildlifeEventHandler {
public static boolean trackTrees;
public static boolean autoSaplings;
public static boolean doYearCycle;
public static int weekLength;
public static long yearLength;
public static boolean doSnowMelt;
public static boolean doSlowCrops;
public static boolean doBiomeCrops;
public static boolean doRawLeather;
public static boolean doNativeTreeKill;
public static int cropsWorst;
public static boolean modifyAnimalDrops;
public static int[] dimensionBlacklist;
}

 

Why?  Because every single one of those is a config option.  There is no need to store a copy per instance level (even if the class is a singleton) because I shouldn't need an instance reference in order to set or retrieve the values.  Plus, if I did have two instances, they should absolutely share the same values!

[nates1984]

EDIT: Please disregard. Looks like I overlooked the getPosition() method, which works as expected.

 

Thanks everyone for your replies.

 

It appears the root problem is with playerLocation, however.

 

Can anyone explain why:

 

System.out.println(playerIn.playerLocation.getX());

 

Throws a NullPointerException on both the client and server sides within the onItemRightClick() method?