diff --git a/internal/plugin/adventure_robbie.go b/internal/plugin/adventure_robbie.go index 4e70a80..0b6e9f5 100644 --- a/internal/plugin/adventure_robbie.go +++ b/internal/plugin/adventure_robbie.go @@ -173,13 +173,9 @@ func (p *AdventurePlugin) robbieVisitPlayer(userID id.UserID, displayName string // frozen legacy CombatLevel — that snapshots at 1–3 once D&D setup // completes, so reading it here would peg every gift at tier 1. tier := robbieGiftTier(arenaDnDLevelOrZero(userID)) - for range robbieGiftCount(char.RobbieVisitCount, len(takenItems)) { - gifts := consumableCache(tier, 1) - if len(gifts) == 0 { - break - } - if err := addAdvInventoryItem(userID, gifts[0]); err == nil { - leftGifts = append(leftGifts, gifts[0]) + for _, gift := range consumableCache(tier, robbieGiftCount(char.RobbieVisitCount, len(takenItems))) { + if err := addAdvInventoryItem(userID, gift); err == nil { + leftGifts = append(leftGifts, gift) } } _ = saveAdvCharacter(char) diff --git a/internal/plugin/adventure_shop.go b/internal/plugin/adventure_shop.go index ac783df..ec12812 100644 --- a/internal/plugin/adventure_shop.go +++ b/internal/plugin/adventure_shop.go @@ -845,7 +845,13 @@ func (p *AdventurePlugin) advSellAll(userID id.UserID) string { var keptConsumable int var keptMagic int for _, item := range items { - if item.Type == "MasterworkGear" || item.Type == "ArenaGear" || item.Type == "card" { + // Keys and tools ride along with the special gear here for the same + // reason Robbie won't take them: both are bought or found precisely so a + // door can be opened *later*, and `sell all` is a bulk-loot verb the + // player fires after every haul without reading the list. Turning one + // into €300 silently deletes the unlock it was carried for. + switch item.Type { + case "MasterworkGear", "ArenaGear", "card", "key", thievesToolsItemType: keptSpecial++ continue } @@ -1079,7 +1085,7 @@ func (p *AdventurePlugin) resolveShopSupplyChoice(ctx MessageContext, interactio // Thieves' tools sit on the supplies shelf but are not a ConsumableDef: // the combat engine scans inventory against that table and would happily // spend them mid-fight. They get their own branch and their own item type. - if containsFold(thievesToolsName, reply) || containsFold("thieves tools", reply) { + if isThievesToolsReply(reply) { return p.buyThievesTools(ctx, interaction) } diff --git a/internal/plugin/dnd_expedition_cmd.go b/internal/plugin/dnd_expedition_cmd.go index 0bdb41b..07f8d88 100644 --- a/internal/plugin/dnd_expedition_cmd.go +++ b/internal/plugin/dnd_expedition_cmd.go @@ -907,12 +907,14 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf ranked := rankForkOptions(g, run, pf) note := "autopilot took the most promising path" + var spendTool int64 if len(ranked) == 0 { - picked, ok := p.autoPickWithTools(run, pf) + picked, toolID, ok := p.autoPickWithTools(run, pf) if !ok { return false } ranked = []pendingChoice{picked} + spendTool = toolID note = "autopilot spent " + thievesToolsName + " on the only way forward" } chosen := ranked[0] @@ -922,6 +924,14 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf "user", run.UserID, "run", run.RunID, "err", err) return false } + // The door is behind us, so now the set is actually used up. Billing before + // the advance would charge the player for a move that failed — and the + // caller's backtrack would then clear the fork the tools just paid for. + if spendTool != 0 { + if err := removeAdvInventoryItem(spendTool); err != nil { + slog.Warn("expedition: autopilot tools spend", "user", run.UserID, "err", err) + } + } fireGraphRegionTransition(run.UserID, g.Nodes[run.CurrentNode], g.Nodes[chosen.To]) if exp != nil { _ = appendExpeditionLog(exp.ID, exp.CurrentDay, "narrative", @@ -930,31 +940,32 @@ func (p *AdventurePlugin) autoPickStaleFork(exp *Expedition, run *DungeonRun, pf return true } -// autoPickWithTools spends one set of thieves' tools to open a fork where every -// route is locked, so a bad Perception roll can't quietly end an expedition the -// player paid days into. It only ever fires when there is no free route left — -// the player's tools are their own, and the autopilot does not get to burn them -// for convenience. -func (p *AdventurePlugin) autoPickWithTools(run *DungeonRun, pf *pendingFork) (pendingChoice, bool) { - owner := id.UserID(run.UserID) - toolID, ok := findThievesTools(owner) +// autoPickWithTools finds a route a set of thieves' tools could open when every +// option on the fork is locked, so a bad Perception roll can't quietly end an +// expedition the player paid days into. It only ever fires when there is no free +// route left — the player's tools are their own, and the autopilot does not get +// to burn them for convenience. +// +// It reports the route and the inventory row to spend, but does not spend it: +// the caller charges the player only once the move has actually committed. +func (p *AdventurePlugin) autoPickWithTools(run *DungeonRun, pf *pendingFork) (pendingChoice, int64, bool) { + toolID, ok := findThievesTools(id.UserID(run.UserID)) if !ok { - return pendingChoice{}, false + return pendingChoice{}, 0, false } for i := range pf.Options { if pf.Options[i].Unlocked || !pickableLock(pf.Options[i].Lock) { continue } - if err := removeAdvInventoryItem(toolID); err != nil { - slog.Warn("expedition: autopilot tools spend", "user", run.UserID, "err", err) - return pendingChoice{}, false - } - pf.Options[i].Unlocked = true - pf.Options[i].Reason = "opened with " + thievesToolsName - _ = writePendingFork(run.RunID, *pf) - return pf.Options[i], true + // Local copy only — advanceZoneRunNode clears node_choices on success, + // and on failure the fork must stay as locked as the player left it + // rather than reading "open" for a set nobody paid for. + chosen := pf.Options[i] + chosen.Unlocked = true + chosen.Reason = "opened with " + thievesToolsName + return chosen, toolID, true } - return pendingChoice{}, false + return pendingChoice{}, 0, false } // backtrackFromDeadFork walks the run back one room when a fork has no route @@ -963,13 +974,17 @@ func (p *AdventurePlugin) autoPickWithTools(run *DungeonRun, pf *pendingFork) (p // losing days of progress to a die roll they never saw and could not answer. // Backtracking at least returns them to a room with other exits. // -// Returns false at the entry node, where there is nowhere behind to go. +// Returns false at the entry node, where there is nowhere behind to go, and +// wherever no fallback room would actually help — see backtrackTarget. func (p *AdventurePlugin) backtrackFromDeadFork(exp *Expedition, run *DungeonRun) bool { - idx := pathIndexOf(run.VisitedNodes, run.CurrentNode) - if idx <= 0 { + g, ok := loadZoneGraph(run.ZoneID) + if !ok { + return false + } + target, ok := backtrackTarget(g, run) + if !ok { return false } - target := run.VisitedNodes[idx-1] // Clear the fork first: it belongs to the node being left, and both // `!zone advance` and `!zone go` would otherwise resolve a prompt pointing @@ -989,6 +1004,35 @@ func (p *AdventurePlugin) backtrackFromDeadFork(exp *Expedition, run *DungeonRun return true } +// backtrackTarget picks the room a dead fork falls back to: the most recently +// entered visited node that is genuinely joined to the current one by an edge +// and that still offers a way on other than the sealed room. +// +// Both halves are load-bearing. VisitedNodes is a first-entry ordered *set*, not +// a path stack (see appendVisited) — after any earlier backtrack the entry +// before CurrentNode can sit on a completely different branch, so stepping to it +// blind teleports the party across the map. `!revisit` refuses exactly that move +// via adjacentNodes, and the autopilot has no business doing what the player is +// forbidden from doing. The second half stops the other failure: falling back +// into a corridor whose only exit is the fork we just fled from just walks +// straight back in — and the lock rolls are seeded per (run, edge), so the +// result is identical every time. That is an infinite loop, not a recovery. +func backtrackTarget(g ZoneGraph, run *DungeonRun) (string, bool) { + adj := adjacentNodes(g, run.CurrentNode) + for i := pathIndexOf(run.VisitedNodes, run.CurrentNode) - 1; i >= 0; i-- { + n := run.VisitedNodes[i] + if !adj[n] { + continue + } + for _, e := range g.outgoingEdges(n) { + if e.To != run.CurrentNode { + return n, true + } + } + } + return "", false +} + func (p *AdventurePlugin) runAutopilotWalk(ctx MessageContext, maxRooms int, compact, inlineBossCombat bool) autopilotWalkResult { exp, err := getActiveExpedition(ctx.Sender) if err != nil { diff --git a/internal/plugin/zone_backtrack_test.go b/internal/plugin/zone_backtrack_test.go new file mode 100644 index 0000000..6b84450 --- /dev/null +++ b/internal/plugin/zone_backtrack_test.go @@ -0,0 +1,60 @@ +package plugin + +import "testing" + +// backtrackGraph — a fork at z.fork with two branches, plus a dead-end spur +// hanging off z.b whose only exit is back into z.b. +// +// z.entry → z.fork → z.a +// → z.b → z.spur +func backtrackGraph() ZoneGraph { + return ZoneGraph{ + Nodes: map[string]ZoneNode{}, + Edges: map[string][]ZoneEdge{ + "z.entry": {{From: "z.entry", To: "z.fork"}}, + "z.fork": {{From: "z.fork", To: "z.a"}, {From: "z.fork", To: "z.b"}}, + "z.b": {{From: "z.b", To: "z.spur"}}, + }, + } +} + +// TestBacktrackTargetRequiresAdjacency is the regression guard for the bug this +// fixes: VisitedNodes is a first-entry ordered set, not a path stack, so the +// entry before CurrentNode can sit on a branch the party never walked from +// here. Stepping to it blind teleports them across the map. +func TestBacktrackTargetRequiresAdjacency(t *testing.T) { + // Walked entry → fork → a, doubled back, then took fork → b. The visited + // set is [entry, fork, a, b]; z.a is *not* joined to z.b. + run := &DungeonRun{ + CurrentNode: "z.b", + VisitedNodes: []string{"z.entry", "z.fork", "z.a", "z.b"}, + } + got, ok := backtrackTarget(backtrackGraph(), run) + if !ok { + t.Fatal("should fall back to the fork") + } + if got != "z.fork" { + t.Errorf("backtrack target = %s, want z.fork (z.a shares no edge with z.b)", got) + } +} + +// TestBacktrackTargetSkipsOneWayCorridor — backing into a room whose only exit +// is the sealed fork walks straight back in, and the lock rolls are seeded per +// (run, edge), so it would do it forever. Refuse instead. +func TestBacktrackTargetSkipsOneWayCorridor(t *testing.T) { + run := &DungeonRun{ + CurrentNode: "z.spur", + VisitedNodes: []string{"z.entry", "z.fork", "z.b", "z.spur"}, + } + if got, ok := backtrackTarget(backtrackGraph(), run); ok { + t.Errorf("backtracked to %s; z.b only leads back to z.spur, so this loops", got) + } +} + +// TestBacktrackTargetAtEntry — nowhere behind the entry node. +func TestBacktrackTargetAtEntry(t *testing.T) { + run := &DungeonRun{CurrentNode: "z.entry", VisitedNodes: []string{"z.entry"}} + if _, ok := backtrackTarget(backtrackGraph(), run); ok { + t.Error("entry node has nowhere to back out to") + } +} diff --git a/internal/plugin/zone_graph_unlock.go b/internal/plugin/zone_graph_unlock.go index b374349..659b426 100644 --- a/internal/plugin/zone_graph_unlock.go +++ b/internal/plugin/zone_graph_unlock.go @@ -41,35 +41,48 @@ func pickableLock(kind string) bool { return kind == string(LockPerception) || kind == string(LockStatCheck) } +// isThievesToolsReply matches a shop reply against the tools. +// +// Deliberately not "is the reply a substring of the name": that predicate is +// true for a bare "s", for "to", and for an empty message body — and this branch +// sits ahead of the consumable list, so a stray keystroke in the Supplies view +// would silently bill the player €600. Match on the item's own words instead. +func isThievesToolsReply(reply string) bool { + return containsFold(reply, "thieves") || containsFold(reply, "thief") || + containsFold(reply, "tools") +} + +// thievesToolsHeld returns the inventory row IDs of every set the player is +// carrying. One read answers both "have they got any" and "how many are left", +// which the unlock flow would otherwise ask the inventory table three times. +func thievesToolsHeld(userID id.UserID) []int64 { + inv, err := loadAdvInventory(userID) + if err != nil { + return nil + } + var ids []int64 + for _, it := range inv { + if strings.EqualFold(it.Name, thievesToolsName) { + ids = append(ids, it.ID) + } + } + return ids +} + // findThievesTools returns the inventory row ID of one set of tools, or ok=false // if the player is carrying none. func findThievesTools(userID id.UserID) (int64, bool) { - inv, err := loadAdvInventory(userID) - if err != nil { + held := thievesToolsHeld(userID) + if len(held) == 0 { return 0, false } - for _, it := range inv { - if strings.EqualFold(it.Name, thievesToolsName) { - return it.ID, true - } - } - return 0, false + return held[0], true } // countThievesTools reports how many sets the player carries, for the "N left" // line after a use. func countThievesTools(userID id.UserID) int { - inv, err := loadAdvInventory(userID) - if err != nil { - return 0 - } - n := 0 - for _, it := range inv { - if strings.EqualFold(it.Name, thievesToolsName) { - n++ - } - } - return n + return len(thievesToolsHeld(userID)) } // zoneCmdUnlock handles `!zone unlock `: spend one set of thieves' tools to @@ -99,12 +112,14 @@ func (p *AdventurePlugin) zoneCmdUnlock(ctx MessageContext, rest string) error { return p.SendDM(ctx.Sender, "No fork pending. Use "+continueHint(ctx.Sender)) } + held := thievesToolsHeld(ctx.Sender) + rest = strings.TrimSpace(rest) if rest == "" { zone := zoneOrFallback(run.ZoneID) return p.SendDM(ctx.Sender, "**Which one?**\n\n"+renderForkPrompt(zone, *pf)+ fmt.Sprintf("\n\n_`!zone unlock ` — spends one set of %s. You carry %d._", - thievesToolsName, countThievesTools(ctx.Sender))) + thievesToolsName, len(held))) } choice := atoiSafe(rest) if choice < 1 || choice > len(pf.Options) { @@ -121,13 +136,12 @@ func (p *AdventurePlugin) zoneCmdUnlock(ctx MessageContext, rest string) error { "🔒 Tools won't help here. %s\n\n_Thieves' tools answer a failed check, not a locked gate._", lockRefusalFor(chosen))) } - toolID, ok := findThievesTools(ctx.Sender) - if !ok { + if len(held) == 0 { return p.SendDM(ctx.Sender, fmt.Sprintf( "🔒 **%s** needs %s and you're carrying none.\n\nLuigi stocks them under `!shop` → Supplies.", chosen.Label, thievesToolsName)) } - if rerr := removeAdvInventoryItem(toolID); rerr != nil { + if rerr := removeAdvInventoryItem(held[0]); rerr != nil { return p.SendDM(ctx.Sender, "Couldn't spend the tools: "+rerr.Error()) } @@ -138,8 +152,8 @@ func (p *AdventurePlugin) zoneCmdUnlock(ctx MessageContext, rest string) error { pf.Options[choice-1].Reason = "opened with " + thievesToolsName _ = writePendingFork(run.RunID, *pf) - left := countThievesTools(ctx.Sender) - header := fmt.Sprintf("🔓 **%s** — picked. _(%s used, %d left)_\n\n", chosen.Label, thievesToolsName, left) + header := fmt.Sprintf("🔓 **%s** — picked. _(%s used, %d left)_\n\n", + chosen.Label, thievesToolsName, len(held)-1) return p.commitForkChoice(ctx, run, pf.Options[choice-1], header) }