feat(AIOFighter): Add wait-for-loot feature#1402
Conversation
Implements a configurable feature that makes the bot wait up to 6 seconds after killing an NPC for loot to appear before attacking the next target. This is useful for enemies with long death animations. - Added toggleWaitForLoot config option in loot section - Added tracking variables for NPC death time and waiting state - Added ActorDeath event handler to detect when we kill an NPC - Modified AttackNpcScript to pause combat while waiting for loot - Shows countdown status while waiting - 6-second hardcoded timeout to resume combat if no loot appears 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add check to respect pauseAllScripts flag during active looting - Improve NPC death detection by checking for lost target while in combat - Remove clearWaitingForLoot() calls that were causing premature combat resumption - Fix rapid clicking between loot and enemies during loot pickup
WalkthroughAdds configurable loot-wait options and plugin-level wait state/timestamp, caches target NPC index in combat to detect death and enter a loot-wait mode with timeout, and updates looting to respect/clear that wait while handling space/eating and pause state. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Combat as AttackNpcScript
participant Plugin as AIOFighterPlugin
participant Game as Game/NPC
participant Loot as LootScript
participant Items as Ground Items
Combat->>Game: attack NPC
Game-->>Combat: NPC reference (index)
Note over Combat: cache target index
Game-->>Combat: NPC dies
Combat->>Plugin: setWaitingForLoot(true), setLastNpcKilledTime(now), clear cached index
Note over Plugin: waiting-for-loot active
alt Loot available before timeout
Loot->>Plugin: isWaitingForLoot?
Plugin-->>Loot: true
Loot->>Items: attempt pickups (handle space/eat)
Items-->>Loot: first pickup success
Loot->>Plugin: clearWaitForLoot("First loot item picked up")
else Timeout reached
Combat->>Plugin: check elapsed >= lootWaitTimeout
Plugin-->>Combat: timeout reached
Combat->>Plugin: clearWaitForLoot("Timeout elapsed")
end
Combat->>Game: resume attack flow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
| private static long lastNpcKilledTime = 0; | ||
|
|
||
| @Getter @Setter | ||
| private static boolean waitingForLoot = false; |
There was a problem hiding this comment.
Why is there a waiting For Loot bool and a timer that you keep track of? It seems like just the time since kill should be enough to determine whether you're waiting for loot or not
There was a problem hiding this comment.
This is to avoid the Fighter interrupting the loot.
There was a problem hiding this comment.
Yes, but you don't need both.
waitingForLoot can just be a function that checks if lastNpcKilledTime is within LOOT_WAIT_TIME
| Actor currentInteracting = Rs2Player.getInteracting(); | ||
|
|
||
| // If we're not interacting but were recently, the NPC probably just died | ||
| if (currentInteracting == null && Rs2Player.isInCombat()) { |
There was a problem hiding this comment.
This is a check that can trigger for a lot of reasons and should not be used to say that the npc died and thus start a waitforLoot timer.
You lose interacting when you're eating, walking, out of arrows. It generally doesn't work well with anyone that has auto retaliate off
There was a problem hiding this comment.
You're right this is a mistake, should just check if NPC hp hits 0.
| // Handle wait for loot feature | ||
| if (config.toggleWaitForLoot()) { | ||
| NPC npc = npcDespawned.getNpc(); | ||
| if (npc != null && npc.isDead()) { |
There was a problem hiding this comment.
I feel like this PR can be reduced to:
- cache npc were fighting
- check if it dies through the NpcDespawned event
- trigger a delay for the AttackNpcScript
- end timeout early when loot is spawned
Looting script priority should automatically make sure that loot is picked up when the AttackNpcScript is delayed while no loot is spawned.
There was a problem hiding this comment.
The problem is that the NPC takes too long to despawn, and in that time the Fighter will go attack the next NPC.
There was a problem hiding this comment.
The problem is that the NPC takes too long to despawn, and in that time the Fighter will go attack the next NPC.
Out of curiosity, which npcs take 6 seconds to spawn loot?
There was a problem hiding this comment.
It will stop waiting as soon as the loot spawns, 6 seconds is just a safeguard. Dragons take like 3 seconds idk, have not measured.
| @Getter @Setter | ||
| private static boolean waitingForLoot = false; | ||
|
|
||
| public static final int LOOT_WAIT_TIMEOUT = 6000; // 6 seconds |
There was a problem hiding this comment.
6 seconds to wait for loot seems like a lot. Doesn't loot spawn immediately after the Npc is de spawned?
There was a problem hiding this comment.
It's a timeout in case loot does not spawn. This is intended for use with NPCs with long death animations as per the PR desc
There was a problem hiding this comment.
Let's say you're fighting an NPC that doesn't have a guaranteed drop (no bones or ashes). In this scenario you'll find yourself waiting 6 seconds whenever nothing spawns.
There was a problem hiding this comment.
If you can figure out the longest delay for something to die/loot to spawn I will set it to that. 6 seconds is to avoid missing edge cases. I did not foresee this being used on anything that does not have guaranteed drops (it is mainly intended for dragons and the like).
… aio-fighter-wait-for-loot # Conflicts: # runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
- Remove unreliable "lost target while in combat" detection that triggered on eating/walking - Track NPC continuously during combat instead of only on initial attack - Add configurable loot wait timeout (1-10 seconds, default 6) - Prevent target switching while waiting to ensure looting priority - Clear cached NPC properly when looting starts or timeout expires
|
Thanks for the feedback! I've addressed the concerns raised: @Krulvis's suggestions addressed:
Key improvements:
|
- Modified LootScript combat check to bypass restriction when waiting for loot - Ensures loot collection completes before engaging new targets - Fixes issue where wait timeout would expire without looting when attacked
| if (!AIOFighterPlugin.isWaitingForLoot()) { | ||
| Actor currentInteracting = Rs2Player.getInteracting(); | ||
| if (currentInteracting instanceof net.runelite.api.NPC) { | ||
| net.runelite.api.NPC npc = (net.runelite.api.NPC) currentInteracting; |
There was a problem hiding this comment.
Instead of using the NPC object, we should be comparing using Rs2NpcModel. You can compare an NPC & an Rs2NpcModel by using the getIndex() method on each of the objects.
… aio-fighter-wait-for-loot
- Empty looting bag automatically when banking - Add blighted food types to Rs2Food enum for proper banking triggers
… aio-fighter-wait-for-loot
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (1)
69-91: pauseAllScripts must be released in a finally block. A thrown exception can freeze the bot.If an exception occurs after pauseAllScripts is set to true, the catch path never resets it, leaving other scripts stuck. Ensure release in finally.
Apply this diff:
- //Pause other scripts before looting - Microbot.pauseAllScripts.getAndSet(true); - for (GroundItem groundItem : groundItems) { - ... - } - Microbot.log("Looting complete"); - Microbot.pauseAllScripts.compareAndSet(true, false); + // Pause other scripts before looting and always release + Microbot.pauseAllScripts.getAndSet(true); + try { + for (GroundItem groundItem : groundItems) { + ... + } + Microbot.log("Looting complete"); + } finally { + Microbot.pauseAllScripts.compareAndSet(true, false); + }
♻️ Duplicate comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java (1)
72-77: waitingForLoot/lastNpcKilledTime are shared across scheduled tasks without visibility guarantees. Mark volatile (or use atomics).These flags are read/written by multiple executor tasks (AttackNpcScript, LootScript). Without volatile/atomic semantics, you risk stale reads or reordering. Make the fields volatile (minimal change) or use AtomicBoolean/AtomicLong.
Apply this diff:
- @Getter @Setter - private static long lastNpcKilledTime = 0; + @Getter @Setter + private static volatile long lastNpcKilledTime = 0; @@ - @Getter @Setter - private static boolean waitingForLoot = false; + @Getter @Setter + private static volatile boolean waitingForLoot = false;Optional follow-ups:
- Reset these on startup/shutdown to avoid stale state across reloads.
- Centralize “clear wait” behavior to one place to avoid duplication.
Add this lifecycle reset (outside the changed hunk):
// In startUp(), near existing initialization: AIOFighterPlugin.setWaitingForLoot(false); AIOFighterPlugin.setLastNpcKilledTime(0L); // In shutDown(): AIOFighterPlugin.setWaitingForLoot(false); AIOFighterPlugin.setLastNpcKilledTime(0L);Centralized helper (optional):
// Add to AIOFighterPlugin public static void clearWaitForLoot(String reason) { setWaitingForLoot(false); setLastNpcKilledTime(0L); // Keep this method as the single point of truth; callers can clear combat cache if needed. Microbot.log("Clearing wait-for-loot state" + (reason != null ? ": " + reason : "")); }Krulvis previously suggested deriving “waiting” purely from time since kill. If you want to simplify state, you could drop waitingForLoot and keep only a deadline (e.g., waitingUntilEpochMs > now). Not mandatory, but it removes a second source of truth.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
112-123: Bug: Wrong instanceof/cast for currentInteracting; should use Rs2NpcModel, not net.runelite.api.NPCRs2Player.getInteracting() wraps NPCs into Rs2NpcModel. Checking against net.runelite.api.NPC will never match, so the cache won’t update and death detection may never arm. This aligns with prior feedback to use Rs2NpcModel and compare via getIndex().
Apply:
- if (!AIOFighterPlugin.isWaitingForLoot()) { - Actor currentInteracting = Rs2Player.getInteracting(); - if (currentInteracting instanceof net.runelite.api.NPC) { - net.runelite.api.NPC npc = (net.runelite.api.NPC) currentInteracting; - // Update our cached target to who we're fighting - if (npc.getHealthRatio() > 0 && !npc.isDead()) { - cachedTargetNpcIndex = npc.getIndex(); - } - } - } + if (!AIOFighterPlugin.isWaitingForLoot()) { + Object currentInteracting = Rs2Player.getInteracting(); + if (currentInteracting instanceof Rs2NpcModel) { + Rs2NpcModel npc = (Rs2NpcModel) currentInteracting; + // Update our cached target to who we're fighting + if (npc.getHealthRatio() > 0 && !npc.isDead()) { + cachedTargetNpcIndex = npc.getIndex(); + } + } + }Note: If this change renders the net.runelite.api.Actor import unused, let the linter tidy it up separately (not blocking).
🧹 Nitpick comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/bank/BankerScript.java (1)
389-393: Depositing containers: ordering is fine; consider a small utility for maintainability.The added Rs2Bank.depositLootingBag() call in this block is safe given the preceding isOpen() guard. As a readability/maintenance tweak, consider grouping these four calls behind a helper (e.g., Rs2Bank.emptyAllContainers()) to keep call sites concise and consistent across features that bank/empty containers.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterConfig.java (1)
401-422: Config options look good; minor UX copy tweak.Everything here wires cleanly. Optional: rename the label to “Loot Wait Timeout (s)” so units are obvious in the UI.
- name = "Loot Wait Timeout", + name = "Loot Wait Timeout (s)",runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/misc/Rs2Food.java (1)
67-70: Prefer ItemID constants over magic numbers for new blighted foods.Using ItemID.* makes this resilient to cache changes and improves readability.
Apply this diff:
- BLIGHTED_MANTA_RAY(24589, 22, "Blighted manta ray", 3), - BLIGHTED_ANGLERFISH(24592, 22, "Blighted anglerfish", 3), - BLIGHTED_KARAMBWAN(24595, 18, "Blighted karambwan", 3); + BLIGHTED_MANTA_RAY(ItemID.BLIGHTED_MANTA_RAY, 22, "Blighted manta ray", 3), + BLIGHTED_ANGLERFISH(ItemID.BLIGHTED_ANGLERFISH, 22, "Blighted anglerfish", 3), + BLIGHTED_KARAMBWAN(ItemID.BLIGHTED_KARAMBWAN, 18, "Blighted karambwan", 3);runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (1)
62-68: Clear-wait timing: move reset until after a successful loot interaction to avoid premature resumption.Currently you clear the wait state as soon as any candidate loot is detected, before interacting. If an exception occurs or the list contains unrelated items, AttackNpcScript could resume one tick earlier than intended (especially if pauseAllScripts flips late).
Suggest clearing after the first successful pickup (or call a centralized helper). Example:
- // Clear wait for loot state since we found loot - if (AIOFighterPlugin.isWaitingForLoot()) { - AIOFighterPlugin.setWaitingForLoot(false); - AIOFighterPlugin.setLastNpcKilledTime(0); - AttackNpcScript.cachedTargetNpcIndex = -1; // Clear the cached NPC index - Microbot.log("Loot found, clearing wait state"); - } + // Defer clearing wait-for-loot until we successfully pick at least one item //Pause other scripts before looting Microbot.pauseAllScripts.getAndSet(true); - for (GroundItem groundItem : groundItems) { + boolean clearedWait = false; + for (GroundItem groundItem : groundItems) { if (Rs2Inventory.emptySlotCount() <= minFreeSlots && !canStackItem(groundItem)) { Microbot.log("Unable to pick loot: " + groundItem.getName() + " making space"); if (!config.eatFoodForSpace()) { continue; } int emptySlots = Rs2Inventory.emptySlotCount(); if (Rs2Player.eatAt(100)) { sleepUntil(() -> emptySlots < Rs2Inventory.emptySlotCount(), 1200); } } Microbot.log("Picking up loot: " + groundItem.getName()); - if (!waitForGroundItemDespawn(() -> interact(groundItem), groundItem)) { + if (!waitForGroundItemDespawn(() -> interact(groundItem), groundItem)) { return; } + if (!clearedWait && AIOFighterPlugin.isWaitingForLoot()) { + // Either call a centralized helper or inline the reset + AIOFighterPlugin.setWaitingForLoot(false); + AIOFighterPlugin.setLastNpcKilledTime(0); + AttackNpcScript.cachedTargetNpcIndex = -1; + Microbot.log("Loot picked up, clearing wait state"); + clearedWait = true; + } }If you introduce AIOFighterPlugin.clearWaitForLoot(String), call it here instead of duplicating the reset.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (2)
124-136: Death detection logic looks solid; consider extracting a helperUsing Rs2Npc.getNpcByIndex and the combined isDead || (healthRatio==0 && healthScale>0) check is robust against “lost interaction” false-positives. Consider extracting this condition into a hasDied(Rs2NpcModel) helper to keep the run-loop lean and reuse elsewhere.
138-154: Loot wait reset confirmed; clamp countdown and consider monotonic timingVerification shows that LootScript correctly clears both the waiting flag and the cached NPC index when loot is found:
- LootScript.java (lines 63–69):
• AIOFighterPlugin.setWaitingForLoot(false)
• AIOFighterPlugin.setLastNpcKilledTime(0)
• AttackNpcScript.cachedTargetNpcIndex = -1Next steps:
• In AttackNpcScript.java, clamp the remaining-seconds calculation to avoid negative UI values. For example, around line 151:
- int secondsLeft = (int)((timeoutMs - timeSinceKill) / 1000); + int secondsLeft = (int) Math.max(0, TimeUnit.MILLISECONDS.toSeconds(timeoutMs - timeSinceKill));• (Optional) For true monotonic timing, migrate last-kill timestamps and comparisons to use System.nanoTime() (with appropriate conversions via TimeUnit).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterConfig.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/bank/BankerScript.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/misc/Rs2Food.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
AttackNpcScript(43-256)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)
⏰ 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
🔇 Additional comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (1)
42-44: Good gating: allow looting while “waiting for loot,” otherwise respect in-combat block.This matches the feature’s intent and avoids interrupting looting when a new attacker tags you during the wait.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (2)
107-111: Good guard: respect pauseAllScripts during active lootingThis early return prevents combat from interrupting an ongoing loot action. Solid placement before target selection.
175-175: No-op formatting changeBlank line is fine and improves readability before NPC selection.
...nt/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
112-122: Tracking the currently-interacted NPC fixes prior false positivesUpdating the cached target only while actively interacting and the target is alive aligns with earlier feedback to stop relying on “lost interaction.” This should remove spurious death detections during eating/walking/out-of-ammo.
🧹 Nitpick comments (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (2)
125-136: Set a status message immediately upon entering wait-for-lootMinor UX nit: set Microbot.status as soon as we flip into waiting to avoid a one-tick gap before the countdown shows.
- AIOFighterPlugin.setWaitingForLoot(true); - AIOFighterPlugin.setLastNpcKilledTime(System.currentTimeMillis()); - Microbot.log("NPC died, waiting for loot..."); + AIOFighterPlugin.setWaitingForLoot(true); + AIOFighterPlugin.setLastNpcKilledTime(System.currentTimeMillis()); + Microbot.status = "Waiting for loot..."; + Microbot.log("NPC died, waiting for loot...");
138-154: Consolidate timeout cleanup via plugin helper; clamp countdown for nicer UX
- Consider using AIOFighterPlugin.clearWaitForLoot("Timeout reached") for symmetry with LootScript and to centralize state resets.
- Optional: display at least 1s while waiting to reduce 0s flicker at the end.
- int timeoutMs = config.lootWaitTimeout() * 1000; + final int timeoutMs = config.lootWaitTimeout() * 1000; if (timeSinceKill >= timeoutMs) { - // Timeout reached, resume combat - AIOFighterPlugin.setWaitingForLoot(false); - AIOFighterPlugin.setLastNpcKilledTime(0); - cachedTargetNpcIndex = -1; // Clear cached NPC on timeout - Microbot.log("Loot wait timeout reached, resuming combat"); + // Timeout reached, resume combat + AIOFighterPlugin.clearWaitForLoot("Loot wait timeout reached"); + cachedTargetNpcIndex = -1; // Clear cached NPC on timeout + Microbot.log("Loot wait timeout reached, resuming combat"); } else { // Still waiting for loot, don't attack - int secondsLeft = (int) Math.max(0, TimeUnit.MILLISECONDS.toSeconds(timeoutMs - timeSinceKill)); + int secondsLeft = (int) Math.max(1, TimeUnit.MILLISECONDS.toSeconds(timeoutMs - timeSinceKill)); Microbot.status = "Waiting for loot... " + secondsLeft + "s"; return; }runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (3)
9-9: Remove unused import AttackNpcScriptThe class is not referenced in this file. Dropping the import avoids a warning and clarifies dependencies.
-import net.runelite.client.plugins.microbot.aiofighter.combat.AttackNpcScript;
62-91: try/finally around pauseAllScripts prevents stuck-pauses; small robustness tweaksThe always-release in finally prevents deadlocks if pickup fails mid-loop. A couple of small improvements to consider:
- Gate the “Picked up” log to debug or reduce frequency to avoid log spam in high-drop scenarios.
- If waitForGroundItemDespawn fails on one item, consider continuing to the next instead of returning early, unless there’s a known transient failure that requires a retry loop.
- Microbot.log("Picking up loot: " + groundItem.getName()); + // Consider lowering verbosity or guarding behind a debug flag + Microbot.log("Picking up loot: " + groundItem.getName()); - if (!waitForGroundItemDespawn(() -> interact(groundItem), groundItem)) { - return; - } + if (!waitForGroundItemDespawn(() -> interact(groundItem), groundItem)) { + // Optional: skip this item and continue rather than aborting the whole pass + continue; + }
68-76: Space-making via eatAt(100) may consume food aggressivelyOut of scope for this PR, but eating at 100% will consume any available food even at full HP. If that’s intended for “make space,” consider a separate helper (e.g., eatAnyFoodForSpace) to avoid conflating combat-healing thresholds with inventory management.
Would you like a follow-up PR to introduce a dedicated “make space” helper that prefers low-heal foods and avoids hitting food you’re trying to keep for combat?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterConfig.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterConfig.java
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java
🧰 Additional context used
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
AttackNpcScript(43-256)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(60-2244)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)
⏰ 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
🔇 Additional comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (2)
107-111: Pause guard integrates cleanly with lootingEarly-returning on Microbot.pauseAllScripts prevents new attacks during active looting. This is the right choke point and keeps the combat loop cheap while looting is in progress.
48-48: Thread-safety for cross-thread flags confirmed
I’ve verified that AIOFighterPlugin’s key fields are already marked volatile, so no further changes are needed here.• In AIOFighterPlugin.java (73–76),
private static volatile long lastNpcKilledTimeandprivate static volatile boolean waitingForLootare correctly declared volatile.
• In AttackNpcScript.java (48),public static volatile int cachedTargetNpcIndexremains volatile as intended.runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (1)
42-44: In-combat looting now correctly respects wait-for-lootAllowing looting to proceed when AIOFighterPlugin.isWaitingForLoot() is true ensures the first drop won’t be preempted by combat. Matches the feature’s intent.
… aio-fighter-wait-for-loot
…nd robustness fixes
|
Addressed the new coderabbit comments |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (1)
67-76: Re-check inventory after eating and use fast-food path to reduce latency.Currently, you attempt pickup even if a slot wasn’t actually freed. Re-check space post-eat and skip the item if still blocked. Also, leverage the
fastFoodpath to minimize time-to-free-slot while waiting for loot.Apply this diff:
- if (Rs2Inventory.emptySlotCount() <= minFreeSlots && !canStackItem(groundItem)) { + if (Rs2Inventory.emptySlotCount() <= minFreeSlots && !canStackItem(groundItem)) { Microbot.log("Unable to pick loot: " + groundItem.getName() + " making space"); if (!config.eatFoodForSpace()) { continue; } int emptySlots = Rs2Inventory.emptySlotCount(); - if (Rs2Player.eatAt(100)) { + if (Rs2Player.eatAt(100, true)) { sleepUntil(() -> emptySlots < Rs2Inventory.emptySlotCount(), 1200); } + // If we still don't have space and can't stack this item, skip it + if (Rs2Inventory.emptySlotCount() <= minFreeSlots && !canStackItem(groundItem)) { + continue; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
🧰 Additional context used
🧬 Code graph analysis (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(60-2244)
⏰ 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
🔇 Additional comments (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (3)
41-43: Guard now correctly allows looting during wait-for-loot.The added
AIOFighterPlugin.isWaitingForLoot()exception to the in-combat early return aligns with the PR goal: continue looting while waiting and otherwise defer during combat unless force-loot is enabled. Looks good.
78-81: Good partial-failure handling during pickup.Skipping a failed item and continuing the pass avoids aborting the entire looting cycle when a single item is contested or unreachable. This improves robustness.
83-86: Clear-on-first-pickup behavior is correct — volatile fields provide the needed atomicity/visibility.Short verification: waitingForLoot and lastNpcKilledTime are declared volatile and accessed via Lombok-generated accessors; clearWaitForLoot writes those volatiles and clears the cached NPC index (also volatile). Single-variable reads/writes are atomic and visible across threads, so the clear-after-first-pickup call is thread-safe for these flags/timers.
Files/locations inspected:
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java
- private static volatile long lastNpcKilledTime = 0;
- private static volatile boolean waitingForLoot = false; (@Getter/@Setter → isWaitingForLoot/setWaitingForLoot)
- public static void clearWaitForLoot(String reason) { setWaitingForLoot(false); setLastNpcKilledTime(0L); AttackNpcScript.cachedTargetNpcIndex = -1; }
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
- public static volatile int cachedTargetNpcIndex = -1;
- Reads AIOFighterPlugin.isWaitingForLoot() and getLastNpcKilledTime() when deciding timeouts/behavior.
Optional note (no blocker): if you need the three fields to change as one indivisible state (no chance of transient inconsistency across multiple variables), synchronize the update or use an atomic composite; otherwise the current volatile-based approach is sufficient.
...te-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (5)
107-110: Redundant pause guard (super.run already checks pauseAllScripts)
super.run()returns false whenMicrobot.pauseAllScriptsis true, so this additional check is redundant.Apply:
- // Check if we should pause while looting is happening - if (Microbot.pauseAllScripts.get()) { - return; // Don't attack while looting - } + // super.run() already returns false when pauseAllScripts is true
144-147: Avoid double-resets of cachedTargetNpcIndexIf
AIOFighterPlugin.clearWaitForLoot(...)already resetsAttackNpcScript.cachedTargetNpcIndex, the additional assignment is redundant.Apply:
- AIOFighterPlugin.clearWaitForLoot("Loot wait timeout reached, resuming combat"); - cachedTargetNpcIndex = -1; // Clear cached NPC on timeout + AIOFighterPlugin.clearWaitForLoot("Loot wait timeout reached, resuming combat");
139-153: Disable wait immediately if the user turns off the toggle at runtimeIf the user disables “Wait for Loot” while already waiting, the script will continue waiting until timeout. Clear the wait state immediately to reflect the new configuration.
Apply:
- // Check if we're waiting for loot - if (config.toggleWaitForLoot() && AIOFighterPlugin.isWaitingForLoot()) { + // Check if we're waiting for loot + if (AIOFighterPlugin.isWaitingForLoot()) { + if (!config.toggleWaitForLoot()) { + AIOFighterPlugin.clearWaitForLoot("Wait disabled by config"); + return; + } long timeSinceKill = System.currentTimeMillis() - AIOFighterPlugin.getLastNpcKilledTime();
149-151: Use ceil for countdown to avoid under-reporting remaining seconds
Math.max(1, toSeconds(...))floors the value; showing 5s when 5.9s remain can feel jumpy. Preferceil.Apply:
- int secondsLeft = (int) Math.max(1, TimeUnit.MILLISECONDS.toSeconds(timeoutMs - timeSinceKill)); + int secondsLeft = (int) Math.ceil((timeoutMs - timeSinceKill) / 1000.0);
45-55: Remove unused currentNpc
currentNpcis assigned only byskipNpc()and not read elsewhere in this class. If other components don’t use it, drop it to reduce surface area; otherwise document intended usage.Apply:
- public static Actor currentNpc = null; - public static void skipNpc() { - currentNpc = null; - } + // Removed unused currentNpc; reintroduce if another component requires it.runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
61-65: Prefer using Rs2GroundItem.runWhilePaused to centralize pause/resume semanticsYou already solved the pause restoration here; however,
Rs2GroundItemexposesrunWhilePausedhelpers that encapsulate pausing behavior and reduce repetition.Sketch:
Rs2GroundItem.runWhilePaused(() -> { boolean clearedWait = false; for (GroundItem groundItem : groundItems) { ... if (!clearedWait && AIOFighterPlugin.isWaitingForLoot()) { AIOFighterPlugin.clearWaitForLoot("First loot item picked up"); clearedWait = true; } } Microbot.log("Looting complete"); return true; });Also applies to: 81-91
135-137: Nit: remove leading space in " ashes"The leading space is harmless due to trimming later, but it’s confusing at first glance.
Apply:
- itemNamesToLoot.add(" ashes"); + itemNamesToLoot.add("ashes");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java
🧰 Additional context used
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(60-2244)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/npc/Rs2NpcManager.java (1)
Rs2NpcManager(26-349)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(60-2244)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)
⏰ 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
🔇 Additional comments (6)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (3)
125-132: Ensure Rs2Npc.getNpcByIndex(...) is called on the client threadDepending on implementation, direct NPC table access must occur on the client thread to avoid race conditions and null/stale reads. Verify
Rs2Npc.getNpcByIndex(...)internally marshals to client thread; otherwise wrap the lookup.Apply (if needed):
- Rs2NpcModel cachedNpcModel = Rs2Npc.getNpcByIndex(cachedTargetNpcIndex); + Rs2NpcModel cachedNpcModel = Microbot.getClientThread().invokeLater(() -> Rs2Npc.getNpcByIndex(cachedTargetNpcIndex)).join();If
invokeLater().join()isn’t available, provide a helper inRs2Npcthat guarantees client-thread execution.
139-147: Make waiting flags/time cross-thread safe
AIOFighterPlugin.isWaitingForLoot()andgetLastNpcKilledTime()are accessed from multiple scheduled tasks. Ensure they’revolatileor useAtomicBoolean/AtomicLong. Otherwise, stale reads can break the wait/timeout logic.Proposed changes in AIOFighterPlugin.java:
- private static boolean waitingForLoot; - private static long lastNpcKilledTime; + private static volatile boolean waitingForLoot; + private static volatile long lastNpcKilledTime;Optional: encapsulate both in a small immutable state object and swap via
AtomicReferenceto keep them consistent.
166-170: Confirm interaction between antiban cooldown and wait-for-lootWhen
actionCooldownActiveis set, this branch runs and returns even if “Wait for Loot” just triggered earlier in the tick (but before cooldown toggled). Given the earlier early-return while waiting, it’s probably fine, but please confirm intended priority is “wait > antiban cooldown work”.If “wait” should always win, add a quick guard at the top of this branch:
- if (Rs2AntibanSettings.actionCooldownActive) { + if (Rs2AntibanSettings.actionCooldownActive) { + if (AIOFighterPlugin.isWaitingForLoot()) return;runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (3)
63-65: Good fix: restore pauseAllScripts to its prior stateCapturing the previous value and restoring it in finally prevents unintentionally unpausing other components. This addresses the earlier review note.
Also applies to: 94-95
41-43: Loot-in-combat guard correctly respects waiting-for-lootThe early return now allows looting during the wait window even if in combat. Matches the PR’s “loot prioritization” objective.
67-80: Space-creation loop: confirm we won’t thrash when minFreeSlots > 0When
minFreeSlotsis configured and inventory is nearly full, the eat-for-space branch can run repeatedly per item. Consider short-circuiting once we cannot increase slots beyondminFreeSlotsafter a single eat attempt to avoid unnecessary retries across items in the same pass.Optional tweak:
- if (Rs2Player.eatAt(100, true)) { + if (Rs2Player.eatAt(100, true)) { sleepUntil(() -> emptySlots < Rs2Inventory.emptySlotCount(), 1200); } - // If we still don't have space and can't stack this item, skip it + // If we still don't have space and can't stack this item, skip it and avoid reattempt in this pass if (Rs2Inventory.emptySlotCount() <= minFreeSlots && !canStackItem(groundItem)) { - continue; + continue; }
...nt/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
Outdated
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java
Outdated
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java
Outdated
Show resolved
Hide resolved
… aio-fighter-wait-for-loot
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (1)
63-65: Pause handling fixed; consider token-based pauser for multi-actor safetyCapturing and restoring the prior pause state avoids unpausing other components—good fix. If multiple systems can pause concurrently, a reference-counted or token-based pauser would make this bulletproof against interleavings where several actors overlap in time.
Example approach (conceptual):
- Use an AtomicInteger counter; increment on pause request; decrement on release; scripts read paused = counter.get() > 0.
- Or issue pause tokens that must be returned, ensuring balanced acquire/release across actors.
Also applies to: 93-95
🧹 Nitpick comments (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (2)
107-111: Maintain wait-for-loot timer even while paused to avoid stale stateWhen LootScript holds the global pause, AttackNpcScript returns early and won’t update/expire the wait timer. Optional: keep servicing the timer while paused to prevent edge cases where a long pause hides timeout progress.
Apply:
- // Check if we should pause while looting is happening - if (Microbot.pauseAllScripts.get()) { - return; // Don't attack while looting - } + // Check if we should pause while looting is happening + if (Microbot.pauseAllScripts.get()) { + // While paused, still maintain/expire the wait-for-loot timer to avoid stale state + if (config.toggleWaitForLoot() && AIOFighterPlugin.isWaitingForLoot()) { + long timeSinceKill = System.currentTimeMillis() - AIOFighterPlugin.getLastNpcKilledTime(); + int timeoutMs = config.lootWaitTimeout() * 1000; + if (timeSinceKill >= timeoutMs) { + AIOFighterPlugin.clearWaitForLoot("Loot wait timeout reached (while paused)"); + cachedTargetNpcIndex = -1; + } else { + int secondsLeft = (int) Math.max(1, TimeUnit.MILLISECONDS.toSeconds(timeoutMs - timeSinceKill)); + Microbot.status = "Waiting for loot... " + secondsLeft + "s"; + } + } + return; // Don't attack while looting + }
139-153: Timeout/resume flow looks solid; consider earlier guards to save workThe countdown/status update and timeout-based resume are correct. Optional: move both the pause guard and the wait-for-loot handling ahead of the expensive NPC filtering/sorting (lines 79–103) to avoid doing heavy work when we’re known to be paused or waiting.
High-level idea:
- Right after the top-level run guards, handle:
- wait-for-loot timeout/status (return if still waiting),
- then pause guard (return if paused),
- then target-caching/death-detection,
- only then build/scan attackable NPC lists.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java(3 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/AIOFighterPlugin.java
🧰 Additional context used
🧬 Code graph analysis (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (1)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/player/Rs2Player.java (1)
Rs2Player(67-2062)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/inventory/Rs2Inventory.java (1)
Rs2Inventory(60-2244)
⏰ 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
🔇 Additional comments (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
41-43: Combat guard correctly exempts wait-for-loot/force-lootThe early return now respects both Force Loot and the new wait-for-loot state, so looting can proceed immediately after a kill without being blocked by incidental combat state. This aligns with the PR's goal of prioritizing loot right after the NPC dies.
86-90: Clear wait state on first successful pickup — matches intended lifecycleClearing the wait-for-loot state after the first successful pickup prevents lingering “waiting…” status once loot begins. The local clearedWait flag correctly avoids multiple clears.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java (3)
48-48: Make cachedTargetNpcIndex volatile — correct for cross-thread visibilityMarking the shared NPC index as volatile is the right call given concurrent access from attack/loot scheduled tasks.
113-122: Continuous target caching during combat — avoids false positivesCaching by NPC index only while actively interacting and the target is alive eliminates prior false positives from eating/walking/ammo. Good alignment with reviewer feedback.
125-137: Death detection via cached index — robust against interaction lossChecking the cached NPC by index and gating on isDead() or (healthRatio==0 && healthScale>0) provides reliable kill detection, especially for long death animations. Clearing the cache and entering wait state is correct.
| return; | ||
| } | ||
| if (Rs2Player.isInCombat() && !config.toggleForceLoot()) { | ||
| if (Rs2Player.isInCombat() && !config.toggleForceLoot() && !AIOFighterPlugin.isWaitingForLoot()) { |
There was a problem hiding this comment.
So, if we dont have toggleForceLoot enabled & we dont have isWaitingForLoot, we just aren't looting at all?
There was a problem hiding this comment.
It's:
- Skip looting if: in combat AND force loot is off AND NOT waiting for loot
- This means: Allow looting when waiting for loot, even if in combat
4 participants
Summary
This PR adds a configurable wait-for-loot feature to AIOFighter that pauses combat after killing an NPC to allow loot to appear before attacking the next target (useful for NPCs with long death animations such as dragons).
Features
Implementation Details
Death detection:
Loot coordination:
pauseAllScriptsflag to prevent interrupting active lootingFiles changed:
AIOFighterConfig.java: Added toggleWaitForLoot and configurable timeout optionsAIOFighterPlugin.java: Added wait state management variablesAttackNpcScript.java: Implemented continuous NPC tracking and death detectionLootScript.java: Added wait state clearing when loot is foundUse Case
Essential for NPCs with:
Summary by CodeRabbit
New Features
Improvements