Skip to content

fix(bigshot.lic): v5.16.0 looting races eloot on resume when both are paused - #2432

Closed
mrhoribu wants to merge 2 commits into
elanthia-online:docs/bigshot-yard-documentationfrom
mrhoribu:fix/bigshot-looting-watch-pause-race
Closed

mrhoribu wants to merge 2 commits into
elanthia-online:docs/bigshot-yard-documentationfrom
mrhoribu:fix/bigshot-looting-watch-pause-race

Conversation

@mrhoribu

@mrhoribu mrhoribu commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Test plan

  • New regression spec spec/bigshot/looting_watch_spec.rb, following the source-extraction-and-eval technique from spec/bigshot/priority_spec.rb. Uses a Mutex/ConditionVariable + Queue-based stub instead of sleep-based polling, so the race is deterministic:
    • blocks while bigshot is paused even though the loot script is also paused, and only returns once bigshot is unpaused
    • still breaks immediately on the loot script's own pause once bigshot itself was never paused (no behavior change for the common case)
    • kill/box-in-hand/rest-flag handling after the loop unaffected
  • Audited every other Script.paused?/Script.running? call site in bigshot.lic for the same bug shape - none found; this was the only raw polling loop watching another script's state without also checking its own
  • bin/migrate run first (gameobj-data fixtures)
  • bundle exec rspec - full suite passing
  • RBENV_VERSION=4.0.5 rubocop -A - clean

Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a453bf0e-6ced-410b-8a5d-b1dd5cae9f0c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

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

…state

looting_watch only checked the watched script's (eloot's) pause state,
never bigshot's own. When something paused bigshot and eloot back to
back (e.g. ecleanse pausing a script list for a disarm-recovery cast),
looting_watch noticed eloot's pause and returned to its caller
immediately, regardless of whether bigshot itself had also just been
paused. That let bigshot's thread advance to the next corpse's
run_script/Script.kill one iteration early, racing eloot's own
independently-resumed thread with no reliable ordering.

Add Script.current at the top of the loop - the same idiom lich-5 uses
elsewhere (echo, fput, ...) to block on the calling script's own pause
via Script#wait_while_paused! - so bigshot's own pause is checked on
every iteration, not just at entry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mrhoribu
mrhoribu force-pushed the fix/bigshot-looting-watch-pause-race branch from c610a14 to 0a13d45 Compare August 26, 2026 19:19
Keep this fix landing as part of the v5.16.0 release the rest of the
stacked PRs are shipping, rather than a separate v5.16.1. Also rewrites
the v5.16.0 changelog entries (including this one) in plain,
player-facing language instead of implementation detail.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mrhoribu mrhoribu changed the title fix(bigshot.lic): v5.16.1 looting_watch races eloot on resume when both are paused fix(bigshot.lic): v5.16.0 looting races eloot on resume when both are paused Aug 26, 2026
@mrhoribu

Copy link
Copy Markdown
Contributor Author

Superseded by #2433 - same commit, pushed directly to this repo instead of from my fork so it groups into the bigshot PR stack correctly.

@mrhoribu mrhoribu closed this Aug 26, 2026
@mrhoribu
mrhoribu deleted the fix/bigshot-looting-watch-pause-race branch August 26, 2026 19:30
mrhoribu added a commit that referenced this pull request Sep 11, 2026
… paused (#2433)

## Summary
- Fixes a rare timing bug in `looting_watch`: it only checked the
*watched* loot script's pause state, never bigshot's own. When something
(e.g. `ecleanse`) paused bigshot and the loot script back-to-back for a
disarm-recovery cast, `looting_watch` would notice the loot script's
pause and hand control back immediately - regardless of whether bigshot
itself had also just been paused.
- That let bigshot advance one extra loot iteration and kill the loot
script for the *next* corpse while the *current* one was independently
resuming and finishing its own action, with no reliable ordering between
the two. Could skip a loot/skin action or cut one off mid-command. Real
session log reproducing it is in the linked work unit.
- Fix: block on bigshot's own pause at the top of the loop (the same
idiom used elsewhere in the script) so it can't race ahead while paused.
- Landing as part of v5.16.0 alongside the rest of the stack rather than
a separate release, and simplified the v5.16.0 changelog entries to
plain language.
- Stacked on `docs/bigshot-yard-documentation`
(#2417), the tip of the open bigshot PR stack
(#2414 -> #2415 -> #2416 -> #2417 -> this).
- Replaces #2432, which was opened from a fork and so didn't group into
the native stack view; this is the same branch pushed directly to this
repo.

## Test plan
- [x] New regression spec `spec/bigshot/looting_watch_spec.rb`,
following the source-extraction-and-eval technique from
`spec/bigshot/priority_spec.rb`. Uses a Mutex/ConditionVariable +
Queue-based stub instead of sleep-based polling, so the race is
deterministic:
- blocks while bigshot is paused even though the loot script is also
paused, and only returns once bigshot is unpaused
- still breaks immediately on the loot script's own pause once bigshot
itself was never paused (no behavior change for the common case)
  - kill/box-in-hand/rest-flag handling after the loop unaffected
- [x] Audited every other `Script.paused?`/`Script.running?` call site
in bigshot.lic for the same bug shape - none found; this was the only
raw polling loop watching another script's state without also checking
its own
- [x] `bin/migrate` run first (gameobj-data fixtures)
- [x] `bundle exec rspec` - full suite passing
- [x] `RBENV_VERSION=4.0.5 rubocop -A` - clean

Generated with Claude Code

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant