diff --git a/internal/peteclient/client.go b/internal/peteclient/client.go index 618f480..dcf2911 100644 --- a/internal/peteclient/client.go +++ b/internal/peteclient/client.go @@ -393,7 +393,9 @@ type PlayerDetail struct { // CanTakeOff), not in Equipped, which stays magic-only (the DnD slots). Slots []EquipSlotView `json:"slots,omitempty"` // Balance is the owner's euro balance, for the web's upgrade/repair confirm. - Balance float64 `json:"balance,omitempty"` + // No omitempty: a €0 balance is a real, informative fact (a broke player), not + // an absent one — dropping it would let the confirm dialog show a stale amount. + Balance float64 `json:"balance"` } // EquipSlotView is one of the 5 standard equipment slots, carrying what the web diff --git a/internal/plugin/pete_equip.go b/internal/plugin/pete_equip.go index 84331e7..755a464 100644 --- a/internal/plugin/pete_equip.go +++ b/internal/plugin/pete_equip.go @@ -122,6 +122,14 @@ func (p *AdventurePlugin) fulfilEquipOrder(ctx context.Context, order peteclient // a human note for Pete, or retry=true for a transient fault that should leave the // order pending. It records nothing and pushes nothing — the caller does both. func (p *AdventurePlugin) applyEquipOrder(owner id.UserID, order peteclient.EquipOrder) (status, detail string, retry bool) { + // Serialize against the owner's own Matrix-side mutations (!give, !equip, !sell, + // arena, …), all of which hold this same per-user lock. Without it the poll + // goroutine's equip could interleave with a concurrent !give of the very item it + // resolved — the duplication the DM equip confirm takes this lock to prevent. + userMu := p.advUserLock(owner) + userMu.Lock() + defer userMu.Unlock() + switch order.Action { case "equip": inv, err := loadAdvInventory(owner) diff --git a/internal/plugin/pete_equip_manage.go b/internal/plugin/pete_equip_manage.go index 1e89f0b..41f9ae6 100644 --- a/internal/plugin/pete_equip_manage.go +++ b/internal/plugin/pete_equip_manage.go @@ -48,10 +48,14 @@ type mwEquipOutcome struct { } // applyMasterworkEquip wears one masterwork/arena backpack piece into its standard -// slot, mirroring the magic path's anti-duplication ordering: evict any special -// occupant, then remove the incoming row FIRST, then write the slot, restoring the -// row on a save fault. One implementation of that ordering per family of gear — -// the DM confirm handler (adventure_masterwork.go) is the message-carrying twin. +// slot. Ordering is anti-duplication AND safe under the equip poll's 30s retry: +// remove the incoming row FIRST (restoring it on a save fault), then write the +// slot, and only THEN evict any displaced special occupant back to the pack. The +// eviction comes last, once the slot no longer references the occupant, so it can +// never mint a duplicate; and it is best-effort — a failure there is logged, not +// aborted on, the same tolerance the DM confirm handler (adventure_masterwork.go) +// lives with. Aborting after the slot write would strand a completed equip for a +// retry that re-evicts the occupant on every tick. func applyMasterworkEquip(uid id.UserID, it AdvItem) (mwEquipOutcome, error) { if it.Slot == "" || (it.Type != "MasterworkGear" && it.Type != "ArenaGear") { return mwEquipOutcome{}, errItemNotEquippable @@ -74,19 +78,18 @@ func applyMasterworkEquip(uid id.UserID, it AdvItem) (mwEquipOutcome, error) { return mwEquipOutcome{}, errEquipDowngrade } - // Evict a special occupant back to the pack. A plain shop-tier occupant is not - // an item — it is just the slot's tier — so it is overwritten, not evicted, the - // same as the DM confirm handler and the shop. - var swappedBack string + // Capture the special occupant to evict, if any, BEFORE the slot write below + // mutates cur in place. A plain shop-tier occupant is not an item — it is just + // the slot's tier — so it is overwritten, not evicted, the same as the DM confirm + // handler and the shop. The actual re-pack happens after the slot write (below), + // so it can never duplicate the piece. + var evicted *AdvItem if cur != nil && (cur.Masterwork || cur.ArenaTier > 0) { old := AdvItem{Name: cur.Name, Type: "MasterworkGear", Tier: cur.Tier, Slot: slot, SkillSource: cur.SkillSource} if cur.ArenaTier > 0 { old.Type = "ArenaGear" } - if err := addAdvInventoryItem(uid, old); err != nil { - return mwEquipOutcome{}, err - } - swappedBack = cur.Name + evicted = &old } // Destructive op first: pull the incoming row before writing the slot, so a save @@ -124,6 +127,20 @@ func applyMasterworkEquip(uid id.UserID, it AdvItem) (mwEquipOutcome, error) { } return mwEquipOutcome{}, err } + + // The slot now holds the incoming piece, so the former occupant is referenced + // nowhere — re-packing it now cannot duplicate it. Best-effort: a failure is a + // bounded, non-compounding loss we log rather than abort on, since the equip has + // already succeeded and aborting would re-run (and re-evict) on the next poll. + var swappedBack string + if evicted != nil { + if err := addAdvInventoryItem(uid, *evicted); err != nil { + slog.Error("equip: masterwork equipped but evicted piece failed to return to pack", + "user", uid, "evicted", evicted.Name, "err", err) + } else { + swappedBack = evicted.Name + } + } return mwEquipOutcome{Name: it.Name, Slot: slot, Tier: it.Tier, Arena: it.Type == "ArenaGear", SwappedBack: swappedBack}, nil }