fix: prevent multiple Login instances in break handler#1524
Merged
chsami merged 1 commit intochsami:developmentfrom Sep 17, 2025
Merged
fix: prevent multiple Login instances in break handler#1524chsami merged 1 commit intochsami:developmentfrom
chsami merged 1 commit intochsami:developmentfrom
Conversation
Fixes login state multiplication by immediately transitioning to LOGGING_IN after creating Login instance in handleLoginRequestedState(). This prevents the repeated execution that was causing multiple login attempts and script multiplication issues.
Contributor
Walkthrough
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java (3)
836-839: Window title always says “Microbreak” even for non‑micro in‑game breaks.When INGAME_BREAK_ACTIVE is due to a normal in‑game break (not micro), the title still shows “In Game Break(Microbreak)”.
Apply:
- String breakType = state == BreakHandlerState.INGAME_BREAK_ACTIVE ? "In Game Break(Microbreak)" : "Break"; + boolean micro = Rs2AntibanSettings.microBreakActive; + String breakType = state == BreakHandlerState.INGAME_BREAK_ACTIVE + ? (micro ? "In Game Break (Microbreak)" : "In Game Break") + : "Break";
511-513: Log text claims a fallback that doesn’t happen.Message says “falling back to world selection mode” but the code breaks out and proceeds with default login (no selection).
- log.warn("Last world {} is not accessible, falling back to world selection mode", preBreakWorld); + log.warn("Last world {} is not accessible, falling back to default login", preBreakWorld);
103-105: Extended‑sleep “every 5 minutes” log spams once per second during that minute.Using
sleepTime % 5 == 0logs every tick within that minute. Gate it to once per 5‑minute mark.@@ - private static volatile Instant extendedSleepStartTime = null; + private static volatile Instant extendedSleepStartTime = null; + // last minute mark we logged during extended sleep to avoid per-second spam + private static long lastExtendedSleepLogMinute = -1; @@ - if (Microbot.isLoggedIn()) { + if (Microbot.isLoggedIn()) { log.info("Login successful during extended sleep, proceeding to break ending"); extendedSleepStartTime = null; // reset extended sleep timer + lastExtendedSleepLogMinute = -1; transitionToState(BreakHandlerState.BREAK_ENDING); return; } @@ - if (extendedSleepStartTime == null) { + if (extendedSleepStartTime == null) { extendedSleepStartTime = Instant.now(); log.info("Extended sleep started for {} minutes", config.extendedSleepDuration()); + lastExtendedSleepLogMinute = -1; return; } @@ - if (sleepTime >= config.extendedSleepDuration()) { + if (sleepTime >= config.extendedSleepDuration()) { log.info("Extended sleep period completed after {} minutes, resuming login attempts", sleepTime); extendedSleepStartTime = null; // reset extended sleep timer + lastExtendedSleepLogMinute = -1; retryCount.set(0); // reset retry count for fresh attempt transitionToState(BreakHandlerState.LOGIN_REQUESTED); } else { - // still in extended sleep - show progress occasionally - if (sleepTime % 5 == 0) { // every 5 minutes - long remaining = config.extendedSleepDuration() - sleepTime; - log.debug("Extended sleep in progress - {} minutes remaining", remaining); - } + // still in extended sleep - show progress once per 5-minute mark + if (sleepTime % 5 == 0 && sleepTime != lastExtendedSleepLogMinute) { + lastExtendedSleepLogMinute = sleepTime; + long remaining = config.extendedSleepDuration() - sleepTime; + log.debug("Extended sleep in progress - {} minutes remaining", remaining); + } }Also applies to: 629-633, 635-639, 643-648, 650-653
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerOverlay.java (2)
52-54: Don’t post-process state with replace("_", " ") now that toString() returns display names.BreakHandlerState.toString() already yields human-friendly labels; replacing underscores is stale logic and can mask future labels.
Apply this diff:
- .left("State: " + BreakHandlerScript.getCurrentState().toString().replace("_", " ")) + .left("State: " + BreakHandlerScript.getCurrentState().toString())
98-109: Prefer consistent formatDuration overloadBoth overloads exist; no compile break. For consistency with other call sites, use the (Duration, String) overload.
- .left("Extended sleep: " + BreakHandlerScript.formatDuration(remainingDuration)) + .left(BreakHandlerScript.formatDuration(remainingDuration, "Extended sleep:"))runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerPlugin.java (1)
51-64: Overlay presence check is inconsistent (name vs instance).Using getName() to detect presence can desync from the actual instance and fail to remove/add correctly. Check by identity to align logic with add/remove using the same instance.
- boolean hasOverlay = overlayManager.anyMatch(ov -> ov.getName().equalsIgnoreCase(BreakHandlerOverlay.class.getSimpleName())); + boolean hasOverlay = overlayManager.anyMatch(ov -> ov == breakHandlerOverlay);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerOverlay.java(5 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerPlugin.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java(28 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerState.java(2 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerState.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Voxsylvae
PR: chsami/Microbot#1415
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/pluginscheduler/SchedulerPlugin.java:2341-2366
Timestamp: 2025-08-25T07:26:53.623Z
Learning: In SchedulerPlugin.startLoginMonitoringThread(), do not gate startup on isBreakHandlerEnabled(); rely on isOnBreak() to suppress the monitor only when a break is actually active. This avoids blocking login monitoring whenever BreakHandler is merely enabled.
📚 Learning: 2025-08-25T07:26:53.623Z
Learnt from: Voxsylvae
PR: chsami/Microbot#1415
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/pluginscheduler/SchedulerPlugin.java:2341-2366
Timestamp: 2025-08-25T07:26:53.623Z
Learning: In SchedulerPlugin.startLoginMonitoringThread(), do not gate startup on isBreakHandlerEnabled(); rely on isOnBreak() to suppress the monitor only when a break is actually active. This avoids blocking login monitoring whenever BreakHandler is merely enabled.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
Comment on lines
+854
to
+856
| if(scheduledFuture != null && !scheduledFuture.isDone()) { | ||
| scheduledFuture.cancel(true); | ||
| } |
Contributor
There was a problem hiding this comment.
Cancelling the wrong future risks leaving the main loop running.
run() schedules work on mainScheduledFuture, but shutdown() cancels scheduledFuture. Cancel the main loop too.
if(scheduledFuture != null && !scheduledFuture.isDone()) {
scheduledFuture.cancel(true);
}
+ if (mainScheduledFuture != null && !mainScheduledFuture.isDone()) {
+ mainScheduledFuture.cancel(true);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if(scheduledFuture != null && !scheduledFuture.isDone()) { | |
| scheduledFuture.cancel(true); | |
| } | |
| if(scheduledFuture != null && !scheduledFuture.isDone()) { | |
| scheduledFuture.cancel(true); | |
| } | |
| if (mainScheduledFuture != null && !mainScheduledFuture.isDone()) { | |
| mainScheduledFuture.cancel(true); | |
| } |
🤖 Prompt for AI Agents
In
runelite-client/src/main/java/net/runelite/client/plugins/microbot/breakhandler/BreakHandlerScript.java
around lines 854 to 856, the shutdown path cancels scheduledFuture but not
mainScheduledFuture, which can leave the main loop running; update shutdown() to
also check mainScheduledFuture for null and !isDone(), call
mainScheduledFuture.cancel(true), and perform any same cleanup/reset used for
scheduledFuture (e.g., nulling references or awaiting termination) so the main
loop is stopped reliably.
chsami
approved these changes
Sep 17, 2025
This was referenced Oct 16, 2025
Merged
Merged
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
2 participants
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.
Appreciate further testing, solves the issues on my end. (also test by @Bolado)