fix(eloot.lic): v2.11.1 locksmith pool/town locksmith fixes + Locksmith Priority option

[mrhoribu]

Summary

Started as a fix for the v2.11.0 locksmith pool "sell to free space and resume" recovery
not firing for loot_specials items. Grew to cover two more pre-existing bugs found while
tracing it (double finalization on recovery, and boxes the town locksmith always refuses
being routed to it anyway), a mark-status guard on trash/drop, and — per review discussion
on this PR — a new Locksmith Priority option for when both routing checkboxes are on.

box_loot drains loot_specials before loot_regular, and only loot_regular was wired
to the pool recovery, so orbs, cursed items, keepers, and uncommon weapons/armor still
paused the script mid-return with the box and the stuck item in hand.

Room looting and critter-bag looting are unchanged throughout.

Reported symptom

From a player's eloot.log (15h session, v2.11.0, debug on). Three Failed to store /
Pausing the script events, all with the same caller chain:

Time	Item	Chain	Room
07:42:08	some ripped yellow socks	box_loot(4858) -> loot_specials(5182) -> single_drag(3701)	2400 (locksmith pool)
14:31:10	singed white oak strongbox	Loot.room(5259) -> loot_specials(5182) -> single_drag(3701)	32493 (field)
21:47:41	heavy quartz orb	box_loot(4858) -> loot_specials(5182) -> single_drag(3701)	2400 (locksmith pool)

The recovery never ran once in the whole session -- zero Selling to free space messages
-- despite 78 stow_box_item entries during the same pool returns.

A second report surfaced a related defect: case boxes, which only the locksmith pool or
a player locksmith can open, were still carried to town on every sell run when routing
sent boxes there (gem bounty setting, pool disabled, F2P, tipping unset, or deposit
failure) -- a silver withdrawal, a trip, refused bells, and the box never opening.

Root cause

1. Missing call-site wiring. loot_specials took only objs, so it had no box
context and called Inventory.single_drag(thing) -- default on_full: :pause. The
recovery hangs off Loot.stow_box_item, reachable only from loot_regular.

2. Double finalization on recovery (pre-existing, v2.11.0). loot_regular's docstring
promised :recovered so "the caller should stop iterating," but all three of its recovery
exits were bare return -- returning nil. box_loot discarded the value and ran
Sell.save_trash_box anyway, after the nested recovery had already trashed or stowed the
box.

3. Box routing ignores per-box openability (pre-existing). process_boxes decided
pool-vs-town for the entire list at once, so any route that sent case boxes to town
produced a permanent no-op loop.

Changes

scripts/eloot.lic

• loot_specials takes box:, location:, data:, sell_recovered: keyword args, all
  defaulted, and stows via Loot.stow_box_item when a box is given.
• loot_regular's recovery exits now return :recovered instead of nil, matching the
  docstring; box_loot/box_loot_ground skip finalization on :recovered.
• Sell.town_openable? (new): false for case nouns (POOL_ONLY_NOUNS) and false for any
  box name Sell.remember_town_refusal recorded this session.
• process_boxes partitions on that predicate and only ever hands the town locksmith boxes
  it can open, guarded by Sell.pool_available?.
• locksmith_open widens its activator match to catch the NPC ignoring a box that is
  verifiably in hand, recording a session-scoped refusal so an unrecognized box type costs
  one attempt instead of every run.
• ELoot.marked_unsellable? / ELoot.toss: trash/drop now checks in-game mark status
  before ever issuing trash or drop, and refuses to toss anything marked -- consolidates 5
  duplicated trash/drop-and-check-hand call sites into one helper.
• New: locksmith_priority setting ('pool' default / 'locksmith'), exposed as a
  GtkComboBoxText dropdown in the Locksmithing frame (Pool First / Locksmith First),
  sensitive only when both "Use locksmith pool" and "Use town locksmith" are checked.
  "Locksmith First" routes town-openable boxes to town first via the new
  Sell.route_town_then_pool helper, then pools whatever's left (including anything a
  runtime town refusal knocks back out). "Pool First" is unchanged and remains the
  default -- no behavior change for anyone who doesn't touch the new setting. The existing
  locksmith_when_gem_bounty override is a narrower, more specific routing decision and
  keeps its current order regardless of this preference.
