/shop add duplicates damaged, enchanted or renamed items

Aperta
  • Qualcosa non va?
#64 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
3/5
Tempo stimato
1-2 giorni
Idoneità per principianti
63/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
java
Ambito
backend

Direzione di ricerca

Read InventoryManagementSystem.canRemove and its countQuantity logic, then trace how AddCommand removes items and how Transaction.process handles leftovers from addItem. Check the existing inventory tests and add coverage for damaged and renamed items during stock addition and purchases into partial stacks. Done when only eligible items are counted and removed, and inventory changes match the quantities recorded by the store.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug

Problem

/shop add counts items by material, but removes them by exact match. A damaged, enchanted or renamed item passes the check, isn't removed from the player, and is still added to the store. This duplicates items.

InventoryManagementSystem.canRemove uses countQuantity, which only compares the material. AddCommand then calls removeItem(new ItemStack(material, quantity)), which only removes plain items, and ignores the leftovers it returns.

Steps to reproduce

  1. Have a store selling diamond_sword, and hold one damaged diamond sword.
  2. Run /shop add diamond_sword 1. The store's quantity goes up and the player keeps the damaged sword.
  3. Run /shop remove diamond_sword 1. The player now has two swords, one of them undamaged.

Reproduced in a test: one sword in the inventory became two.

Related

getAvailableSpace also counts partial stacks by material only, so space in a stack of renamed items is treated as free. On a real server addItem can't merge into those stacks, so a purchase could take the player's money and lose the items that didn't fit. Transaction.process ignores the leftovers from addItem. MockBukkit merged those stacks in testing, so this part comes from reading the code and should be confirmed on a test server.

Scope

  • Decide how non-plain items are treated. The simplest rule is that only plain items (matching a new ItemStack of that material) can be added.
  • Use the same rule when counting, checking space and removing items.

Done when

  • A damaged, enchanted or renamed item can't be added as plain stock, and the player's inventory always changes by exactly what the store records.
  • Tests cover damaged and renamed items for add, and for buying into partial stacks.
Lingua principale
Java
Stelle
1
Fork
0
Merge medio
2h 35m
PR unite (30g)
14

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di BrendonButler/Shops

Tutte le issue di BrendonButler/Shops

Issue simili

Altre issue su Java

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.