Skip to content

Fix "lore" metadata duplication#239

Open
Ch1sKey wants to merge 1 commit intoMaxlego08:developfrom
Ch1sKey:fix/zmenu-new-api-compat
Open

Fix "lore" metadata duplication#239
Ch1sKey wants to merge 1 commit intoMaxlego08:developfrom
Ch1sKey:fix/zmenu-new-api-compat

Conversation

@Ch1sKey
Copy link
Copy Markdown

@Ch1sKey Ch1sKey commented Apr 30, 2026

There is an issue lore duplication for some items. This affect zShop and causes this issue . Fix is blunt and straightforward. I'm open to comment on better approach.

@1robie
Copy link
Copy Markdown
Collaborator

1robie commented May 1, 2026

Hey, please change the branch to develop.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes lore/metadata duplication caused by mutating ItemStack instances when adding items to menus (impacting zShop, per linked issue), by separating the “source” stack from the “displayed” stack.

Changes:

  • Clone the provided ItemStack into a new displayStack.
  • Apply anti-dupe protection to displayStack instead of mutating the original itemStack.
  • Use displayStack when placing items into the Bukkit inventory/player inventory.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 117 to +139
@@ -130,11 +132,11 @@ public ItemButton addItem(boolean inPlayerInventory, int slot, ItemStack itemSta
if (inPlayerInventory) {

this.playerInventoryItems.put(slot, button);
if (!needCancel) this.player.getInventory().setItem(slot, itemStack);
if (!needCancel) this.player.getInventory().setItem(slot, displayStack);
} else {

this.items.put(slot, button);
if (!needCancel) this.inventory.setItem(slot, itemStack);
if (!needCancel) this.inventory.setItem(slot, displayStack);
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ItemButton is still constructed with the original itemStack, but the inventory slot is populated with displayStack (potentially dupe-protected). This creates a mismatch where itemButton.getDisplayItem() no longer reflects what is actually displayed/sent. In particular, InventoryListener implementations (e.g. PacketEvents listeners) use itemButton.getDisplayItem() to send items via packets when needCancel is true, which would bypass the anti-dupe protection you just applied to displayStack. Construct the ItemButton using the same stack that will be displayed (e.g., displayStack), or otherwise ensure the protected stack is what listeners and packet sends will use.

Copilot uses AI. Check for mistakes.
@Maxlego08 Maxlego08 changed the base branch from main to develop May 1, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants