From f424300d87adddce110884b99fb679de46b213d3 Mon Sep 17 00:00:00 2001 From: prosolis <5590409+prosolis@users.noreply.github.com> Date: Fri, 15 May 2026 14:43:11 -0700 Subject: [PATCH] Ship-audit P0/P1 sweep on open5e: prose, combat math, UX hygiene MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 — actual bugs: - eldritch_blast: SRD-imported "Whatever" placeholder leaked to players; overlay added in dnd_spells_data.go. - Battle Master disarming_attack + parry: math.Max on multiplicative DamageReduct was a no-op vs the 1.0 default. Switched to *= and added regression tests covering the stacking case too. - Extended TestSpellDescriptionsAreJargonFree to catch "Whatever", trailing ellipses, mid-word truncation, and empty-size phrases. This surfaced 4 more genuinely-broken SRD descriptions (find_familiar, clairvoyance, glyph_of_warding, teleportation_circle); all now overlayed in dnd_spells_data.go. P1 — UX hygiene: - Setup completion now nudges new casters toward !spells/!cast. - Magic-item attunement vocab unified to "bond" everywhere user-facing; renamed the Treasures sheet section to avoid clashing with the bond vocab. - Ambient DM footers stripped of "Threat +N" / "Supplies −N SU" hidden- meter leakage; verb-form footers instead. - !expedition status defaults to a days-left + threat-label summary; raw SU / threat-out-of-100 / zone-stack / roll-modifier moved behind `!expedition status --debug`. - !zone taunt/compliment help text dropped the "(mood −10)" / "(mood +5)" numerics that turned the hidden TwinBee-mood mechanic into a min/max knob. - TwinBee pronoun pass: switched she/her to singular they on the two surfaces I touched plus achievements.bj_beat_twinbee. P1 — correctness/scale: - adventure_shop curio purchase now guards the euro.Debit bool return before inventory grant + pot cut; the GetBalance preflight isn't enough since a concurrent blackjack/lottery debit can slip the balance across BLACKJACK_DEBT_LIMIT between read and write. - equipMagicItem reordered: remove inventory item BEFORE equip, with a best-effort rollback if equip fails. The prior order left a dup window if the inventory delete tripped on a transient DB error. - magicItemsByRarityCache promoted to sync.Once — concurrent cold-start loot rolls otherwise raced on the lazy assignment. Stale-audit verifications (no code change needed): - light / druidcraft / water_walk overlays already exist and win the SRD merge — the broken raw text never reaches players. - bootstrapPhase5BHPRefresh is already wired at adventure.go:228. Deferred: - combatantsForSession per-turn 6-load DB chatter (perf P1-A): touches the hot combat path; needs its own test pass before shipping. Tracked in memory at project_combat_session_cache_deferred. go vet clean; go test -race ./... green (229s). Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/plugin/achievements.go | 2 +- internal/plugin/adventure_shop.go | 12 +++- internal/plugin/dnd_expedition_cmd.go | 50 ++++++++++++---- internal/plugin/dnd_setup.go | 12 +++- internal/plugin/dnd_sheet.go | 6 +- internal/plugin/dnd_spells_data.go | 18 ++++++ internal/plugin/dnd_spells_prose_test.go | 11 ++++ internal/plugin/dnd_subclass_combat.go | 11 ++-- internal/plugin/dnd_subclass_combat_test.go | 39 +++++++++++++ internal/plugin/dnd_zone_cmd.go | 4 +- internal/plugin/expedition_ambient.go | 10 +++- internal/plugin/magic_items_gameplay.go | 65 +++++++++++++-------- 12 files changed, 190 insertions(+), 50 deletions(-) diff --git a/internal/plugin/achievements.go b/internal/plugin/achievements.go index 4fdeb50..f8434f1 100644 --- a/internal/plugin/achievements.go +++ b/internal/plugin/achievements.go @@ -556,7 +556,7 @@ func (p *AchievementsPlugin) buildAchievements() []achievementDef { }, }, { - ID: "bj_beat_twinbee", Name: "Gotcha 🐝", Description: "She is furious and will not forget this.", + ID: "bj_beat_twinbee", Name: "Gotcha 🐝", Description: "They are furious and will not forget this.", Emoji: "🐝", Check: func(d *sql.DB, u id.UserID) bool { // Granted by blackjack plugin when beating the bot diff --git a/internal/plugin/adventure_shop.go b/internal/plugin/adventure_shop.go index c12a4b2..8969057 100644 --- a/internal/plugin/adventure_shop.go +++ b/internal/plugin/adventure_shop.go @@ -1301,7 +1301,17 @@ func (p *AdventurePlugin) resolveShopCurioChoice(ctx MessageContext, interaction return p.SendDM(ctx.Sender, fmt.Sprintf("You need €%.0f for %s but only have €%.0f.", price, match.Name, balance)) } - p.euro.Debit(ctx.Sender, price, "shop_curio") + // Guard the Debit return: GetBalance above is a snapshot read, but + // the debt-limit check inside Debit (BLACKJACK_DEBT_LIMIT) can refuse + // the write if a concurrent debit (blackjack, lottery, etc.) has + // slipped the balance across the limit between read and write. + // Without this guard the curio would be granted on a refused debit. + if !p.euro.Debit(ctx.Sender, price, "shop_curio") { + p.pending.Store(string(ctx.Sender), interaction) + return p.SendDM(ctx.Sender, fmt.Sprintf( + "Tried to charge €%.0f for %s, but the purse refused — your balance moved out from under us. Try again.", + price, match.Name)) + } if potCut := int(math.Round(price * 0.05)); potCut > 0 { communityPotAdd(potCut) trackTaxPaid(ctx.Sender, potCut) diff --git a/internal/plugin/dnd_expedition_cmd.go b/internal/plugin/dnd_expedition_cmd.go index af339e3..972d316 100644 --- a/internal/plugin/dnd_expedition_cmd.go +++ b/internal/plugin/dnd_expedition_cmd.go @@ -49,7 +49,7 @@ func (p *AdventurePlugin) handleDnDExpeditionCmd(ctx MessageContext, args string case "": // If active, show status; otherwise help. if exp, _ := getActiveExpedition(ctx.Sender); exp != nil { - return p.expeditionCmdStatus(ctx) + return p.expeditionCmdStatus(ctx, "") } return p.SendDM(ctx.Sender, expeditionHelpText()) case "help", "?": @@ -59,7 +59,7 @@ func (p *AdventurePlugin) handleDnDExpeditionCmd(ctx MessageContext, args string case "start", "begin", "go": return p.expeditionCmdStart(ctx, c, rest) case "status", "info": - return p.expeditionCmdStatus(ctx) + return p.expeditionCmdStatus(ctx, rest) case "log", "history": return p.expeditionCmdLog(ctx) case "abandon", "quit": @@ -271,7 +271,18 @@ func estimateDays(maxSU, dailyBurn float32) int { // ── status ────────────────────────────────────────────────────────────────── -func (p *AdventurePlugin) expeditionCmdStatus(ctx MessageContext) error { +func (p *AdventurePlugin) expeditionCmdStatus(ctx MessageContext, args string) error { + debug := false + for _, tok := range strings.Fields(args) { + switch strings.ToLower(tok) { + case "--debug", "-d", "debug", "raw": + debug = true + } + } + return p.expeditionCmdStatusImpl(ctx, debug) +} + +func (p *AdventurePlugin) expeditionCmdStatusImpl(ctx MessageContext, debug bool) error { exp, err := getActiveExpedition(ctx.Sender) if err != nil { return p.SendDM(ctx.Sender, "Couldn't read expedition state: "+err.Error()) @@ -298,21 +309,36 @@ func (p *AdventurePlugin) expeditionCmdStatus(ctx MessageContext) error { if c != nil { b.WriteString(fmt.Sprintf("❤️ **HP:** %d / %d\n", c.HPCurrent, c.HPMax)) } - b.WriteString(fmt.Sprintf("🎒 **Supplies:** %.1f / %.1f SU _(burn %.1f/day → ~%d days left)_\n", - exp.Supplies.Current, exp.Supplies.Max, currentBurn(exp), - estimateDays(exp.Supplies.Current, currentBurn(exp)))) - b.WriteString(fmt.Sprintf("⏰ **Threat:** %d / 100 — %s\n", - exp.ThreatLevel, threatThresholdLabel(exp.ThreatLevel, exp.SiegeMode))) - if exp.TemporalStack != 0 { - b.WriteString(fmt.Sprintf("🌡 **Zone stack:** %d\n", exp.TemporalStack)) + // Default view is verb/outcome-led: days-left summary + threat label, + // no raw SU / threat-out-of-100 / zone-stack / roll-modifier numbers. + // Pass `--debug` to !expedition status for the engineering view. + days := estimateDays(exp.Supplies.Current, currentBurn(exp)) + if debug { + b.WriteString(fmt.Sprintf("🎒 **Supplies:** %.1f / %.1f SU _(burn %.1f/day → ~%d days left)_\n", + exp.Supplies.Current, exp.Supplies.Max, currentBurn(exp), days)) + b.WriteString(fmt.Sprintf("⏰ **Threat:** %d / 100 — %s\n", + exp.ThreatLevel, threatThresholdLabel(exp.ThreatLevel, exp.SiegeMode))) + if exp.TemporalStack != 0 { + b.WriteString(fmt.Sprintf("🌡 **Zone stack:** %d\n", exp.TemporalStack)) + } + } else { + b.WriteString(fmt.Sprintf("🎒 **Supplies:** ~%d day%s left\n", + days, plural(days))) + b.WriteString(fmt.Sprintf("⏰ **Threat:** %s\n", + threatThresholdLabel(exp.ThreatLevel, exp.SiegeMode))) } if exp.Camp != nil && exp.Camp.Active { b.WriteString(fmt.Sprintf("⛺ **Camp:** %s (room %d)\n", exp.Camp.Type, exp.Camp.RoomIndex+1)) } state := supplyDepletion(exp.Supplies) if state != SupplyNormal { - b.WriteString(fmt.Sprintf("⚠ **%s** — roll modifier %d\n", - depletionLabel(state), supplyRollModifier(state))) + if debug { + b.WriteString(fmt.Sprintf("⚠ **%s** — roll modifier %d\n", + depletionLabel(state), supplyRollModifier(state))) + } else { + b.WriteString(fmt.Sprintf("⚠ **%s** — you're slower and clumsier than usual.\n", + depletionLabel(state))) + } } b.WriteString(fmt.Sprintf("\nStarted: %s Last activity: %s", exp.StartDate.Format("2006-01-02 15:04"), diff --git a/internal/plugin/dnd_setup.go b/internal/plugin/dnd_setup.go index 5ca2929..7348cf5 100644 --- a/internal/plugin/dnd_setup.go +++ b/internal/plugin/dnd_setup.go @@ -610,16 +610,26 @@ func renderConfirmPreview(c *DnDCharacter) string { func renderSetupComplete(c *DnDCharacter) string { ri, _ := raceInfo(c.Race) ci, _ := classInfo(c.Class) + nextLine := "Use `!sheet` anytime to review. `!zone list` to head out, or `!expedition list` for a longer run." + if isSpellcaster(c) { + // Casters get auto-granted spells on character creation but no + // in-game discovery prompt for them. Surface !spells / !cast so + // they can actually find their kit. Long-rest refresh is the + // other piece they need to know, but that becomes obvious the + // moment they look at !spells. + nextLine = "Use `!sheet` anytime to review. `!spells` lists what you can cast and `!cast ` does the deed. `!zone list` heads out; `!expedition list` is the longer run." + } return fmt.Sprintf( "⚔️ **Character Sheet Forged**\n\n"+ "You are a **Level %d %s %s**.\n"+ " HP %d/%d AC %d\n"+ " STR %d DEX %d CON %d INT %d WIS %d CHA %d\n\n"+ "_%s_\n\n"+ - "Use `!sheet` anytime to review. `!zone list` to head out, or `!expedition list` for a longer run.", + "%s", c.Level, ri.Display, ci.Display, c.HPCurrent, c.HPMax, c.ArmorClass, c.STR, c.DEX, c.CON, c.INT, c.WIS, c.CHA, ri.Passive, + nextLine, ) } diff --git a/internal/plugin/dnd_sheet.go b/internal/plugin/dnd_sheet.go index 8c4e487..cc5eea0 100644 --- a/internal/plugin/dnd_sheet.go +++ b/internal/plugin/dnd_sheet.go @@ -167,9 +167,11 @@ func renderDnDSheet(c *DnDCharacter, adv *AdventureCharacter, meta *PlayerMeta, } } - // Attunements (re-using adventure_treasures per v1.1 §7.4) + // Treasure bonuses (re-using adventure_treasures per v1.1 §7.4). + // Player-facing label kept distinct from the magic-item "bond" vocab + // so the sheet doesn't read as two competing attunement systems. if len(treasures) > 0 { - b.WriteString("\n**Attunements** (treasures)\n") + b.WriteString("\n**Treasures**\n") // Group by treasure_key — one treasure can have multiple bonuses. byKey := map[string][]AdvTreasureBonus{} var keys []string diff --git a/internal/plugin/dnd_spells_data.go b/internal/plugin/dnd_spells_data.go index dad7205..a4d770c 100644 --- a/internal/plugin/dnd_spells_data.go +++ b/internal/plugin/dnd_spells_data.go @@ -413,6 +413,24 @@ func buildSpellList() []SpellDefinition { {ID: "druidcraft", Name: "Druidcraft", Level: 0, School: "transmutation", Classes: []DnDClass{ClassDruid}, Effect: EffectUtility, CastTime: CastAction, AOE: true, Description: "Whisper to the spirits and they whisper back — usually about tomorrow's weather, sometimes a tiny gust, sometimes a sapling. Mostly harmless theatre."}, + {ID: "eldritch_blast", Name: "Eldritch Blast", Level: 0, School: "evocation", + Classes: []DnDClass{ClassWarlock}, Effect: EffectDamageAttack, CastTime: CastAction, + AttackRoll: true, DamageDice: "1d10", DamageType: "force", + Description: "A lance of pact-fed force punches out of your palm and into a target. Your patron's idea of a polite greeting."}, + {ID: "find_familiar", Name: "Find Familiar", Level: 1, School: "conjuration", + Classes: []DnDClass{ClassMage}, Effect: EffectBuffSelf, CastTime: CastRitual, + Description: "Call up a small spirit-companion in animal shape — cat, raven, frog, the usual. It scouts, fetches, and judges you silently."}, + {ID: "clairvoyance", Name: "Clairvoyance", Level: 3, School: "divination", + Classes: []DnDClass{ClassMage, ClassCleric, ClassBard, ClassSorcerer, ClassWarlock}, + Effect: EffectBuffSelf, CastTime: CastAction, Concentration: true, MaterialCost: 100, + Description: "Plant an invisible eye-or-ear in a place you know or can describe, and watch the world through it from afar."}, + {ID: "glyph_of_warding", Name: "Glyph of Warding", Level: 3, School: "abjuration", + Classes: []DnDClass{ClassMage, ClassCleric, ClassBard}, Effect: EffectDamageSave, CastTime: CastAction, + SaveStat: "DEX", DamageDice: "5d8", DamageType: "thunder", MaterialCost: 200, AOE: true, + Description: "Etch a hidden rune on a surface or in a sealed object. Whoever trips the trigger eats the blast — not you."}, + {ID: "teleportation_circle", Name: "Teleportation Circle", Level: 5, School: "conjuration", + Classes: []DnDClass{ClassMage, ClassBard, ClassSorcerer}, Effect: EffectUtility, CastTime: CastAction, + Description: "Trace a sigil-circle on the ground that links to a far-off permanent circle you know. Step through and arrive there."}, {ID: "light", Name: "Light", Level: 0, School: "evocation", Classes: []DnDClass{ClassMage, ClassCleric, ClassBard, ClassSorcerer}, Effect: EffectUtility, CastTime: CastAction, AOE: true, Description: "Tap an object and it glows like a friendly torch. Easy to carry, harder to lose."}, diff --git a/internal/plugin/dnd_spells_prose_test.go b/internal/plugin/dnd_spells_prose_test.go index 4545f3d..dd54631 100644 --- a/internal/plugin/dnd_spells_prose_test.go +++ b/internal/plugin/dnd_spells_prose_test.go @@ -30,6 +30,17 @@ func TestSpellDescriptionsAreJargonFree(t *testing.T) { {"hit points equal to", regexp.MustCompile(`(?i)hit points equal to`)}, {"NdN dice notation", regexp.MustCompile(`\b\d+d\d+\b`)}, {"bare die notation", regexp.MustCompile(`\bd(4|6|8|10|12|20|100)\b`)}, + // Catch SRD-import placeholders that have leaked through past + // overlays. "Whatever" was the eldritch_blast tell; the truncation + // patterns caught water_walk/druidcraft/light in earlier audits. + // "Whatever" placeholder used by the SRD importer when a field + // went missing. Allow mid-prose "Whatever's"/"Whatever it" + // (legit contraction / sentence-start) by requiring a following + // space-then-lowercase, which is the signature of the bug. + {"Whatever placeholder", regexp.MustCompile(`\bWhatever [a-z]`)}, + {"trailing ellipsis", regexp.MustCompile(`(?:\.\.\.|…)\s*$`)}, + {"trailing truncated word", regexp.MustCompile(`\b[a-z]{1,3}(?:\.\.\.|…)\s*$`)}, + {"empty size phrase", regexp.MustCompile(`(?i)\bno larger than in any\b`)}, } for id, s := range dndSpellRegistry { diff --git a/internal/plugin/dnd_subclass_combat.go b/internal/plugin/dnd_subclass_combat.go index beb3fe1..66e7bb9 100644 --- a/internal/plugin/dnd_subclass_combat.go +++ b/internal/plugin/dnd_subclass_combat.go @@ -1,7 +1,5 @@ package plugin -import "math" - // Phase 10 SUB2a — subclass combat hooks. // // applySubclassPassives layers subclass-driven flags onto CombatModifiers @@ -791,8 +789,10 @@ func init() { Description: "Knock the enemy's weapon aside: their damage is reduced for the rest of the fight (consumes 1 superiority die).", Apply: func(c *DnDCharacter, mods *CombatModifiers) { // 25% incoming-damage cut models a fighter who is now - // swinging an improvised weapon at -d4-ish. - mods.DamageReduct = math.Max(mods.DamageReduct, 0.25) + // swinging an improvised weapon at -d4-ish. DamageReduct + // is multiplicative (1.0 neutral, lower = less damage), + // so multiply down. + mods.DamageReduct *= 0.75 }, } dndActiveAbilities["menacing_attack"] = DnDAbility{ @@ -814,7 +814,8 @@ func init() { Resource: "superiority", Description: "Set yourself for incoming blows — physical damage taken is sharply reduced (consumes 1 superiority die).", Apply: func(c *DnDCharacter, mods *CombatModifiers) { - mods.DamageReduct = math.Max(mods.DamageReduct, 0.40) + // 40% reduction; multiplicative (see disarming_attack note). + mods.DamageReduct *= 0.60 }, } diff --git a/internal/plugin/dnd_subclass_combat_test.go b/internal/plugin/dnd_subclass_combat_test.go index bd9100d..69665f6 100644 --- a/internal/plugin/dnd_subclass_combat_test.go +++ b/internal/plugin/dnd_subclass_combat_test.go @@ -446,6 +446,45 @@ func TestTripAttack_ApplySetsEnemySkipFirst(t *testing.T) { } } +func TestDisarmingAttack_ApplyMultipliesDamageReduct(t *testing.T) { + // Regression: this used to set DamageReduct via math.Max(_, 0.25), + // which against the default 1.0 was a no-op. Should multiply down. + ab, ok := dndActiveAbilities["disarming_attack"] + if !ok { + t.Fatal("disarming_attack not registered") + } + mods := &CombatModifiers{DamageReduct: 1.0} + ab.Apply(&DnDCharacter{Class: ClassFighter, Subclass: SubclassBattleMaster, Level: 5}, mods) + if mods.DamageReduct >= 1.0 { + t.Errorf("Disarming Attack DamageReduct = %.3f, want < 1.0", mods.DamageReduct) + } + if mods.DamageReduct < 0.74 || mods.DamageReduct > 0.76 { + t.Errorf("Disarming Attack DamageReduct = %.3f, want ~0.75", mods.DamageReduct) + } + // Stacks multiplicatively with prior reductions. + mods2 := &CombatModifiers{DamageReduct: 0.90} + ab.Apply(&DnDCharacter{Class: ClassFighter, Subclass: SubclassBattleMaster, Level: 5}, mods2) + if mods2.DamageReduct >= 0.90 { + t.Errorf("Disarming Attack should stack: got %.3f, want < 0.90", mods2.DamageReduct) + } +} + +func TestParry_ApplyMultipliesDamageReduct(t *testing.T) { + // Regression: math.Max(_, 0.40) against default 1.0 was a no-op. + ab, ok := dndActiveAbilities["parry"] + if !ok { + t.Fatal("parry not registered") + } + mods := &CombatModifiers{DamageReduct: 1.0} + ab.Apply(&DnDCharacter{Class: ClassFighter, Subclass: SubclassBattleMaster, Level: 5}, mods) + if mods.DamageReduct >= 1.0 { + t.Errorf("Parry DamageReduct = %.3f, want < 1.0", mods.DamageReduct) + } + if mods.DamageReduct < 0.59 || mods.DamageReduct > 0.61 { + t.Errorf("Parry DamageReduct = %.3f, want ~0.60", mods.DamageReduct) + } +} + func TestRally_ApplySetsHealItem(t *testing.T) { ab := dndActiveAbilities["rally"] mods := &CombatModifiers{DamageReduct: 1.0} diff --git a/internal/plugin/dnd_zone_cmd.go b/internal/plugin/dnd_zone_cmd.go index 575ca0c..97bf09a 100644 --- a/internal/plugin/dnd_zone_cmd.go +++ b/internal/plugin/dnd_zone_cmd.go @@ -82,8 +82,8 @@ func zoneHelpText() string { b.WriteString("`!zone advance` — resolve the current room and move on\n") b.WriteString("`!zone go ` — at a fork, take path #n\n") b.WriteString("`!zone abandon` — end the active run (no rewards)\n") - b.WriteString("`!zone taunt` — poke TwinBee (mood −10)\n") - b.WriteString("`!zone compliment` — flatter TwinBee (mood +5)\n") + b.WriteString("`!zone taunt` — poke TwinBee (they'll remember)\n") + b.WriteString("`!zone compliment` — flatter TwinBee (they'll like that)\n") b.WriteString("`!zone lore` — TwinBee shares zone history") return b.String() } diff --git a/internal/plugin/expedition_ambient.go b/internal/plugin/expedition_ambient.go index ce436b5..eae11d3 100644 --- a/internal/plugin/expedition_ambient.go +++ b/internal/plugin/expedition_ambient.go @@ -311,12 +311,13 @@ func (p *AdventurePlugin) applyAmbientEffect(e *Expedition, ev ambientEvent) str if err := applyThreatDelta(e.ID, 1, "ambient: distant noise"); err != nil { slog.Warn("expedition: ambient threat delta", "expedition", e.ID, "err", err) } - return "Threat +1" + // Verb-form footer — "Threat +N" exposed the hidden meter. + return "The dungeon listens a little harder." case "faction_whisper": if err := applyThreatDelta(e.ID, 2, "ambient: faction whisper"); err != nil { slog.Warn("expedition: ambient threat delta", "expedition", e.ID, "err", err) } - return "Threat +2" + return "Something out there is paying attention now." case "pack_rat": drain := float32(0.2) + float32(rng.IntN(2))*float32(0.1) // 0.2 or 0.3 ns := e.Supplies @@ -328,7 +329,10 @@ func (p *AdventurePlugin) applyAmbientEffect(e *Expedition, ev ambientEvent) str slog.Warn("expedition: ambient supply drain", "expedition", e.ID, "err", err) return "" } - return fmt.Sprintf("Supplies −%.1f SU", drain) + // Don't surface the raw SU number — it's a hidden meter and + // "SU" doesn't appear in player vocab anywhere else. + _ = drain + return "A little less in the pack than there was." case "lucky_find": coins := 1 + rng.IntN(4) // 1d4 if p.euro != nil { diff --git a/internal/plugin/magic_items_gameplay.go b/internal/plugin/magic_items_gameplay.go index 6de82b7..b09f810 100644 --- a/internal/plugin/magic_items_gameplay.go +++ b/internal/plugin/magic_items_gameplay.go @@ -6,6 +6,7 @@ import ( "math/rand/v2" "sort" "strings" + "sync" "time" "gogobee/internal/db" @@ -28,25 +29,29 @@ import ( // ── Rarity / loot indexing ────────────────────────────────────────────────── -// magicItemsByRarity buckets the registry by rarity. Built once, lazily. -var magicItemsByRarityCache map[DnDRarity][]MagicItem +// magicItemsByRarity buckets the registry by rarity. Built once via +// sync.Once — concurrent loot rolls on cold start would otherwise race +// on the cache assignment below. +var ( + magicItemsByRarityOnce sync.Once + magicItemsByRarityCache map[DnDRarity][]MagicItem +) func magicItemsByRarity() map[DnDRarity][]MagicItem { - if magicItemsByRarityCache != nil { - return magicItemsByRarityCache - } - idx := make(map[DnDRarity][]MagicItem) - ids := make([]string, 0, len(magicItemRegistry)) - for id := range magicItemRegistry { - ids = append(ids, id) - } - sort.Strings(ids) // deterministic bucket order - for _, id := range ids { - mi := magicItemRegistry[id] - idx[mi.Rarity] = append(idx[mi.Rarity], mi) - } - magicItemsByRarityCache = idx - return idx + magicItemsByRarityOnce.Do(func() { + idx := make(map[DnDRarity][]MagicItem) + ids := make([]string, 0, len(magicItemRegistry)) + for id := range magicItemRegistry { + ids = append(ids, id) + } + sort.Strings(ids) // deterministic bucket order + for _, id := range ids { + mi := magicItemRegistry[id] + idx[mi.Rarity] = append(idx[mi.Rarity], mi) + } + magicItemsByRarityCache = idx + }) + return magicItemsByRarityCache } // pickMagicItemForRarity returns a uniform-random registry item of the given @@ -541,12 +546,26 @@ func (p *AdventurePlugin) resolveMagicEquipReply(ctx MessageContext, interaction attune = true } } - if err := equipMagicItem(ctx.Sender, mi.Slot, mi.ID, attune); err != nil { + // Remove the inventory row FIRST, then equip. If equip fails after the + // remove succeeded, restore inventory. Doing it in the other order + // meant a transient DB error on remove left the item both equipped + // *and* still in inventory — a free duplication. + if err := removeAdvInventoryItem(it.ID); err != nil { + slog.Error("magic-item: failed to remove from inventory before equip", + "user", ctx.Sender, "item", mi.ID, "err", err) return p.SendDM(ctx.Sender, "Failed to equip that item.") } - if err := removeAdvInventoryItem(it.ID); err != nil { - slog.Error("magic-item: failed to remove equipped item from inventory", - "user", ctx.Sender, "item", mi.ID, "err", err) + if err := equipMagicItem(ctx.Sender, mi.Slot, mi.ID, attune); err != nil { + // Roll back: try to put the item back in inventory so the player + // doesn't lose it. Best-effort; log if the rollback also fails. + restored := magicItemSell(mi) + restored.Value = it.Value + restored.SkillSource = "magic_item:" + mi.ID + if rbErr := addAdvInventoryItem(ctx.Sender, restored); rbErr != nil { + slog.Error("magic-item: equip failed AND inventory rollback failed", + "user", ctx.Sender, "item", mi.ID, "equip_err", err, "rollback_err", rbErr) + } + return p.SendDM(ctx.Sender, "Failed to equip that item.") } var sb strings.Builder @@ -554,10 +573,10 @@ func (p *AdventurePlugin) resolveMagicEquipReply(ctx MessageContext, interaction mi.Name, mi.Slot, magicItemEffectSummary(mi))) switch { case mi.Attunement && attune: - sb.WriteString(fmt.Sprintf("\nAttuned (%d/%d attunement slots used).", + sb.WriteString(fmt.Sprintf("\nBonded (%d/%d bond slots used).", countAttunedMagicItems(equipped)+1, dndMagicItemAttuneLimit)) case mi.Attunement && atCap: - sb.WriteString(fmt.Sprintf("\n⚠️ All %d attunement slots are full — it's worn but **inert** until you free one (`!adventure equip-magic` to swap).", + sb.WriteString(fmt.Sprintf("\n⚠️ All %d bond slots are full — it's worn but **inert** until you free one (`!adventure equip-magic` to swap).", dndMagicItemAttuneLimit)) } return p.SendDM(ctx.Sender, sb.String())