Conversation
…n/java/dev/shared/halizeur/death_rate_switcher/DeathRateProfileSwitcher.java
Contributor
Reviewer's GuideAdds two new behavior-based features: a Revive Loop Watchdog that monitors prolonged destruction states to pause and later resume the bot, and a Death Rate Profile Switcher that tracks deaths/hour to temporarily switch between configured DarkBot profiles during PvP hunting spikes. Sequence diagram for ReviveLoopWatchdog stuck-revive detection and recoverysequenceDiagram
participant ReviveLoopWatchdog
participant RepairAPI
participant HeroAPI
participant BotAPI
ReviveLoopWatchdog->>RepairAPI: isDestroyed()
alt ship_destroyed_and_deadSince_null
ReviveLoopWatchdog->>ReviveLoopWatchdog: deadSince = Instant.now()
end
ReviveLoopWatchdog->>ReviveLoopWatchdog: Duration.between(deadSince, Instant.now())
alt stuckMinutes >= STUCK_THRESHOLD_MINUTES
ReviveLoopWatchdog->>BotAPI: setRunning(false)
ReviveLoopWatchdog->>ReviveLoopWatchdog: pausedByWatchdog = true
end
loop while pausedByWatchdog
ReviveLoopWatchdog->>HeroAPI: getLocationInfo()
ReviveLoopWatchdog->>RepairAPI: isDestroyed()
alt loaded && !destroyed
ReviveLoopWatchdog->>BotAPI: setRunning(true)
ReviveLoopWatchdog->>ReviveLoopWatchdog: pausedByWatchdog = false
ReviveLoopWatchdog->>ReviveLoopWatchdog: deadSince = null
end
end
Sequence diagram for DeathRateProfileSwitcher death spike handlingsequenceDiagram
participant DeathRateProfileSwitcher
participant RepairAPI
participant ConfigAPI
loop each_tick_onTickBehavior_or_onStoppedBehavior
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: process()
DeathRateProfileSwitcher->>RepairAPI: isDestroyed()
alt destroyed && !wasDestroyed
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: recentDeaths.addLast(Instant.now())
end
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: pruneOldDeaths()
DeathRateProfileSwitcher->>ConfigAPI: getCurrentProfile()
alt revertAt == null and current == FROM_PROFILE and recentDeaths.size() >= DEATHS_PER_HOUR_THRESHOLD
DeathRateProfileSwitcher->>ConfigAPI: setConfigProfile(TO_PROFILE)
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: revertAt = Instant.now().plus(WINDOW)
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: recentDeaths.clear()
else revertAt != null and recentDeaths.size() >= DEATHS_PER_HOUR_THRESHOLD
DeathRateProfileSwitcher->>ConfigAPI: setConfigProfile(FROM_PROFILE)
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: revertAt = null
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: recentDeaths.clear()
else revertAt != null and Instant.now().isAfter(revertAt)
DeathRateProfileSwitcher->>ConfigAPI: setConfigProfile(FROM_PROFILE)
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: revertAt = null
DeathRateProfileSwitcher->>DeathRateProfileSwitcher: recentDeaths.clear()
end
end
File-Level Changes
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 5 issues, and left some high level feedback:
- In both features,
onTickBehavior/onStoppedBehaviorassumeconfighas been initialized; consider adding a null check or default config to avoid potential NPEs if the plugin is ticked beforesetConfigis called. - The
BotAPI botconstructor parameter inDeathRateProfileSwitcheris never used; you can remove it from the constructor signature and DI wiring to keep the class interface minimal. - Both classes use
System.out.printlnfor logging; if the DarkBot API or this plugin already has a logging mechanism, consider routing messages through that instead so they integrate consistently with existing logs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In both features, `onTickBehavior` / `onStoppedBehavior` assume `config` has been initialized; consider adding a null check or default config to avoid potential NPEs if the plugin is ticked before `setConfig` is called.
- The `BotAPI bot` constructor parameter in `DeathRateProfileSwitcher` is never used; you can remove it from the constructor signature and DI wiring to keep the class interface minimal.
- Both classes use `System.out.println` for logging; if the DarkBot API or this plugin already has a logging mechanism, consider routing messages through that instead so they integrate consistently with existing logs.
## Individual Comments
### Comment 1
<location path="src/main/java/dev/shared/halizeur/death_rate_switcher/DeathRateProfileSwitcher.java" line_range="54-56" />
<code_context>
+ // Set once we've switched to TO_PROFILE, holds when the 60 min revert is due.
+ private Instant revertAt = null;
+
+ public DeathRateProfileSwitcher(ConfigAPI configAPI, RepairAPI repair, BotAPI bot) {
+ this.configAPI = configAPI;
+ this.repair = repair;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Constructor takes a BotAPI parameter that is never stored or used.
The `DeathRateProfileSwitcher` constructor takes a `BotAPI bot` argument that isn’t used. If it’s not needed, please remove it to keep the constructor clean; if it will be needed later, consider storing it in a field now so its presence is justified.
Suggested implementation:
```java
// Set once we've switched to TO_PROFILE, holds when the 60 min revert is due.
private Instant revertAt = null;
public DeathRateProfileSwitcher(ConfigAPI configAPI, RepairAPI repair, BotAPI bot) {
this.configAPI = configAPI;
this.repair = repair;
}
import eu.darkbot.api.config.ConfigSetting;
```
```java
import eu.darkbot.api.extensions.Feature;
import eu.darkbot.api.managers.ConfigAPI;
```
If there are any other places in the codebase that call the `DeathRateProfileSwitcher` constructor with three arguments, they will need to be updated to remove the `BotAPI` argument and call the two-argument constructor instead.
</issue_to_address>
### Comment 2
<location path="src/main/java/dev/shared/halizeur/death_rate_switcher/DeathRateProfileSwitcher.java" line_range="107" />
<code_context>
+ }
+
+ private void process() {
+ if (config.FROM_PROFILE.isEmpty() || config.TO_PROFILE.isEmpty()) return;
+
+ trackDeathEdge();
</code_context>
<issue_to_address>
**issue (bug_risk):** Accessing config fields without a null check can NPE before configuration is applied.
In `process()`, this can happen if `onTickBehavior` or `onStoppedBehavior` executes before `setConfig` has run. Please add a null guard at the top of `process()`, e.g. `if (config == null) return;`, before dereferencing `config` fields.
</issue_to_address>
### Comment 3
<location path="src/main/java/dev/shared/halizeur/revive_loop_watchdog/ReviveLoopWatchdog.java" line_range="88-90" />
<code_context>
+ deadSince = Instant.now();
+ }
+
+ long stuckMinutes = Duration.between(deadSince, Instant.now()).toMinutes();
+ if (stuckMinutes >= config.STUCK_THRESHOLD_MINUTES) {
+ pauseForStuckLoop(stuckMinutes);
+ }
+ } else {
</code_context>
<issue_to_address>
**issue (bug_risk):** Plugin may unintentionally resume a manually paused bot.
`pauseForStuckLoop` always sets `pausedByWatchdog = true`, even if the bot was already paused/stopped by the user. Then `checkForRecovery()` resumes the bot whenever `pausedByWatchdog` is true and recovery conditions are met, so a user-paused bot could be resumed unintentionally. Consider only setting `pausedByWatchdog` (and later resuming) if the bot was running at the time of the watchdog pause, or otherwise checking the current run state before resuming.
</issue_to_address>
### Comment 4
<location path="src/main/java/dev/shared/halizeur/revive_loop_watchdog/ReviveLoopWatchdog.java" line_range="89" />
<code_context>
+ }
+
+ long stuckMinutes = Duration.between(deadSince, Instant.now()).toMinutes();
+ if (stuckMinutes >= config.STUCK_THRESHOLD_MINUTES) {
+ pauseForStuckLoop(stuckMinutes);
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** Config is used without a null check, which could cause NPE before configuration is set.
In `onStoppedBehavior`, `config.STUCK_THRESHOLD_MINUTES` is used without first ensuring `config` is initialized. If `onStoppedBehavior` can run before `setConfig`, this will throw a `NullPointerException`. Consider an early return when `config == null` (and similarly in `onTickBehavior` if applicable).
</issue_to_address>
### Comment 5
<location path="src/main/java/dev/shared/halizeur/death_rate_switcher/DeathRateProfileSwitcher.java" line_range="54" />
<code_context>
+ // Set once we've switched to TO_PROFILE, holds when the 60 min revert is due.
+ private Instant revertAt = null;
+
+ public DeathRateProfileSwitcher(ConfigAPI configAPI, RepairAPI repair, BotAPI bot) {
+ this.configAPI = configAPI;
+ this.repair = repair;
</code_context>
<issue_to_address>
**suggestion (review_instructions):** The BotAPI constructor parameter (and corresponding import) are unused, which slightly reduces code quality and should be removed.
`BotAPI bot` is injected but never referenced in this class, and the `BotAPI` import is therefore unused as well. To keep the codebase clean and maintainable, please remove the unused parameter and import, or use the dependency if it was intended for future logic.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `*.java`
**Instructions:**
Minimum code quality must be maintained.
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
sourcery-ai
Bot
dismissed
their stale review
September 1, 2026 19:12
Sourcery withdrew this approval because it has stopped reviewing this pull request.
Contributor
|
Sourcery has withdrawn its approval of this pull request. It auto-reviews a pull request 5 times, and this push is past that limit, so the approval no longer reflects code Sourcery has read. Comment |
|
This was referenced Sep 2, 2026
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.
Adds two new features under dev.shared.halizeur:
Revive Loop Watchdog
Detects DarkBot's known stuck-on-revive refresh loop bug (see darkbot-reloaded/DarkBot #391, #455). If the ship stays destroyed continuously past a configurable threshold (default 3 min), it pauses the bot instead of refreshing indefinitely, then automatically resumes once the game finishes loading and the ship is confirmed alive again. All state changes are logged.
Death Rate Profile Switcher
Watches deaths/hour on a configured "home" profile. If deaths spike past a configurable threshold in a rolling 60-minute window (PvP hunting), it switches to a configured "safe" profile for up to 60 minutes. If the death rate spikes again on the safe profile, it reverts immediately; otherwise it reverts automatically after a clean 60-minute window.
Both features compiled and tested against darkbot-impl 0.9.8 / DarkBot dc48506543, matching this repo's pinned versions.
Summary by Sourcery
Protect bot operation from revive loops and high-death periods by adding recovery monitoring and adaptive profile switching.
New Features:
Bug Fixes:
Chores: