Add optional long and mega breaks to Break Handler V2 - #1833
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughBreakHandler V2 adds configurable long breaks with randomized intervals and durations. The script tracks pending and active long-break state, merges due long breaks into active breaks, preserves timers across normal breaks, and resets them during lifecycle transitions. It exposes runtime metrics, break counts, countdowns, and break type. The overlay displays these values and identifies long breaks. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Config.java`:
- Around line 87-101: Update the default values in BreakHandlerV2Config’s
minLongBreakInterval() and maxLongBreakInterval() so the default long-break
window starts after the normal playtime range, preventing long breaks from
preempting normal breaks; leave the configured range constraints unchanged.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Script.java`:
- Around line 74-75: Update the currentBreakIsLong field in BreakHandlerV2Script
to provide thread-safe visibility between the script/scheduler writer and
BreakHandlerV2Overlay.render’s isCurrentBreakLong() reader, preferably by
declaring it volatile or consistently synchronizing both access paths.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c21c6005-ab3c-47a5-bf1d-3eca64c65a7a
📒 Files selected for processing (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Config.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Overlay.javarunelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Script.java
| @Range(min = 1, max = 600) | ||
| default int minLongBreakInterval() { | ||
| return 20; | ||
| } | ||
|
|
||
| @ConfigItem( | ||
| keyName = "maxLongBreakInterval", | ||
| name = "Max Long Break Interval (minutes)", | ||
| description = "Maximum time to play before a long break can trigger", | ||
| position = 6, | ||
| section = breakTimingSettings | ||
| ) | ||
| @Range(min = 1, max = 600) | ||
| default int maxLongBreakInterval() { | ||
| return 30; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Prevent the default long-break interval from suppressing normal breaks.
The default long-break interval is 20-30 minutes. The default normal playtime is 45-90 minutes. The scheduler creates both deadlines at the same time and handles a due long break first. After that long break, it reschedules the normal deadline. Therefore, enabling long breaks with defaults prevents a normal break from starting.
Set the default long-break interval above the normal-playtime range, or retain the normal deadline when a long break completes.
🤖 Prompt for 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.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Config.java`
around lines 87 - 101, Update the default values in BreakHandlerV2Config’s
minLongBreakInterval() and maxLongBreakInterval() so the default long-break
window starts after the normal playtime range, preventing long breaks from
preempting normal breaks; leave the configured range constraints unchanged.
| private boolean longBreakDue = false; | ||
| private boolean currentBreakIsLong = false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'BreakHandlerV2(Script|Overlay)\.java$' . || true
echo "== relevant snippets =="
for f in $(fd 'BreakHandlerV2(Script|Overlay)\.java$' .); do
echo "--- $f ($(wc -l < "$f") lines) ---"
ast-grep outline "$f" --view compact || true
rg -n "currentBreakIsLong|isCurrentBreakLong|nextLongBreakTime|break|longBreak|synchronized|volatile|BlockingEventManager|Thread|new Thread|schedule" "$f" -C 3
done
echo "== search scheduler writes/reads =="
rg -n "currentBreakIsLong|isCurrentBreakLong|nextLongBreakTime" runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2 -C 2Repository: chsami/Microbot
Length of output: 50371
Publish currentBreakIsLong to the overlay thread.
BreakHandlerV2Script writes currentBreakIsLong from the script/scheduler path, while BreakHandlerV2Overlay.render reads it through isCurrentBreakLong() later. Add volatile or use the same lock for both reads and writes so the overlay shows the current break type reliably.
🤖 Prompt for 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.
In
`@runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/breakhandlerv2/BreakHandlerV2Script.java`
around lines 74 - 75, Update the currentBreakIsLong field in
BreakHandlerV2Script to provide thread-safe visibility between the
script/scheduler writer and BreakHandlerV2Overlay.render’s isCurrentBreakLong()
reader, preferably by declaring it volatile or consistently synchronizing both
access paths.
MuffinKid23
changed the title
|
Thanks for the contribution. Before this is ready to merge, please address the following:
Once those are addressed, we can review it again for merge. |
|
Addressed the review feedback:
|
|
Thanks for the update. The
After the scope is clean and the timing behavior is corrected, please let the repository Build check run; the current PR still has no Build result. |
39535e3 to
2b3f3a5
Compare
|
Addressed the latest review feedback:
|
2 participants
Summary
Test