Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SlotItemHandler#getMaxStackSize call IItemHandlerModifiable#setStackInSlot #1896

Open
reasure3 opened this issue Jan 23, 2025 · 1 comment
Open
Labels
triage Needs triaging and confirmation

Comments

@reasure3
Copy link

reasure3 commented Jan 23, 2025

Minecraft Version: 1.21.1

NeoForge Version: 21.1.97

Steps to Reproduce:

  1. make SlotItemHandler in your menu class
  2. Place 32 dirt in the slot
  3. Add 1 dirt to the slot
  4. IItemHandler#setStackInSlot is called 3 times (First: Air, Second: Dirt * 32, Third: Dirt * 33)
  5. This creates unintended side effects.

Description of issue:
Below is a portion of the SlotItemHandler#getMaxStackSize code.

@Override
public int getMaxStackSize(ItemStack stack) {
    ItemStack maxAdd = stack.copy();
    int maxInput = stack.getMaxStackSize();
    maxAdd.setCount(maxInput);

    IItemHandler handler = this.getItemHandler();
    ItemStack currentStack = handler.getStackInSlot(index);
    if (handler instanceof IItemHandlerModifiable) {
        IItemHandlerModifiable handlerModifiable = (IItemHandlerModifiable) handler;

        handlerModifiable.setStackInSlot(index, ItemStack.EMPTY);

        ItemStack remainder = handlerModifiable.insertItem(index, maxAdd, true);

        handlerModifiable.setStackInSlot(index, currentStack);

        return maxInput - remainder.getCount();
    }

As you can see, there is a part

handlerModifiable.setStackInSlot(index, ItemStack.EMPTY);
handlerModifiable.setStackInSlot(index, currentStack);

This creates unintended side effects.
For example, IItemHandler#onContentChanged is called a total of 3 times.
In my case, I want to update the recipe when a different item is inserted, just like the StoneCutter block works.
However, it didn't work properly because ItemStack.Empty is always inserted once when an item is changed in the slot.

@reasure3 reasure3 added the triage Needs triaging and confirmation label Jan 23, 2025
reasure3 added a commit to reasure3/CrystalOdyssey-Neoforge that referenced this issue Jan 24, 2025
@reasure3 reasure3 changed the title SlotItemHandler#getMaxStackSize call IItemHandlerModifiable#setStackInSlot SlotItemHandler#getMaxStackSize call IItemHandlerModifiable#setStackInSlot Jan 24, 2025
@KnightMiner
Copy link
Contributor

This has been the case for awhile, I suspect Forge was mimicking some vanilla logic or otherwise forgot they had their own method for this. I have a fix that I implemented in my own mods here: https://github.com/SlimeKnights/Mantle/blob/1.20/src/main/java/slimeknights/mantle/inventory/SmartItemHandlerSlot.java

Easy enough to add to a mod or to just fix in Neo, I don't think there are any advantages to doing it the weird vanilla way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage Needs triaging and confirmation
Projects
None yet
Development

No branches or pull requests

2 participants