[SPIGOT-1080] CraftHumanEntity.openInventory(Inventory inventory) only works with CraftInventory Created: 27/Jul/15  Updated: 31/Jul/15  Resolved: 31/Jul/15

Status: Resolved
Project: Spigot
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Riccardo Paolo Bestetti Assignee: md_5
Resolution: Fixed Votes: 0
Labels: craftbukkit, entity, inventory
Environment:

Any; I've traced the bug in the code.


 Description   

I was rewriting a Virtual Chest Plugin when I noticed this bug.

The method CraftHumanEntity.openInventory(Inventory inventory), which of course implements HumanEntity.openInventory(Inventory inventory), does something which in my opinion (explained below) is very nasty: it casts the received Inventory to CraftInventory (without even checking, which is something that IMO should be corrected as soon as possible).
What this results in is that a plugin writer that has written a perfectly fine implementation of Inventory, with a different storage mechanism than CraftInventory, to fit his purpose, can't use it at all because it will cause an exception to be thrown when the plugin tries to finally send it to the client.

Here you can find a shrinked version of my Inventory implementation, to see what I'm talking about: http://pastebin.com/uAFgxTdX

Here's my complete opinion on the matter:
One could argue that is it legitimate for CraftBukkit to internally use and work with CraftInventory instances. And that is completely true! I understand that CraftInventory internally uses a Minecraft IInventory, and that it makes the code very convenient as all it has to be done then is to call Minecraft's EntityHuman.openContainer on that IInventory, and then that IInventory would be directly updated by the Minecraft server.
But it should also provide a fallback for classes other than CraftInventory that don't use a IInventory as the internal storage method OR the interface of Inventory should be changed to have a getInventory method which returns a IInventory, which is of course something not to do because then Bukkit would depend on Minecraft (and I think that now even the most skeptical will start to see my point).
I discussed this on the Bukkit forums a lot of time ago, and people told me that I should just extend CraftInventory, and I hope that everyone will see why that's not something to do when developing a plugin with the Bukkit API.
CraftBukkit should glue Bukkit-implementing plugins to Minecraft. It is its exact and only purpose. It should do the work of creating a Minecraft IInventory as the Inventory is sent to the client, and of course place a hook into that IInventory to update the corresponding Inventory as the player manipulates the items in it.
And, again, it can of course check for and use CraftInventory instances the way it does now to optimize, but it should work with any Bukkit Inventory.

Note 1: I created this as major because in my opinion is goes against the purpose of CraftBukkit and also because while this is being fixed there should be at least a check in the code that will throw an appropriate exception (and not a ClassCastException).

Note 2: If it is of any help, I had already reported the bug here, on the old Bukkit tracker: https://bukkit.atlassian.net/browse/BUKKIT-5750

 Comments   
Comment by md_5 [ 27/Jul/15 ]

The problem is that intrinsically Minecraft requires a net.minecraft.server.IInventory implementing class to handle it's inventory functions.
It is non trivial to implement this interface:

package net.minecraft.server;

import org.bukkit.craftbukkit.entity.CraftHumanEntity; // CraftBukkit

public interface IInventory extends INamableTileEntity {

    int getSize();

    ItemStack getItem(int i);

    ItemStack splitStack(int i, int j);

    ItemStack splitWithoutUpdate(int i);

    void setItem(int i, ItemStack itemstack);

    int getMaxStackSize();

    void update();

    boolean a(EntityHuman entityhuman);

    void startOpen(EntityHuman entityhuman);

    void closeContainer(EntityHuman entityhuman);

    boolean b(int i, ItemStack itemstack);

    int getProperty(int i);

    void b(int i, int j);

    int g();

    void l();

    // CraftBukkit start
    ItemStack[] getContents();

    void onOpen(CraftHumanEntity who);

    void onClose(CraftHumanEntity who);

    java.util.List<org.bukkit.entity.HumanEntity> getViewers();

    org.bukkit.inventory.InventoryHolder getOwner();

    void setMaxStackSize(int size);

    int MAX_STACK = 64;
    // CraftBukkit end
}

I am happy to update the javadoc and exceptions of this method, but if you want to actually be able to use a custom Inventory, I'm going to ask that you code the required changes (Inventory -> IInventory wrapper) and submit them to us via the contributing section in the readme: https://hub.spigotmc.org/stash/projects/SPIGOT/repos/craftbukkit/browse

Generated at Sat Dec 13 11:47:24 UTC 2025 using Jira 10.3.13#10030013-sha1:56dd970ae30ebfeda3a697d25be1f6388b68a422.