fix(antiban): prevent action-cooldown stall when play-style tick intervals cross#1795
fix(antiban): prevent action-cooldown stall when play-style tick intervals cross#1795pjmarz wants to merge 2 commits into
Conversation
…rvals cross Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a defensive fix and regression coverage for a cooldown stall caused by crossed tick-interval bounds, and adjusts cooldown pausing to avoid leaving scripts paused if interval computation fails.
Changes:
- Harden
PlayStyle#getRandomTickInterval()by ordering bounds before callingThreadLocalRandom.nextInt. - Add a regression test covering crossed/equal tick interval bounds.
- Move
pauseAllScriptsactivation to occur afterTIMEOUTis computed.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| runelite-client/src/test/java/net/runelite/client/plugins/microbot/util/antiban/enums/PlayStyleRandomTickIntervalTest.java | Adds regression test that reproduces crossed-bound intervals and asserts no exception + valid range. |
| runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/antiban/enums/PlayStyle.java | Prevents IllegalArgumentException by defensively ordering interval bounds before random selection. |
| runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/antiban/Rs2Antiban.java | Reorders pause flag setting to occur after interval computation to reduce “paused with no countdown” failure mode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // primaryTickInterval and secondaryTickInterval drift independently in evolvePlayStyle() | ||
| // and can cross (primary > secondary). nextInt(origin, bound) requires bound > origin, so | ||
| // order the bounds defensively to avoid IllegalArgumentException. | ||
| int lo = Math.min(primaryTickInterval, secondaryTickInterval); | ||
| int hi = Math.max(primaryTickInterval, secondaryTickInterval); | ||
| return ThreadLocalRandom.current().nextInt(lo, hi + 1); |
| PlayStyle style = PlayStyle.values()[0]; | ||
| int origPrimary = getInterval(style, "primaryTickInterval"); | ||
| int origSecondary = getInterval(style, "secondaryTickInterval"); |
| setInterval(style, "primaryTickInterval", 10); | ||
| setInterval(style, "secondaryTickInterval", 5); | ||
| for (int i = 0; i < 10_000; i++) { | ||
| int v = style.getRandomTickInterval(); | ||
| assertTrue("interval " + v + " must fall within [5,10]", v >= 5 && v <= 10); | ||
| } |
| // Pause and set the active flag together, only after TIMEOUT is computed. If computing the | ||
| // interval ever throws, scripts must not be left paused with no countdown to clear them. | ||
| if (Rs2AntibanSettings.universalAntiban) | ||
| Microbot.pauseAllScripts.compareAndSet(false, true); | ||
|
|
||
| Rs2AntibanSettings.actionCooldownActive = true; |
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds defensive interval bound ordering to 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
…rate comment Addresses Copilot review feedback on chsami#1795. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed: the inclusive upper bound is computed in long so Integer.MAX_VALUE cannot overflow, the test pins PlayStyle.MODERATE instead of values()[0], and the reorder comment now describes the conditional pause plus unconditional flag precisely. On the enum-state mutation concern: the intervals are restored in a finally block and the unit-test suite runs single-threaded; noted in the test. |
Summary
With "Use Non-Linear Intervals" enabled (which the per-skill antiban setup templates turn on by default),
PlayStyle.evolvePlayStyle()random-walksprimaryTickIntervalandsecondaryTickIntervalindependently, so over time they can cross (primary greater than secondary). Once that happens,PlayStyle.getRandomTickInterval()callsThreadLocalRandom.nextInt(origin, bound)with origin above bound and throws on every stat change:Captured live on 2.6.5 while high alching.
The impact is worse than the exception:
performActionCooldown()setsMicrobot.pauseAllScripts = trueBEFORE computing the interval, andactionCooldownActive = trueonly after. The throw lands between the two, so the cooldown clearer (AntibanPlugin.performActionBreak, gated onactionCooldownActive) never runs and every running script stays frozen until the interval random-walk happens to uncross (observed multi-minute stalls). Any script using a templated antiban setup is exposed.Fix
PlayStyle.getRandomTickInterval(): order the bounds with min/max before callingnextInt(lo, hi + 1), so crossed intervals can never throw.Rs2Antiban.performActionCooldown(): compute the interval first, then setpauseAllScriptsandactionCooldownActivetogether, so no exception path can leave scripts paused with nothing to release them.PlayStyleRandomTickIntervalTestforces the crossed-interval state (which deterministically threw before the fix) plus the equal-bounds edge case.Testing
./gradlew :client:runUnitTests --tests "*PlayStyleRandomTickIntervalTest"passes (failed before the fix)../gradlew :client:compileJavagreen.🤖 Generated with Claude Code