Instead of making demands why don't you explain whatever the problem is and why it should be "fixed"?

It should remove stacks from slots. If you look at all the implementations they're mostly copypasted, except for the player inventory. That one handles both normal inventory and armor inventory, which takes a bit more effort than copypasting. Thus, it is safe to assume that method is meant for removing stacks from slots.

Sure, it's only ever called by vanilla when removing stacks on container close, but that doesn't make it any less of a removeStackFromSlot.

From my understanding this is for stuff like the crafting table, i.e. 'temporary' inventories. AKA nothing in OC. If you can point me to a single use-case (in vanilla) that says otherwise, please do.

It's only /used/ that way, but it's ALWAYS implemented in the exact same way: If the slot is empty return null, otherwise return whatever is in there, while setting it to null.

Also, it wouldn't do any harm to implement it that way.

The fact that both player inventories and beacons have specialized implementations (instead of copypaste and/or being left unimplemented) and the fact that all implementations remove stack from slot makes me believe it's supposed to be named removeStackFromSlot instead of getStackInSlotOnClosing.

Hmm, interesting, that indeed sounds likely. It'd also explain why it never caused any issues in over two years; nobody's actually calling that because of the weird name :P Since it at least isn't called from anywhere within OC, I suppose I might as well change it.

af49402