Ship-audit P0/P1 sweep on open5e: prose, combat math, UX hygiene

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) <noreply@anthropic.com>
This commit is contained in:
prosolis
2026-05-15 14:43:11 -07:00
parent ba84d38979
commit f424300d87
12 changed files with 190 additions and 50 deletions

View File

@@ -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

View File

@@ -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)

View File

@@ -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))
}
// 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),
estimateDays(exp.Supplies.Current, currentBurn(exp))))
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 {
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"),

View File

@@ -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 <spell>` 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,
)
}

View File

@@ -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

View File

@@ -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."},

View File

@@ -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 {

View File

@@ -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
},
}

View File

@@ -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}

View File

@@ -82,8 +82,8 @@ func zoneHelpText() string {
b.WriteString("`!zone advance` — resolve the current room and move on\n")
b.WriteString("`!zone go <n>` — 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()
}

View File

@@ -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 {

View File

@@ -6,6 +6,7 @@ import (
"math/rand/v2"
"sort"
"strings"
"sync"
"time"
"gogobee/internal/db"
@@ -28,13 +29,16 @@ 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
}
magicItemsByRarityOnce.Do(func() {
idx := make(map[DnDRarity][]MagicItem)
ids := make([]string, 0, len(magicItemRegistry))
for id := range magicItemRegistry {
@@ -46,7 +50,8 @@ func magicItemsByRarity() map[DnDRarity][]MagicItem {
idx[mi.Rarity] = append(idx[mi.Rarity], mi)
}
magicItemsByRarityCache = idx
return 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())