[1.11.2] ItemStack Capabilities

[TLHPoE]

Goal: Create a cool down system for firing a weapon, so getting and setting an integer within an itemstack

How: Using forge capability system

Problem: Can't set (but can retrieve capability)

 

IRepeaterData:

package com.kain.slippworld.data.repeater;

public interface IRepeaterData {
	public int getCoolDown();

	public void setCoolDown(int coolDown);
}

 

RepeaterData:

package com.kain.slippworld.data.repeater;

public class RepeaterData implements IRepeaterData {
	private int coolDown;
	
	@Override
	public int getCoolDown() {
		return coolDown;
	}

	@Override
	public void setCoolDown(int coolDown) {
		this.coolDown = coolDown;
	}

}

 

RepeaterCapability:

package com.kain.slippworld.data.repeater;

import com.kain.slippworld.*;
import com.kain.slippworld.data.*;

import net.minecraft.item.*;
import net.minecraft.nbt.*;
import net.minecraft.util.*;
import net.minecraftforge.common.capabilities.*;

public class RepeaterCapability {
	public static final ResourceLocation NAME = new ResourceLocation(Reference.MOD_ID, "_Repeater");

	@CapabilityInject(IRepeaterData.class)
	public static final Capability<IRepeaterData> REPEATER_DATA_CAPABILITY = null;

	public static final EnumFacing DEFAULT_FACING = null;

	public static void register() {
		CapabilityManager.INSTANCE.register(IRepeaterData.class, new Capability.IStorage<IRepeaterData>() {
			@Override
			public NBTBase writeNBT(Capability<IRepeaterData> capability, IRepeaterData instance, EnumFacing side) {
				return new NBTTagInt(instance.getCoolDown());
			}

			@Override
			public void readNBT(Capability<IRepeaterData> capability, IRepeaterData instance, EnumFacing side, NBTBase nbt) {
				instance.setCoolDown(((NBTTagInt) nbt).getInt());
			}
		}, () -> new RepeaterData());
	}

	public static IRepeaterData getCapability(ItemStack itemStack) {
		return itemStack != null && itemStack.hasCapability(REPEATER_DATA_CAPABILITY, DEFAULT_FACING) ? itemStack.getCapability(REPEATER_DATA_CAPABILITY, DEFAULT_FACING) : null;
	}

	public static ICapabilityProvider createProvider() {
		return new GenericCapabilityProvider<>(REPEATER_DATA_CAPABILITY, DEFAULT_FACING);
	}

	public static ICapabilityProvider createProvider(IRepeaterData repeaterData) {
		return new GenericCapabilityProvider(REPEATER_DATA_CAPABILITY, DEFAULT_FACING, repeaterData);
	}
}

 

GenericCapabilityProvider:

package com.kain.slippworld.data;

import net.minecraft.nbt.*;
import net.minecraft.util.*;
import net.minecraftforge.common.capabilities.*;

public class GenericCapabilityProvider<HANDLER> implements ICapabilitySerializable<NBTBase> {
	private final Capability<HANDLER> capability;

	private final EnumFacing facing;

	private final HANDLER instance;

	public GenericCapabilityProvider(Capability<HANDLER> capability, EnumFacing facing) {
		this(capability, facing, capability.getDefaultInstance());
	}

	public GenericCapabilityProvider(Capability<HANDLER> capability, EnumFacing facing, HANDLER instance) {
		this.capability = capability;
		this.facing = facing;
		this.instance = instance;
	}

	@Override
	public boolean hasCapability(Capability<?> capability, EnumFacing facing) {
		return capability == getCapability();
	}

	@Override
	public <T> T getCapability(Capability<T> capability, EnumFacing facing) {
		if(capability == getCapability()) {
			return (T) instance;
		}
		return null;
	}

	@Override
	public NBTBase serializeNBT() {
		return getCapability().writeNBT(getInstance(), getFacing());
	}

	@Override
	public void deserializeNBT(NBTBase nbt) {
		getCapability().readNBT(getInstance(), getFacing(), nbt);
	}

	public Capability<HANDLER> getCapability() {
		return capability;
	}

	public EnumFacing getFacing() {
		return facing;
	}

	public HANDLER getInstance() {
		return instance;
	}
}

 

ItemRepeater

package com.kain.slippworld.item.tool;

import javax.annotation.*;

import com.kain.slippworld.*;
import com.kain.slippworld.data.repeater.*;

import net.minecraft.creativetab.*;
import net.minecraft.entity.*;
import net.minecraft.entity.player.*;
import net.minecraft.item.*;
import net.minecraft.nbt.*;
import net.minecraft.util.*;
import net.minecraft.world.*;
import net.minecraftforge.common.capabilities.*;
import net.minecraftforge.fml.relauncher.*;

public class ItemRepeater extends Item {
	public final ToolMaterial material;
	protected final float damage;
	protected final int maxCoolDown;

	public ItemRepeater(ToolMaterial material, float distance, int maxCoolDown) {
		super();

		...
	}

	@Override
	public void onUpdate(ItemStack itemStack, World world, Entity e, int slot, boolean selected) {
		super.onUpdate(itemStack, world, e, slot, selected);

		if(!world.isRemote) {
			IRepeaterData data = RepeaterCapability.getCapability(itemStack);

			if(data != null) {
				int coolDown = data.getCoolDown();

				if(coolDown > 0) {
					System.out.println("COOL DOWN");
					data.setCoolDown(coolDown--);
				}
			}
		}
	}

	...

	@Override
	@Nullable
	public ICapabilityProvider initCapabilities(ItemStack stack, @Nullable NBTTagCompound nbt) {
		return RepeaterCapability.createProvider();
	}

	public void attackEntity(ItemStack itemStack, EntityLivingBase entity, EntityPlayer player) {
		if(!player.world.isRemote) {
			IRepeaterData data = RepeaterCapability.getCapability(itemStack);

			if(data != null && data.getCoolDown() == 0) {
				entity.hurtResistantTime = 0;
				entity.attackEntityFrom(DamageSource.GENERIC, getDamage());

				data.setCoolDown(100);

				System.out.println("BANG");
			}
		}
	}

	public float getDamage() {
		return damage;
	}

	public int getMaxCoolDown() {
		return maxCoolDown;
	}
}

 

ItemRepeater#attackEntity is being called externally by a packet. Right now when I right click (trigger the attackEntity method), it detects the cool down value as being 0 and outputs "BANG," but it never sets the cool down value to 100. I'm sure that there's some markDirty method somewhere but I just can't find it.

 

I based my code around choonster's test mod.

Edited April 30, 2017 by TLHPoE
Forgot a class

[Choonster]

11 minutes ago, TLHPoE said:

ItemRepeater#attackEntity is being called externally by a packet. Right now when I right click (trigger the attackEntity method), it detects the cool down value as being 0 and outputs "BANG," but it never sets the cool down value to 100.

 

What makes you think the cooldown isn't being set to 100? Are you looking at data from the client or from the server?

 

Capabilities aren't automatically synced between the server and client, you need to sync them yourself. I explain how to sync item capabilities here.

Edited April 30, 2017 by Choonster

[TLHPoE]

4 hours ago, Choonster said:

 

What makes you think the cooldown isn't being set to 100? Are you looking at data from the client or from the server?

 

Capabilities aren't automatically synced between the server and client, you need to sync them yourself. I explain how to sync item capabilities here.

 

Both times I access the capabilities are server side.

[Jay Avery]

So what exactly tells you that the cool down is not being set to 100?

[TLHPoE]

2 hours ago, Jay Avery said:

So what exactly tells you that the cool down is not being set to 100?

 

This check:

if(data != null && data.getCoolDown() == 0) {

 

This passes every time, and inside the clause it sets the cool down to 100.

[Choonster]

I can't see any obvious issues with your code, could you create a Git repository for it (if you don't already have one) and link it here? I'll try debugging it myself.

[TLHPoE]

18 hours ago, Choonster said:

I can't see any obvious issues with your code, could you create a Git repository for it (if you don't already have one) and link it here? I'll try debugging it myself.

https://github.com/phan-kavin/SlippWorld

 

Forge version 1.11.2-13.20.0.2228

Edited May 2, 2017 by TLHPoE

[Choonster]

In future, please create the Git repository in the root directory of your mod (where build.gradle is) and include the Gradle buildscript (build.gradle and gradle.properties), the Gradle wrapper (gradlew, gradlew.bat and the gradle directory), the source code and assets (the src directory) and your mod's license and readme files. See my TestMod3 for an example of the proper structure.

 

I'll start debugging it soon.

[Choonster]

MessageTargetEntity is sending the client-side Repeater ItemStack to the server and MessageTargetEntityHandler is passing the copy of this it read from the byte buffer as an argument to ItemRepeater#attackEntity. This copy's IRepeaterData has its cooldown modified, but this cooldown is never used again and the object is garbage collected. The copy in the server-side player's inventory remains unmodified.

 

Instead of sending an ItemStack in the packet, use the ItemStack already in the player's inventory.

 

Another issue I encountered was this code in ItemRepeater#onUpdate:

int coolDown = data.getCoolDown();

if(coolDown > 0) {
	System.out.println("COOL DOWN");
	data.setCoolDown(coolDown--);
}

 

Because you're using the post-decrement operator, this calls IRepeaterData#setCoolDown with the existing value of the coolDown local variable (setting RepeaterData#coolDown to the same value it already had) and then decrements the coolDown local variable (which is never used again). If you use the pre-decrement operator instead, the cooldown will be decremented as intended.

 

There's no reason to call Object#equals on an ItemStack, ItemStack doesn't override it so it's the same as using the == operator. If you want to compare two ItemStacks by value instead of reference, use the static equality methods in the ItemStack class.

 

Object#finalize shouldn't be called explicitly, it should only be called by the JVM. MessageTargetEntityHandler calls it, even though it doesn't override it and as such will do absolutely nothing.

 

MessageTargetEntityHandler calls ItemRepeater#attackEntity without actually checking if the targeted entity is in range of the player. A malicious client could currently send this packet to attack any entity in the same dimension as the player, regardless of the distance between them. The server should never trust the client.

 

The way the Repeater sets itself as the active item while it's held is a bit hacky and doesn't take into account the off hand. I'd recommend removing this and sending the EnumHand holding the Repeater in MessageTargetEntityHandler instead of using EntityLivingBase#getActiveItemStack.

 

I've fixed the first four issues and pushed the changes to a fork of your repository. You can view or merge the changes here.

Edited May 3, 2017 by Choonster