Upgraded Shulkers

Upgraded Shulkers

Mods
1M Downloads

ItemEntityMixin#fillExistingStack is faulty, causing nbt merging when picking up stuff which can lead to dupe bugs

quat1024 opened this issue ยท 0 comments

commented

shulkerupgrades/src/main/java/net/kyrptonaught/upgradedshulker/mixin/ItemEntityMixin.java

Lines 72 to 84 in 0ce6cf0

private void fillExistingStack(PlayerEntity player, ItemStack pickupStack, PlayerInventory playerInventory) {
if (pickupStack.isStackable())
for (int i = 0; i < playerInventory.size(); i++) {
ItemStack shulker = playerInventory.getStack(i);
if (shulker.isStackable() && shulker.getCount() < shulker.getMaxCount() && shulker.isItemEqual(pickupStack)) {
int amount = Math.min(shulker.getMaxCount() - shulker.getCount(), pickupStack.getCount());
shulker.increment(amount);
pickupStack.decrement(amount);
if (pickupStack.getCount() <= 0)
break;
}
}
}

This code (where shulker is actually the itemstack in the player's inventory) gets called pretty much any time anyone picks up anything, see

shulkerupgrades/src/main/java/net/kyrptonaught/upgradedshulker/mixin/ItemEntityMixin.java

Lines 45 to 52 in 0ce6cf0

@Inject(method = "onPlayerCollision", at = @At(value = "HEAD"), cancellable = true)
public void attemptHopperPickup(PlayerEntity player, CallbackInfo ci) {
if (!this.world.isClient) {
ItemStack itemStack = this.getStack();
Item item = itemStack.getItem();
int i = itemStack.getCount();
if (this.pickupDelay == 0 && (this.owner == null || this.owner.equals(player.getUuid()))) {
fillExistingStack(player, itemStack, player.getInventory());

I think the intention is to partially fill the player's inventory with the picked-up itemstack before allowing the hopper-box feature to absorb the rest, but line 76 assumes that all pairs of itemstacks that have the same item actually can stack with each other.

This means that items with nbt will always be coerced to whichever one is already in your inventory, e.g. if you rename a stick in an anvil, then drop and pick up a non-renamed stick, you will have two renamed sticks. If someone adds a mod that makes shulkerboxes stackable, they will be able to duplicate the items inside. This code led to a dupe bug in my mod which adds stackable container items. And other bad stuff.