external_storage dupe bug(1.12)
CatAndHorse opened this issue · 12 comments
1.build an rs system with 2 external storage bus connect to 2 different chest
2.put 32 item into each chest
3.try to get 64 item by shift clicking in terminal or by export bus
4.you get 64 items and 32 remain in rs sys
Version (make sure you are on the latest version before reporting):
- Minecraft: 1.12
- Forge:2420
- Refined Storage:1513
Does this issue occur on a server? [yes/no]
yes
Are you using Colossal chests? If so, are you connecting External Storage to Chest core?
Nope ,i tried vanilla chest , extrautil tiny chest and acatully addition wooden crate,all of them can reproduce this dupe
In other words: this only happens with ExU mini chest? In that case, it might be an ExU dupe bug issue. Will still investigate.
This appears to be a ExU issue.
I tested this with 2 external storages both connected to its own Mini-Chest.
The problem is as follows:
- When shift clicking, RS asks 64 of that item
- Then, ExU gives back a stack of 64 items even though it only has 32
requested
is 64, because we want 64 items for shift click- We got back a stack of 64 even though the Mini-Chest only has 32 of that item (look at
took
)
My testing setup:
Both Mini-Chests have 32 items. But ExU just gives back 64 items for the first chest, even though it only has 32. So RS stops and doesn't extract from the second one.
In conclusion: this is a ExU issue. They appear to trust the "amount" parameter without checking if it doesn't exceed their own stack size.
And the problem is - @rwtema never reads github issues... ExtraUtils 2 issue tracker just drowned in issues
@raoulvdberge confirmed.
This bug can be reproduced. @CatAndHorse just forgot to mention about priority.
Actually additions' storage crates can reproduce this too @raoulvdberge
okay i will open issue there too
I just looked into again some more and I was wrong: this is not an ExU or ActAdd issue. It's in fact an RS issue.
What's going on: I assumed IItemHandler#extract
is guaranteed to make copies of the stack, and I could just safely modify the stack returned by it.
Apparantly it does copy the stack, but not when requesting more than the stack size.
@Nonnull
public ItemStack extractItem(int slot, int amount, boolean simulate) {
if (amount == 0) {
return ItemStack.EMPTY;
} else {
this.validateSlotIndex(slot);
ItemStack existing = (ItemStack)this.stacks.get(slot);
if (existing.isEmpty()) {
return ItemStack.EMPTY;
} else {
int toExtract = Math.min(amount, existing.getMaxStackSize());
if (existing.getCount() <= toExtract) {
if (!simulate) {
this.stacks.set(slot, ItemStack.EMPTY);
this.onContentsChanged(slot);
}
return existing;
} else {
if (!simulate) {
this.stacks.set(slot, ItemHandlerHelper.copyStackWithSize(existing, existing.getCount() - toExtract));
this.onContentsChanged(slot);
}
return ItemHandlerHelper.copyStackWithSize(existing, toExtract);
}
}
}
}
Look at the return existing;
.
Thanks to that, RS just modifies the stack when extracting and false assumptions are made.