Open
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughHunt script bumped to 5.12.2. Monitor now detects inventory-overflow ("too many items") server messages and sets a rest flag and rest_reason; hunting and looting loops (including looting_watch) break early when the rest flag is set. Some renew/command-result handling and rest_reason assignments were guarded. Changes
Sequence Diagram(s)sequenceDiagram
participant Server as Server
participant Monitor as HuntMonitor
participant Main as MainLoop
participant LootWatch as LootingWatch
Server->>Monitor: send server_string ("too many items")
Monitor-->>Monitor: match -> set bigshot_should_rest = true\nset rest_reason = "Too many items"
Main->>Monitor: periodic check
alt bigshot_should_rest == true
Main-->>Main: break hunting/looting loop
LootWatch->>Monitor: check bigshot_should_rest
LootWatch-->>LootWatch: break looting_watch early
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip Migrating from UI to YAML configuration.Use the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/bigshot.lic`:
- Line 8224: Remove the trailing blank line at the end of the file to satisfy
RuboCop's Layout/TrailingEmptyLines rule: open scripts/bigshot.lic, remove any
extra newline(s) at the very end of the file so the file ends on the last
non-empty line, then save and re-run the linter/CI.
- Around line 2364-2366: The rest reason "Too many items" is being set when
matching the server_string but later overwritten by the generic
$bigshot_should_rest branch in ready_to_rest?, so preserve the specific reason
by ensuring ready_to_rest? does not clobber $rest_reason; update either the
initial branch that sets $rest_reason ('Too many items') to set a flag or leave
$rest_reason intact, or change ready_to_rest? to only assign a generic
$rest_reason when $rest_reason is nil/empty (or when a more specific flag is not
set). Reference $bigshot_should_rest, $rest_reason and the ready_to_rest? logic
to implement the conditional assignment so the specific "Too many items" reason
is retained in rest/status output.
- Around line 6525-6536: The post-watch cleanup only kills the looter when it's
paused, so if $bigshot_should_rest becomes true while the loot script is still
running it continues in background; update the conditional after computing
box_in_hand so it triggers when either Script.paused?(script_ran.name) OR
$bigshot_should_rest is true, set $bigshot_should_rest and $rest_reason as
before, and call Script.kill(script_ran.name) (guarded by Script.running? if you
prefer) so the looter is terminated whether it was paused or still running.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5c500364-24b8-4979-ae97-6b094230a7c2
📒 Files selected for processing (1)
scripts/bigshot.lic
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/bigshot.lic`:
- Around line 6525-6536: The conditional uses the local/method identifier
bigshot_should_rest instead of the global $bigshot_should_rest, causing a
NoMethodError; update the conditional expression that reads
"(Script.running?(script_ran.name) && bigshot_should_rest)" to reference the
global variable "$bigshot_should_rest" so it matches other uses (e.g., where
$bigshot_should_rest and $rest_reason are set) and ensure the
Script.kill(script_ran.name) cleanup path uses the correct global flag.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 83dd729b-277b-41db-b546-cb2673c760f0
📒 Files selected for processing (1)
scripts/bigshot.lic
Contributor
Author
|
@coderabbitai summary |
Contributor
✅ Actions performedSummary regeneration triggered. |
mrhoribu
changed the title
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
scripts/bigshot.lic (1)
6529-6537:⚠️ Potential issue | 🟡 MinorAvoid clobbering an existing rest reason in
looting_watch.At Line 6536,
$rest_reasonis overwritten wheneverbox_in_handis true. This can replace a more specific earlier reason (e.g., “Too many items”), reducing rest/status clarity.💡 Suggested fix
- $rest_reason = "Box in hand, couldn't store" if box_in_hand + $rest_reason = "Box in hand, couldn't store" if box_in_hand && ($rest_reason.nil? || $rest_reason.empty?)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/bigshot.lic` around lines 6529 - 6537, In looting_watch (the block that checks box_in_hand and sets $bigshot_should_rest), avoid overwriting an existing $rest_reason; change the assignment ` $rest_reason = "Box in hand, couldn't store" if box_in_hand` so it only sets the message when there is no prior reason (e.g. use a conditional like `unless $rest_reason && !$rest_reason.empty?` or the Ruby idiom `$rest_reason ||= "Box in hand, couldn't store"`), keeping the rest of the logic around $bigshot_should_rest, box_in_hand and Script.kill(script_ran.name) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@scripts/bigshot.lic`:
- Around line 6529-6537: In looting_watch (the block that checks box_in_hand and
sets $bigshot_should_rest), avoid overwriting an existing $rest_reason; change
the assignment ` $rest_reason = "Box in hand, couldn't store" if box_in_hand` so
it only sets the message when there is no prior reason (e.g. use a conditional
like `unless $rest_reason && !$rest_reason.empty?` or the Ruby idiom
`$rest_reason ||= "Box in hand, couldn't store"`), keeping the rest of the logic
around $bigshot_should_rest, box_in_hand and Script.kill(script_ran.name)
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 931c6d42-b33d-4050-99be-641b8847d758
📒 Files selected for processing (1)
scripts/bigshot.lic
Contributor
Author
|
@coderabbitai review again |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/bigshot.lic`:
- Around line 3937-3941: The check for unhandled timeout/no-match should mirror
other uses of dothistimeout: replace the current nil check with an explicit
false check so the echo warning runs when dothistimeout returns false; locate
the block using result = dothistimeout('renew 1015', 3, /Renewing "Song of
Depression" for 6 mana.|But you are not singing that spellsong./), the
subsequent if using result =~ /But you are not singing that spellsong./ and the
echo("Did not match renew messaging...") and change the condition to detect
result == false (or result == false || result.nil? if you prefer to be
defensive) so timeout/unmatched cases are handled consistently with other
occurrences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2fe47c7b-1cf1-4520-8465-1ee2efd0501d
📒 Files selected for processing (1)
scripts/bigshot.lic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@coderabbitai summary