Auto Refiner: guard against transient GUI/API runtime errors#106
Open
candaysa wants to merge 2 commits intoDarkbot-Plugins:mainfrom
Open
Auto Refiner: guard against transient GUI/API runtime errors#106candaysa wants to merge 2 commits intoDarkbot-Plugins:mainfrom
candaysa wants to merge 2 commits intoDarkbot-Plugins:mainfrom
Conversation
Contributor
Reviewer's GuideAdds defensive runtime guards to AutoRefin.onTickBehavior to prevent crashes when GUI or trade-window state is transient/invalid while preserving existing behavior in healthy states. Sequence diagram for guarded AutoRefin.onTickBehavior refine flowsequenceDiagram
participant AutoRefin
participant Stats
participant GuiManager
participant DarkbotApi
AutoRefin->>AutoRefin: onTickBehavior()
rect rgb(235, 235, 235)
AutoRefin->>AutoRefin: try block
AutoRefin->>AutoRefin: isReadyForRefining()
alt not ready
AutoRefin-->>AutoRefin: return
else ready
AutoRefin->>AutoRefin: getCargoPercent()
alt cargoPercent < triggerPercent
AutoRefin->>AutoRefin: reset lastCargoAmount and lastRefineAttemptFailed if needed
AutoRefin-->>AutoRefin: return
else cargoPercent >= triggerPercent
AutoRefin->>Stats: getCargo()
Stats-->>AutoRefin: currentCargo
alt lastRefineAttemptFailed and cargo unchanged
AutoRefin-->>AutoRefin: return
else cargo changed or no previous failure
AutoRefin->>AutoRefin: build refineMap from Ore.values()
AutoRefin->>AutoRefin: lastRefineAttemptFailed = true
AutoRefin->>AutoRefin: lastCargoAmount = currentCargo
AutoRefin->>AutoRefin: select ore with max refineable amount
alt refineable ore found
AutoRefin->>GuiManager: getAddress()
GuiManager-->>AutoRefin: guiAddress
alt guiAddress == 0
AutoRefin-->>AutoRefin: skip refine
else guiAddress != 0
AutoRefin->>DarkbotApi: readLong(guiAddress + 0x78)
DarkbotApi-->>AutoRefin: tradeWindowAddress
alt tradeWindowAddress == 0
AutoRefin-->>AutoRefin: skip refine
else tradeWindowAddress != 0
AutoRefin->>DarkbotApi: refine(tradeWindowAddress, ore, amount)
DarkbotApi-->>AutoRefin: refine result
AutoRefin->>AutoRefin: lastRefineAttemptFailed = false
end
end
else no refineable ore
AutoRefin-->>AutoRefin: no refine call
end
end
end
end
end
AutoRefin->>AutoRefin: catch RuntimeException
AutoRefin-->>AutoRefin: ignore error and retry next tick
Class diagram for AutoRefin with guarded onTickBehaviorclassDiagram
class AutoRefin {
- AutoRefinConfig config
- Stats stats
- GuiManager guiManager
- DarkbotApi darkbotApi
- int lastCargoAmount
- boolean lastRefineAttemptFailed
+ void setConfig(ConfigSetting_AutoRefinConfig setting)
+ void onTickBehavior()
+ boolean isReadyForRefining()
+ int getCargoPercent()
+ boolean shouldRefineOre(Ore ore)
+ int maxRefine(Ore ore)
}
class AutoRefinConfig {
+ int triggerPercent
}
class Stats {
+ int getCargo()
}
class GuiManager {
+ long getAddress()
}
class DarkbotApi {
+ long readLong(long address)
+ void refine(long tradeWindowAddress, Ore ore, int amount)
}
class Ore {
}
class ConfigSetting_AutoRefinConfig {
+ AutoRefinConfig getValue()
+ void setValue(AutoRefinConfig value)
}
AutoRefin --> AutoRefinConfig
AutoRefin --> Stats
AutoRefin --> GuiManager
AutoRefin --> DarkbotApi
AutoRefin --> Ore
AutoRefin --> ConfigSetting_AutoRefinConfig
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- Catching and silently ignoring all RuntimeExceptions in
onTickBehaviorrisks hiding unrelated bugs; consider narrowing the try/catch to just thedarkbotApi/GUI interaction and at least logging unexpected failures for debugging. - The hard-coded offset
0x78for the trade window address is used inline; consider extracting this into a named constant to make the intent clearer and reduce the risk of inconsistencies if it ever changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Catching and silently ignoring all RuntimeExceptions in `onTickBehavior` risks hiding unrelated bugs; consider narrowing the try/catch to just the `darkbotApi`/GUI interaction and at least logging unexpected failures for debugging.
- The hard-coded offset `0x78` for the trade window address is used inline; consider extracting this into a named constant to make the intent clearer and reduce the risk of inconsistencies if it ever changes.
## Individual Comments
### Comment 1
<location path="src/main/java/dev/shared/kopoklesz/behaviour/AutoRefin.java" line_range="57-66" />
<code_context>
+ try {
</code_context>
<issue_to_address>
**issue (bug_risk):** Catching and ignoring all RuntimeExceptions may hide real bugs and complicate debugging.
Wrapping the whole `onTickBehavior` in `try { ... } catch (RuntimeException ignored)` will silently swallow any logic/config/state errors, not just transient GUI/API issues, making failures hard to detect. Please either narrow the `try` to only the unstable API calls (e.g. `darkbotApi` / `guiManager`) or log the exception before ignoring it so real defects remain visible.
</issue_to_address>
### Comment 2
<location path="src/main/java/dev/shared/kopoklesz/behaviour/AutoRefin.java" line_range="93-101" />
<code_context>
+ .filter(e -> e.getValue() > 0)
+ .max(Map.Entry.comparingByValue())
+ .ifPresent(entry -> {
+ long guiAddress = guiManager.getAddress();
+ if (guiAddress == 0)
+ return;
+
+ long tradeWindowAddress = darkbotApi.readLong(guiAddress + 0x78);
+ if (tradeWindowAddress == 0)
+ return;
+
+ darkbotApi.refine(tradeWindowAddress, entry.getKey(), entry.getValue());
+ lastRefineAttemptFailed = false; // refine attempt succeeded
+ });
</code_context>
<issue_to_address>
**issue (bug_risk):** Early returns on invalid GUI addresses leave `lastRefineAttemptFailed` stuck as `true` even though no refine was attempted.
Because `lastRefineAttemptFailed` is set to `true` before this block, the early returns on `guiAddress == 0` or `tradeWindowAddress == 0` leave it stuck as `true` even though `refine` was never called, while `lastCargoAmount` has already been updated. That causes the `lastRefineAttemptFailed && currentCargo == lastCargoAmount` guard to suppress future refine attempts until the cargo changes, misclassifying a non-attempt as a failed attempt. Consider treating the 0‑address paths as “no attempt” (e.g., reset `lastRefineAttemptFailed` there) or only setting it to `true` immediately before calling `refine`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent AutoRefin-related crash-to-desktop scenarios by adding defensive guards around refine calls during transient/invalid GUI states (as reported in issue #91).
Changes:
- Wraps
onTickBehaviorlogic in aRuntimeExceptionguard to avoid hard crashes on transient client/API failures. - Adds validation for GUI and trade-window addresses before invoking
darkbotApi.refine(...). - Keeps normal behavior when state is valid; skips the tick when state is invalid.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+94
to
+99
| if (guiAddress == 0) | ||
| return; | ||
|
|
||
| long tradeWindowAddress = darkbotApi.readLong(guiAddress + 0x78); | ||
| if (tradeWindowAddress == 0) | ||
| return; |
There was a problem hiding this comment.
lastRefineAttemptFailed/lastCargoAmount are updated before verifying GUI/trade-window addresses, and the lambda returns early when guiAddress/tradeWindowAddress are 0. That leaves lastRefineAttemptFailed=true with unchanged cargo, so subsequent ticks will return at the if (lastRefineAttemptFailed && currentCargo == lastCargoAmount) guard and never re-check the addresses until cargo changes. This prevents recovery after a transient invalid GUI state. Consider only marking an attempt as "failed" (and caching cargo) after the addresses are validated and an actual refine call is attempted, or resetting the tracking variables when skipping due to invalid addresses so the next tick can retry.
Suggested change
| if (guiAddress == 0) | |
| return; | |
| long tradeWindowAddress = darkbotApi.readLong(guiAddress + 0x78); | |
| if (tradeWindowAddress == 0) | |
| return; | |
| if (guiAddress == 0) { | |
| // GUI address invalid, don't treat this as a failed refine attempt | |
| lastRefineAttemptFailed = false; | |
| lastCargoAmount = -1; | |
| return; | |
| } | |
| long tradeWindowAddress = darkbotApi.readLong(guiAddress + 0x78); | |
| if (tradeWindowAddress == 0) { | |
| // Trade window address invalid, don't treat this as a failed refine attempt | |
| lastRefineAttemptFailed = false; | |
| lastCargoAmount = -1; | |
| return; | |
| } |
Comment on lines
+104
to
+107
| } catch (RuntimeException ignored) { | ||
| // Keep bot alive on transient client/API states (for example while user interacts | ||
| // with upgrade windows), retry next tick. | ||
| } |
There was a problem hiding this comment.
The broad catch (RuntimeException ignored) swallows all runtime failures in this tick without any signal. If this turns into a persistent failure mode (not just a transient GUI state), it will be hard to diagnose and AutoRefin may silently stop working. Consider narrowing the catch to the specific exception(s) thrown by the API here and/or emitting a rate-limited log (or at least resetting state) when an exception is caught.
|
Contributor
Kopoklesz
approved these changes
Mar 24, 2026
Contributor
|
Resolve the conflicts |
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
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.
Summary
This PR adds defensive guards to AutoRefin to prevent potential runtime crashes during transient client/UI states.
Changes
Why
Issue reports mention CTD while manually interacting with upgrade/boost UI while Auto Refiner is enabled. This patch focuses on stability by preventing transient invalid state from bubbling up as a crash.
Related: #91
Summary by Sourcery
Add defensive safeguards to AutoRefin tick processing to prevent crashes during transient GUI or API states.
Bug Fixes:
Enhancements: