[1.11.2] [Unsolved] Accessing an entity's gui container

[OrangeVillager61]

Hmmm, the inventory still destroys items even in newly spawned villagers. I'll update the github if you wish to see there and I'll post the affected code below.

Container:

public class ContainerIvVillagerHireNitwit extends Container{

	private IvVillager villager;
	private IItemHandler handler;
	
	public ContainerIvVillagerHireNitwit(IvVillager villager, IInventory playerInv){
		this.villager = villager;
		this.handler = this.villager.getCapability(CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, null);

		this.addSlotToContainer(new SlotItemHandler(handler, 0, 76, 47));
		int xPos = 8;
		int yPos = 84;
		
		for (int y = 0; y < 3; ++y) {
			for (int x = 0; x < 9; ++x) {
				this.addSlotToContainer(new Slot(playerInv, x + y * 9 + 9, xPos + x * 18, yPos + y * 18));
			}
		}
				
		for (int x = 0; x < 9; ++x) {
			this.addSlotToContainer(new Slot(playerInv, x, xPos + x * 18, yPos + 58));
		}
	}

New Item Handler Code:

    private ItemStackHandler item_handler;
@Override
	public boolean hasCapability(net.minecraftforge.common.capabilities.Capability<?> capability, @Nullable net.minecraft.util.EnumFacing facing)	{
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY)
		{
			return true;
		}
		else
		{
			return super.hasCapability(capability, facing);
		}
	}
    @Override
    @Nullable
    public <T> T getCapability(Capability<T> capability, @Nullable EnumFacing facing)
    {
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY.cast(this.item_handler))
		{
			return (T) this.item_handler;
		}
		else
		{
			return super.getCapability(capability, facing);
		}
    }

 

[Choonster]

Your getCapability method is wrong. Please read my instructions carefully, I've told you exactly what you need to do:

 

10 hours ago, Choonster said:

Instead of casting this.item_handler directly to T and suppressing the unchecked warnings resulting from it, call CapabilityItemHandler.ITEM_HANDLER_CAPABILITY.cast with this.item_handler as the argument.

 

I didn't tell you to change the if statement, I told you to change the cast of this.item_handler.

 

Edited May 31, 2017 by Choonster

[OrangeVillager61]

10 hours ago, Choonster said:

Your getCapability method is wrong. Please read my instructions carefully, I've told you exactly what you need to do:

 

 

I didn't tell you to change the if statement, I told you to change the cast of this.item_handler.

 

Alright, so is the below be what you suggested?

	@Override
	public boolean hasCapability(Capability<?> capability, @Nullable EnumFacing facing)	{
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY)
		{
			return true;
		}
		else
		{
			return super.hasCapability(capability, facing);
		}
	}
    @Override
    @Nullable
    public <T> T getCapability(Capability<T> capability, @Nullable EnumFacing facing)
    {
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY)
		{
			return this.item_handler.cast(this.item_handler);
		}
		else
		{
			return super.getCapability(capability, facing);
		}
    }

 

Edited May 31, 2017 by OrangeVillager61

[Jay Avery]

No, you are trying to call cast on your item_handler. The method is called from CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, with your item_handler as the argument.

[OrangeVillager61]

	@Override
	public boolean hasCapability(net.minecraftforge.common.capabilities.Capability<?> capability, @Nullable net.minecraft.util.EnumFacing facing)	{
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY)
		{
			return true;
		}
		else
		{
			return super.hasCapability(capability, facing);
		}
	}
    @Override
    @Nullable
    public <T> T getCapability(Capability<T> capability, @Nullable EnumFacing facing)
    {
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY.cast(this.item_handler))
		{
			return (T) this.item_handler;
		}
		else
		{
			return super.getCapability(capability, facing);
		}
    }

So then how is this wrong then?

[Jay Avery]

Like choonster said, you need to cast it in the return statement instead of using (T), not change the if statement.

[OrangeVillager61]

12 minutes ago, Jay Avery said:

Like choonster said, you need to cast it in the return statement instead of using (T), not change the if statement.

Ah, okay, I think you mean this then?

@Override
    @Nullable
    public <T> T getCapability(Capability<T> capability, @Nullable EnumFacing facing)
    {
		if (capability == CapabilityItemHandler.ITEM_HANDLER_CAPABILITY)
		{
			return CapabilityItemHandler.ITEM_HANDLER_CAPABILITY.cast(this.item_handler);
		}
		else
		{
			return super.getCapability(capability, facing);
		}
    }

 

[Jay Avery]

Yep, that's right.

[OrangeVillager61]

I get a null pointerexception when I activate the GUI

java.util.concurrent.ExecutionException: java.lang.NullPointerException
	at java.util.concurrent.FutureTask.report(Unknown Source) ~[?:1.8.0_121]
	at java.util.concurrent.FutureTask.get(Unknown Source) ~[?:1.8.0_121]
	at net.minecraft.util.Util.runTask(Util.java:30) [Util.class:?]
	at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:754) [MinecraftServer.class:?]
	at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:699) [MinecraftServer.class:?]
	at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:156) [IntegratedServer.class:?]
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:548) [MinecraftServer.class:?]
	at java.lang.Thread.run(Unknown Source) [?:1.8.0_121]
Caused by: java.lang.NullPointerException
	at net.minecraftforge.items.SlotItemHandler.getStack(SlotItemHandler.java:79) ~[SlotItemHandler.class:?]
	at net.minecraft.inventory.Container.getInventory(Container.java:71) ~[Container.class:?]
	at net.minecraft.inventory.Container.addListener(Container.java:57) ~[Container.class:?]
	at net.minecraftforge.fml.common.network.internal.FMLNetworkHandler.openGui(FMLNetworkHandler.java:102) ~[FMLNetworkHandler.class:?]
	at net.minecraft.entity.player.EntityPlayer.openGui(EntityPlayer.java:2744) ~[EntityPlayer.class:?]
	at orangeVillager61.ImprovedVillagers.Entities.IvVillager.processInteract(IvVillager.java:699) ~[IvVillager.class:?]
	at net.minecraft.entity.EntityLiving.processInitialInteract(EntityLiving.java:1338) ~[EntityLiving.class:?]
	at net.minecraft.entity.player.EntityPlayer.interactOn(EntityPlayer.java:1280) ~[EntityPlayer.class:?]
	at net.minecraft.network.NetHandlerPlayServer.processUseEntity(NetHandlerPlayServer.java:1067) ~[NetHandlerPlayServer.class:?]
	at net.minecraft.network.play.client.CPacketUseEntity.processPacket(CPacketUseEntity.java:94) ~[CPacketUseEntity.class:?]
	at net.minecraft.network.play.client.CPacketUseEntity.processPacket(CPacketUseEntity.java:15) ~[CPacketUseEntity.class:?]
	at net.minecraft.network.PacketThreadUtil$1.run(PacketThreadUtil.java:21) ~[PacketThreadUtil$1.class:?]
	at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source) ~[?:1.8.0_121]
	at java.util.concurrent.FutureTask.run(Unknown Source) ~[?:1.8.0_121]
	at net.minecraft.util.Util.runTask(Util.java:29) ~[Util.class:?]
	... 5 more
[17:40:07] [Server thread/ERROR]: Encountered an unexpected exception
net.minecraft.util.ReportedException: Ticking player
	at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:801) ~[MinecraftServer.class:?]
	at net.minecraft.server.MinecraftServer.tick(MinecraftServer.java:699) ~[MinecraftServer.class:?]
	at net.minecraft.server.integrated.IntegratedServer.tick(IntegratedServer.java:156) ~[IntegratedServer.class:?]
	at net.minecraft.server.MinecraftServer.run(MinecraftServer.java:548) [MinecraftServer.class:?]
	at java.lang.Thread.run(Unknown Source) [?:1.8.0_121]
Caused by: java.lang.NullPointerException
	at net.minecraftforge.items.SlotItemHandler.getStack(SlotItemHandler.java:79) ~[SlotItemHandler.class:?]
	at net.minecraft.inventory.Container.detectAndSendChanges(Container.java:93) ~[Container.class:?]
	at net.minecraft.entity.player.EntityPlayerMP.onUpdate(EntityPlayerMP.java:319) ~[EntityPlayerMP.class:?]
	at net.minecraft.world.World.updateEntityWithOptionalForce(World.java:2126) ~[World.class:?]
	at net.minecraft.world.WorldServer.updateEntityWithOptionalForce(WorldServer.java:883) ~[WorldServer.class:?]
	at net.minecraft.world.World.updateEntity(World.java:2092) ~[World.class:?]
	at net.minecraft.world.WorldServer.tickPlayers(WorldServer.java:680) ~[WorldServer.class:?]
	at net.minecraft.world.World.updateEntities(World.java:1879) ~[World.class:?]
	at net.minecraft.world.WorldServer.updateEntities(WorldServer.java:651) ~[WorldServer.class:?]
	at net.minecraft.server.MinecraftServer.updateTimeLightAndEntities(MinecraftServer.java:795) ~[MinecraftServer.class:?]

I have updated my github to reflect the recent changes.

[Jay Avery]

You never seem to initialise your item handler.

[OrangeVillager61]

47 minutes ago, Jay Avery said:

You never seem to initialise your item handler.

Ah, okay, what do I initialize it with?

 

Edit - I used ItemStackHandler from Choonster's suggestions.

Edited May 31, 2017 by OrangeVillager61

[OrangeVillager61]

The items now stay! But the button that I used doesn't work when I press it nor render correctly. I get no console errors from this.

GUI Code:

public class GuiIvVillagerHireNitwit extends GuiContainer{

	private IvVillager villager;
	private IInventory playerInv;
	private EntityPlayer player;
	
	public GuiIvVillagerHireNitwit(IvVillager villager, IInventory playerInv, EntityPlayer player) {
		super(new ContainerIvVillagerHireNitwit(villager, playerInv));
		
		this.xSize = 176;
		this.ySize = 166;
		this.player = player;
		
		this.villager = villager;
		this.playerInv = playerInv;
	}

	@Override
	protected void drawGuiContainerBackgroundLayer(float partialTicks, int mouseX, int mouseY) {
		GlStateManager.color(1.0F, 1.0F, 1.0F, 1.0F);
		this.mc.getTextureManager().bindTexture(new ResourceLocation(Reference.MOD_ID, "gui/hire_nitwit.png"));
		this.drawTexturedModalRect(this.getGuiLeft(), this.getGuiTop(), 0, 0, this.xSize, this.ySize);
	}
	
	@Override
	protected void drawGuiContainerForegroundLayer(int mouseX, int mouseY)
    {
        String s = this.villager.getName();
        int i = 0;
        Boolean has_emeralds;
        this.mc.fontRenderer.drawString(String.valueOf(villager.getHireCost()), 46, 50, 4210752);
        this.mc.fontRenderer.drawString(s, this.xSize / 2 - this.mc.fontRenderer.getStringWidth(s) / 2, 6, 4210752);
        this.mc.fontRenderer.drawString(this.playerInv.getDisplayName().getFormattedText(), 8, 72, 4210752);
        if (this.inventorySlots.getSlot(0).getStack().getCount() >= villager.getHireCost() && this.inventorySlots.getSlot(0).getStack().getItem().equals(Items.EMERALD)){ //this is where the container needs to be accessed
        	has_emeralds = true;
        	if (this.inventorySlots.getSlot(0).getStack().getCount() > villager.getHireCost())
        	{
        		i = this.inventorySlots.getSlot(0).getStack().getCount() - villager.getHireCost();
        	}
        }
        else 
        {
        	has_emeralds = false;
        }
        this.addButton(new Button_Hire(0, this.getGuiLeft() + 115, this.getGuiTop() + 20, 50, 25, "Hire", this.villager, has_emeralds, this.player, i));
    }

}

Button_Hire:

public class Button_Hire extends GuiButton{
	
	public IvVillager villager;
	
	
	public Button_Hire(int buttonId, int x, int y, int widthIn, int heightIn, String buttonText, IvVillager villager, boolean has_emeralds, EntityPlayer player, int remaining_i) {
		super(buttonId, x, y, widthIn, heightIn, buttonText);
		
		PointerInfo a = MouseInfo.getPointerInfo();
		Point b = a.getLocation();
		int mouseX = (int) b.getX();
		int mouseY = (int) b.getY();
		if (this.mousePressed(mouseX, mouseY))
		{
			if (has_emeralds){
				villager.hire_Villager(player, remaining_i);
			}
		}
	}
	public boolean mousePressed(int mouseX, int mouseY)
    {
        return this.enabled && this.visible && mouseX >= this.xPosition && mouseY >= this.yPosition && mouseX < this.xPosition + this.width && mouseY < this.yPosition + this.height;
    }

}

 

Edited May 31, 2017 by OrangeVillager61

[Choonster]

You should add the button once in an override of GuiScreen#initGui, after calling the super method. You're currently adding a new button every frame. Override GuiScreen#actionPerformed to perform an action when the button is pressed.

 

When clicked, the button should send a packet to the server that hires the villager after checking that the player is allowed to do so, e.g. they're within range and there are enough emeralds in the inventory. GUIs only exist on the client, the game state needs to be controlled by the server.

[OrangeVillager61]

17 hours ago, Choonster said:

You should add the button once in an override of GuiScreen#initGui, after calling the super method. You're currently adding a new button every frame. Override GuiScreen#actionPerformed to perform an action when the button is pressed.

Is this what you meant?

public class GuiIvVillagerHireNitwit extends GuiContainer{

	private IvVillager villager;
	private IInventory playerInv;
	private EntityPlayer player;
	protected int remaining_i = 0;
	
	public GuiIvVillagerHireNitwit(IvVillager villager, IInventory playerInv, EntityPlayer player) {
		super(new ContainerIvVillagerHireNitwit(villager, playerInv));
		
		this.xSize = 176;
		this.ySize = 166;
		this.player = player;
		
		this.villager = villager;
		this.playerInv = playerInv;

	}

	@Override
	public void initGui()
	{
		super.initGui();
        Boolean has_emeralds;
        if (this.inventorySlots.getSlot(0).getStack().getCount() >= villager.getHireCost() && this.inventorySlots.getSlot(0).getStack().getItem().equals(Items.EMERALD)){ //this is where the container needs to be accessed
        	has_emeralds = true;
        	if (this.inventorySlots.getSlot(0).getStack().getCount() > villager.getHireCost())
        	{
        	this.remaining_i = this.inventorySlots.getSlot(0).getStack().getCount() - villager.getHireCost();
        	}
        }
        else 
        {
        	has_emeralds = false;
        }
        this.addButton(new Button_Hire(0, this.getGuiLeft() + 115, this.getGuiTop() + 20, 50, 25, "Hire", this.villager, has_emeralds, this.player, this.remaining_i));
	}
	@Override
	protected void drawGuiContainerBackgroundLayer(float partialTicks, int mouseX, int mouseY) {
		GlStateManager.color(1.0F, 1.0F, 1.0F, 1.0F);
		this.mc.getTextureManager().bindTexture(new ResourceLocation(Reference.MOD_ID, "gui/hire_nitwit.png"));
		this.drawTexturedModalRect(this.getGuiLeft(), this.getGuiTop(), 0, 0, this.xSize, this.ySize);
	}
	
	@Override
    protected void actionPerformed(GuiButton button)
    {
		this.villager.hire_Villager(this.player, remaining_i);
    }
	
	@Override
	protected void drawGuiContainerForegroundLayer(int mouseX, int mouseY)
    {
        String s = this.villager.getName();

        this.mc.fontRenderer.drawString(String.valueOf(villager.getHireCost()), 46, 50, 4210752);
        this.mc.fontRenderer.drawString(s, this.xSize / 2 - this.mc.fontRenderer.getStringWidth(s) / 2, 6, 4210752);
        this.mc.fontRenderer.drawString(this.playerInv.getDisplayName().getFormattedText(), 8, 72, 4210752);
    }

}

public void hire_Villager(EntityPlayer player, int remaining_i)
	 {
		 if (!this.world.isRemote){
			 this.setHired(true);
	         this.setOwnerId(player.getUniqueID());
     		 this.entityDropItem(new ItemStack(Items.EMERALD, remaining_i), 0);
		 }
	 }

17 hours ago, Choonster said:

 

When clicked, the button should send a packet to the server that hires the villager after checking that the player is allowed to do so, e.g. they're within range and there are enough emeralds in the inventory. GUIs only exist on the client, the game state needs to be controlled by the server.

How to send a packet to the server? I'm also particularly bad with packets and client-server connections.

[Choonster]

25 minutes ago, OrangeVillager61 said:

Is this what you meant?

 

Only check the emerald count when the button is clicked (or when drawing the screen if you want the button to look different when there's enough emeralds), don't check it in initGui.

 

World#isRemote will always be true in a method called from a GUI, because GUIs only exist on the logical client.

 

26 minutes ago, OrangeVillager61 said:

How to send a packet to the server? I'm also particularly bad with packets and client-server connections.

 

Use the Simple Network Implementation to send packets.

[OrangeVillager61]

6 hours ago, Choonster said:

 

Only check the emerald count when the button is clicked (or when drawing the screen if you want the button to look different when there's enough emeralds), don't check it in initGui.

 

World#isRemote will always be true in a method called from a GUI, because GUIs only exist on the logical client.

 

 

Use the Simple Network Implementation to send packets.

Alright, but how to send the villager instance, the number of emeralds to drop and the player in the same class or is that not possible?

[Choonster]

20 minutes ago, OrangeVillager61 said:

Alright, but how to send the villager instance, the number of emeralds to drop and the player in the same class or is that not possible?

 

Send the villager's entity ID in the packet.

 

Don't send the number of emeralds to drop, let the server determine that itself. Malicious clients can send whatever packets they want, they could tell the server to drop a hundred emeralds.

 

You don't need to send the player, the packet handler knows which player's client sent the packet.

Edited June 1, 2017 by Choonster

[OrangeVillager61]

21 hours ago, Choonster said:

 

Send the villager's entity ID in the packet.

 

Don't send the number of emeralds to drop, let the server determine that itself. Malicious clients can send whatever packets they want, they could tell the server to drop a hundred emeralds.

 

You don't need to send the player, the packet handler knows which player's client sent the packet.

Alright, do I call the function that hires the villager in the onMessage or in IvVillager? Also, how to get the entity from the id?

Edited June 2, 2017 by OrangeVillager61

[Choonster]

37 minutes ago, OrangeVillager61 said:

Alright, do I call the function that hires the villager in the onMessage or in IvVillager?

 

The IMessageHandler#onMessage implementation should call the IvVillager method that hires the villager (after checking that it's allowed).

 

 

39 minutes ago, OrangeVillager61 said:

Also, how to get the entity from the id?

 

Look at the GuiHandler code you merged from my fork of the repository, I did the same thing there.

[OrangeVillager61]

2 minutes ago, Choonster said:

(after checking that it's allowed)

How do I check if it is allowed from onMessage and access the GUI from onMessage.

[Choonster]

2 minutes ago, OrangeVillager61 said:

How do I check if it is allowed from onMessage and access the GUI from onMessage.

 

On 2017-6-1 at 3:41 AM, Choonster said:

When clicked, the button should send a packet to the server that hires the villager after checking that the player is allowed to do so, e.g. they're within range and there are enough emeralds in the inventory. 

 

You don't access the GUI, you access the IItemHandler inventory that the GUI interacts with.

 

The IMessageHandler should check that the player is in range and then call the hire method. The hire method should take an EntityPlayer argument and check that there are enough emeralds in the villager's inventory and the villager isn't already hired (plus any other prerequisites) before setting the villager as hired and the player as its owner.

[OrangeVillager61]

Is this what is needed to accomplish what I am trying to do?

@Override
    protected void actionPerformed(GuiButton button)
    {
		Boolean has_emeralds;
        if (this.inventorySlots.getSlot(0).getStack().getCount() >= villager.getHireCost() && this.inventorySlots.getSlot(0).getStack().getItem().equals(Items.EMERALD)){ 
        	has_emeralds = true;
        }
        else 
        {
        	has_emeralds = false;
        }
        if (has_emeralds)
        {
        	Reference.PACKET_MODID.sendToServer(new EntityIdProxy(this.villager.getEntityId()));
        }
    }

public class PacketHandler implements IMessageHandler<EntityIdProxy, IMessage> {
// Do note that the default constructor is required, but implicitly defined in this case

@Override public IMessage onMessage(EntityIdProxy message, MessageContext ctx) {
 // This is the player the packet was sent to the server from
 EntityPlayerMP serverPlayer = ctx.getServerHandler().player;
 // The value that was sent
 int amount = message.toSend;
 int remaining_i;
 IvVillager villager = (IvVillager) ctx.getServerHandler().player.world.getEntityByID(amount);
 ItemStackHandler item_handler = (ItemStackHandler) villager.getCapability(CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, null);
 if (item_handler.getStackInSlot(15).getCount() >= villager.getHireCost() && item_handler.getStackInSlot(15).getItem().equals(Items.EMERALD) && villager.getHired())
 {
	 remaining_i = item_handler.getStackInSlot(15).getCount() - villager.getHireCost();
	 villager.hire_Villager(serverPlayer, remaining_i);
 }
 // No response packet
 return null;
}
}

 

[Choonster]

You're not doing it the way I told you to in my previous post, but it's mostly correct.

 

The main issues are that you're running this code on the network thread instead of the main thread (see the Warning box on the Simple Network Wrapper documentation page for an explanation) and that you're checking that the villager is hired before attempting to hire it.

 

Why are using slot 15 of the IItemHandler? You should expose the IItemHandler that stores the emeralds directly (either via ICapabilityProvider#getCapability or a custom method) and use that rather than using a wrapper of all of the villager's inventories (which I presume is why you're using slot 15). You only need to expose an object through ICapabilityProvider if you want external code to be able to access it.

 

Use descriptive names for your classes, fields, methods, etc. that reflect their purpose:

• EntityIdProxy doesn't tell me that the class is an IMessage, nor does it tell me anything about what the packet does. I recommend naming your packets Message[Action], where [Action] is the action that the packet performs (i.e. why it's sent in the first place). This packet hires a villager, so name it something like MessageHireVillager.
• toSend doesn't tell me anything about the contents or purpose of the field, the fact that it's sent in a packet is already obvious from the context so it doesn't need to be included in the name. This field stores the villager's entity ID, so name it something like entityID or villagerEntityID.

 

Only declare local variables in the scope where they're used. There's no reason to declare the remaining_i variable at the start of the method if you're only going to use inside the if statement.

[OrangeVillager61]

18 minutes ago, Choonster said:

You're not doing it the way I told you to in my previous post, but it's mostly correct.

 

The main issues are that you're running this code on the network thread instead of the main thread (see the Warning box on the Simple Network Wrapper documentation page for an explanation).

So I put the code to run from the IvVillager like this?

@Override public IMessage onMessage(MessageSendEntityId message, MessageContext ctx) {
 // This is the player the packet was sent to the server from
 EntityPlayerMP serverPlayer = ctx.getServerHandler().player;
 // The value that was sent
 int amount = message.EntityID;
 IvVillager villager = (IvVillager) ctx.getServerHandler().player.world.getEntityByID(amount);
 ItemStackHandler item_handler = (ItemStackHandler) villager.getCapability(CapabilityItemHandler.ITEM_HANDLER_CAPABILITY, null);
 if (item_handler.getStackInSlot(15).getCount() >= villager.getHireCost() && item_handler.getStackInSlot(15).getItem().equals(Items.EMERALD) && !villager.getHired())
 {
	 int remaining_i = item_handler.getStackInSlot(15).getCount() - villager.getHireCost();
	 WorldServer.addScheduledTask(villager.hire_Villager(serverPlayer, remaining_i));
 }
 // No response packet
 return null;
}

Quote

Why are using slot 15 of the IItemHandler? You should expose the IItemHandler that stores the emeralds directly (either via ICapabilityProvider#getCapability or a custom method) and use that rather than using a wrapper of all of the villager's inventories (which I presume is why you're using slot 15). You only need to expose an object through ICapabilityProvider if you want external code to be able to access it.

So I don't access the IItemHandler directly through the villager? Do I need to directly make a new ICapabiltyProvider or what do I need to do?

[Choonster]

22 minutes ago, OrangeVillager61 said:

So I put the code to run from the IvVillager like this?

 

You can't safely interact with normal Minecraft classes from the network thread, so the first thing you do in IMessageHandler#onMessage should be to schedule the task to run on the main thread. Only get the villager and hire it inside the scheduled task.

 

WorldServer#addScheduledTask isn't a static method, you need to call it on an instance of WorldServer. The MinecraftServer class also implements IThreadListener, so you can use the server instance (EntityPlayerMP#mcServer) to schedule the task instead of a WorldServer instance (the WorldServer implementation of IThreadListener delegates to the MinecraftServer implementation anyway).

 

The MessageSendEntityID name isn't much better than EntityIDProxy. The purpose of the packet is to hire a villager, the fact that it sends an entity ID is just an implementation detail.

 

 

22 minutes ago, OrangeVillager61 said:

So I don't access the IItemHandler directly through the villager? Do I need to directly make a new ICapabiltyProvider or what do I need to do?

 

Entity already implements ICapabilityProvider, so IvVillager implements it as well. You don't need to create a new implementation. What I was trying to say is that you only need to expose an IItemHandler (or any other capability handler instance) through the ICapabilityProvider methods if you want vanilla or other mods to interact with it. If only your code needs to interact with it, you can create your own method in IvVillager to return it.

 

The IMessageHandler shouldn't really be checking the hire cost itself, it should simply check that the player is within range of the villager before calling IvVillager#hire_Villager. IvVillager#hire_Villager should check the hire cost and the hired status before setting the villager as hired and the player as its owner.