• Changelog entry updated in place (no version bump -- v2.11.1 is still unreleased on this
  branch) and trimmed to match the terser style of older entries; content unchanged.

spec/scripts/eloot_spec.rb -- 41 new examples across five describe blocks (32 from the
original fix set, plus 9 for Locksmith Priority), following the existing
extract-from-source-and-eval pattern. Sell.route_town_then_pool has no navigation of its
own, so it's exercised for real against a small stand-in for Sell/ELoot rather than
asserted structurally.

Not in scope

The no-disk case still pauses. pool_full_recovery? requires has_disk, since
pool_sell_recovery has to park the box somewhere to free both hands to sell. Closing it
needs a staging decision with real player cost (ground/re-pool/carry) -- a product
decision, deliberately left alone.

always_check_pool isn't honored on the Locksmith First path -- consistent with the
existing gem-bounty branch, which also doesn't check that setting; pool is only visited if
boxes remain after the town step.

Testing

bundle exec rspec spec/scripts/eloot_spec.rb -- 81 examples, 0 failures.

Regression, runnable spec subset (spec/autostart spec/bigshot spec/log spec/rofl-puzzles spec/scripts): 149 -> 190 examples, 0 failures.

The routing was also replayed against the reported scenario with the shipped
process_boxes body and stubbed collaborators (2 cases + gem bounty; case + steel chest;
cases-only with pool unavailable; mixed with pool unavailable), all matching expected
routing.

For the new priority option specifically, Sell.route_town_then_pool was verified by
deliberately breaking it three ways (dropped the town_openable? filter, skipped the box
refresh before pooling, dropped the pool_available? guard) and confirming each broke a
specific new example, then reverting -- so the new specs aren't just matching whatever the
code happens to say.

Also clean: ruby -c on both changed files, ASCII-only byte scan (0 bytes > 127), RuboCop
1.39.0 with the vendored Custom/AsciiOnlySource cop -- no offenses.

Reviewer notes

• Scope flag. This PR now spans a recovery-wiring bug, two pre-existing routing bugs,
  a mark-status fix + refactor, and a feature addition. Happy to split the Locksmith
  Priority option into its own PR if you'd rather keep this one to the bug-fix set --
  flagging it since it's the one piece that isn't a fix.
• POOL_ONLY_NOUNS is a second place to maintain. gameobj-data.xml already
  enumerates these box descriptors. Raised separately rather than expanding this PR.
• Flagged, not fixed: pool_full_recovery?'s has_disk test is looser than what
  single_drag_box actually requires; not reachable in the logged session, worth a
  separate look.
• Sandbox caveat: a few spec subdirectories couldn't load in my environment (missing
  gems); none touch eloot.lic, please confirm against a full CI run.

Checklist

• Version bumped and changelog entry added in the script header
• Specs added; verified failing on master and passing on this branch
• Zero regression on the runnable spec subset
• RuboCop clean, ASCII-only source
• ruby -c clean
• Full CI spec suite green -- please confirm
• Confirm scope decision on the two pre-existing fixes (single commit vs. split)
• Confirm case is the only town-unopenable noun (community/Discord check)

[coderabbitai]

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 60.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title identifies the main locksmith pool recovery fix, but it also mentions changes not shown in the provided summary.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/eloot.lic`:
- Around line 5261-5264: Return an explicit :recovered status from loot_specials
when pool recovery has already re-looted and finalized the box instead of
returning []. In scripts/eloot.lic:5261-5264, update that recovery branch; in
scripts/eloot.lic:4885-4887, make box_loot return before Sell.save_trash_box
when the result is :recovered; in scripts/eloot.lic:4930-4931, skip to the next
ground box for the same result. In spec/scripts/eloot_spec.rb:241-245, assert
the recovery status and add caller coverage preventing duplicate box
finalization.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 374747ae-fc53-4f49-86ba-5d3ce946b8e4

📥 Commits

Reviewing files that changed from the base of the PR and between 74340cb and 55733b8.

📒 Files selected for processing (2)

• scripts/eloot.lic
• spec/scripts/eloot_spec.rb