AlexIIL / LibBlockAttributes

Library mod for fabric that adds items and attribute API's for blocks.
Mozilla Public License 2.0
43 stars 12 forks source link

Fix Chiseled Bookshelf dupe #52

Closed Kneelawk closed 1 year ago

Kneelawk commented 1 year ago

This PR

This PR fixes Chiseled Bookshelves duping books due to not being willing to set a slot to an empty stack.

Implementation Notes

This uses a mixin into ChiseledBookshelfBlockEntity to implement a new CustomLogicInventory which FixedInventoryVanillaWrapper then checks for.

AlexIIL commented 1 year ago

I'd rather not mixin into vanilla classes if we can avoid it. Since we're missing javadocs from Mojang, it's possible that this means Inventory.setStack is not intended to be called with an empty stack - and Inventory.removeStack(slot) should be used instead. (It's also possible that this is just an oversight by mojang). Personally, I think a better fix would be to change FixedInventoryVanillaWrapper in this way, which hopefully doesn't break anything else:

@Override
public boolean setInvStack(int slot, ItemStack to, Simulation simulation) {
    boolean allowed = false;
    ItemStack current = getInvStack(slot);
    boolean removing = to.isEmpty();
    if (removing) {
        allowed = canExtract(slot, current);
    } else {
        if (current.isEmpty()) {
            allowed = canInsert(slot, to);
        } else if (ItemStackUtil.areEqualIgnoreAmounts(to, current)) {
            if (to.getCount() < current.getCount()) {
                allowed = canExtract(slot, current);
            } else {
                allowed = canInsert(slot, to);
            }
        } else {
            allowed = canInsert(slot, to) && canExtract(slot, current);
        }
    }
    if (allowed) {
        if (simulation == Simulation.ACTION) {
            if (removing) {
                inv.removeStack(slot);
            } else {
                inv.setStack(slot, to);
            }
            inv.markDirty();
        }
        return true;
    }
    return false;
}
Kneelawk commented 1 year ago

Thanks for taking a look at this. Will do.

Kneelawk commented 1 year ago

Ok, the way you suggested works with chiseled bookshelves. And after looking at many of the usages and implementations of removeStack, I think this should be fine.

I'll merge it now.