AIO Fighter - Fixes and improvements #1445
Conversation
…em usage after pathfinding
refactor: rollback all the unnecessary changes that broke the looting add: looting will now check the eat for space setting
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRefactors looting into a parameterized pipeline with unified filtering in Rs2GroundItem and a new eat-for-space flag. Adjusts combat gating in AttackNpcScript to consider antiban and in-combat state. Broadens FlickerScript reset condition. Temporarily toggles pathfinder’s bank-item usage in Rs2NpcManager with partial restoration coverage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant LootScript
participant Rs2GroundItem
participant Inventory
participant Antiban as Antiban/Config
participant World as GroundItems
User->>LootScript: run()
LootScript->>Antiban: read looterStyle, params, toggles
alt Style: Names
LootScript->>Rs2GroundItem: lootItemsBasedOnNames(params)
else Style: Value
LootScript->>Rs2GroundItem: lootItemBasedOnValue(params)
else Categories
LootScript->>Rs2GroundItem: lootCoins/lootRunes/... (params)
end
Rs2GroundItem->>World: scan items (range/ownership)
Rs2GroundItem->>Rs2GroundItem: filter by predicate (names/value/category)
Rs2GroundItem->>Inventory: ensureSpaceFor(item) (may eat food)
alt Despawn wait needed
Rs2GroundItem->>World: wait until safe to loot
end
Rs2GroundItem-->>LootScript: success/failure
LootScript->>LootScript: pause other scripts on success
LootScript-->>User: done
sequenceDiagram
autonumber
participant CombatLoop as AttackNpcScript
participant Antiban
participant Rs2Combat
CombatLoop->>Antiban: check antibanEnabled, actionCooldownChance
alt antiban enabled & chance > 0
CombatLoop->>Antiban: proceed only if actionCooldownActive
else fallback
CombatLoop->>Rs2Combat: inCombat()?
alt in combat
CombatLoop->>CombatLoop: handleItemOnNpcToKill(...)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
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. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
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/util/npc/Rs2NpcManager.java (1)
294-337: Fix pathfinder flag leak: restore useBankItems on all paths (use try/finally).Early returns (no locations/valid centers/coords) bypass the restoration, leaving isUseBankItems enabled globally. Wrap the body in try/finally or restore before each return.
Apply:
public static MonsterLocation getClosestLocation(String npcName, int minClustering, boolean avoidWilderness) { - boolean originalUseBankItems = ShortestPathPlugin.getPathfinderConfig().isUseBankItems(); - ShortestPathPlugin.getPathfinderConfig().setUseBankItems(true); + final var cfg = ShortestPathPlugin.getPathfinderConfig(); + final boolean originalUseBankItems = cfg.isUseBankItems(); + cfg.setUseBankItems(true); Microbot.log(Level.INFO,"Finding closest location for: " + npcName); - List<MonsterLocation> allLocations = getNpcLocations(npcName); - if (allLocations.isEmpty()) { - Microbot.log(Level.INFO,"No locations found for " + npcName); - return null; - } + try { + List<MonsterLocation> allLocations = getNpcLocations(npcName); + if (allLocations.isEmpty()) { + Microbot.log(Level.INFO,"No locations found for " + npcName); + return null; + } List<String> allNames = allLocations.stream() .map(MonsterLocation::getLocationName) .collect(Collectors.toList()); Microbot.log(Level.INFO,"Found " + allLocations.size() + " locations for " + npcName + ": " + String.join(", ", allNames)); List<MonsterLocation> validLocations = allLocations.stream() .filter(loc -> loc.getCoords().size() > minClustering) .filter(loc -> !avoidWilderness || !loc.getLocationName().contains("Wilderness")) .collect(Collectors.toList()); - if (validLocations.isEmpty()) { - Microbot.log(Level.INFO,"No valid locations after filtering for " + npcName); - return null; - } + if (validLocations.isEmpty()) { + Microbot.log(Level.INFO,"No valid locations after filtering for " + npcName); + return null; + } List<WorldPoint> centers = validLocations.stream() .map(MonsterLocation::getBestClusterCenter) .filter(Objects::nonNull) .collect(Collectors.toList()); - if (centers.isEmpty()) { - Microbot.log(Level.INFO,"Could not compute any centers for " + npcName); - return null; - } + if (centers.isEmpty()) { + Microbot.log(Level.INFO,"Could not compute any centers for " + npcName); + return null; + } // 6) Find nearest and return int idx = Rs2Walker.findNearestAccessibleTarget(centers, true); MonsterLocation closest = validLocations.get(idx); if (closest.getCoords().isEmpty()) { Microbot.log(Level.INFO,"Closest location had no coords for " + npcName); return null; } Microbot.log(Level.INFO,"Closest location for " + npcName + ": " + closest.getLocationName()); - ShortestPathPlugin.getPathfinderConfig().setUseBankItems(originalUseBankItems); - return closest; + return closest; + } finally { + cfg.setUseBankItems(originalUseBankItems); + } }
🧹 Nitpick comments (12)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/LootingParameters.java (1)
65-75: Constructor wiring looks correct; consider documenting eatFoodForSpace semantics.Brief Javadoc on when food is eaten (any HP vs only if healing) will avoid misuse.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java (2)
35-35: Clarify despawn threshold units and intent.Add a comment (ticks, ~600ms per tick) and rationale for 150 to guide tuning.
296-303: Consider adding optional line-of-sight to the base filter.Previous value-based looting used hasLineOfSight; dropping it may increase misclick/pathing attempts through obstacles. Make LoS an opt-in on LootingParameters or include it here.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (9)
21-23: Remove empty constructor unless framework requires itRedundant if no reflective instantiation depends on it.
40-46: Short-circuit after a successful loot to reduce redundant passesWhen style is MIXED, both name- and value-based looters run every tick. Consider bailing once one succeeds to save cycles and clicks.
47-53: Category handlers run unconditionally each tick; consider early-exit on successHave the handlers return boolean and stop further handlers when one loots an item. This reduces overwork and duplicate targeting.
54-56: Log the stack trace for easier diagnosisUse the logger and include the exception.
- Microbot.log("Looterscript: " + ex.getMessage()); + log.error("LootScript error", ex);
62-77: Arrow stack threshold should be configurableHardcoding
minQuantity=10can be too rigid. Add a config value (e.g.,minArrowStackToLoot) and use it here.
107-108: Remove leading spaces in name filtersRelying on trimming in downstream helpers is fragile; pass clean needles.
- " ashes" + "ashes"- " rune" + "rune"Also applies to: 126-127
139-151: Names arg likely unused for coins/untradables
lootCoins/lootUntradablestypically ignore name filters. Keeping dummy strings adds noise.Also applies to: 157-171
28-28: Consider making the 200 ms tick configurableExpose as
config.lootIntervalMs()to tune load vs responsiveness.
34-36: Replace deprecated Rs2Inventory.getEmptySlots() with emptySlotCount()Rs2Inventory.getEmptySlots() is defined but marked @deprecated; switch to emptySlotCount() to remove deprecated API usage.
- if (((Rs2Inventory.isFull() || Rs2Inventory.getEmptySlots() <= minFreeSlots) && !config.eatFoodForSpace()) || (Rs2Player.isInCombat() && !config.toggleForceLoot())) { + if (((Rs2Inventory.isFull() || Rs2Inventory.emptySlotCount() <= minFreeSlots) && !config.eatFoodForSpace()) || (Rs2Player.isInCombat() && !config.toggleForceLoot())) {
📜 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/combat/AttackNpcScript.java(2 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/FlickerScript.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java(1 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/LootingParameters.java(5 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java(4 hunks)runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/npc/Rs2NpcManager.java(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Krulvis
PR: chsami/Microbot#1422
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/BuryScatterScript.java:36-45
Timestamp: 2025-08-23T10:39:13.332Z
Learning: In the AIOFighter BuryScatterScript, when Rs2Magic.canCast() returns true for Sinister Offering (bones) or Demonic Offering (ashes) but there are fewer than 3 items, the script intentionally waits without fallback to accumulate more items for better XP efficiency. Fallback to manual bury/scatter should only occur when the spells cannot be cast.
📚 Learning: 2025-08-25T15:51:39.272Z
Learnt from: runsonmypc
PR: chsami/Microbot#1417
File: runelite-client/src/main/java/net/runelite/client/plugins/microbot/agility/AgilityScript.java:13-13
Timestamp: 2025-08-25T15:51:39.272Z
Learning: Both net.runelite.api.ItemID and net.runelite.api.gameval.ItemID are valid import paths in the RuneLite codebase. The microbot plugins consistently use net.runelite.api.ItemID as their established pattern.
Applied to files:
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
🧬 Code graph analysis (3)
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/combat/Rs2Combat.java (1)
Rs2Combat(27-244)
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)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.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 (5)
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/FlickerScript.java (1)
176-178: Ignore forceReset over-trigger concernresetLastAttack(true) is only called once—in AIOFighterPlugin.java:451 on hitsplat events—with no looping or repeated invocations, so it cannot spam resets during long animations.
Likely an incorrect or invalid review comment.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/LootingParameters.java (1)
6-12: LGTM: Lombok getters/setters and eatFoodForSpace field added.Defaulting eatFoodForSpace to false via constructors is sane; public setter allows opt-in.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java (1)
555-565: LGTM: canTakeGroundItem correctly handles stackables vs empty slots.This helper makes space checks clearer and re-usable.
runelite-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java (2)
154-171: Terminology consistency: untradable vs untradeableConfig uses “Untradables” while the needle is “untradeable”. Ensure downstream filters don’t rely on this string; otherwise align spelling.
173-201: Parameterized looting via LootingParameters is a solid improvementCleaner API surface and centralized filtering/space handling. Nice.
...nt/src/main/java/net/runelite/client/plugins/microbot/aiofighter/combat/AttackNpcScript.java
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java
Show resolved
Hide resolved
...client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java
Show resolved
Hide resolved
...client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java
Show resolved
Hide resolved
...client/src/main/java/net/runelite/client/plugins/microbot/util/grounditem/Rs2GroundItem.java
Show resolved
Hide resolved
...te-client/src/main/java/net/runelite/client/plugins/microbot/aiofighter/loot/LootScript.java
Show resolved
Hide resolved
3 participants
Changelog
LootingParameters: Added eatFoodForSpace option and improved item looting logic.
LootScript: Rolled back breaking changes; added support for checking the "eat for space" setting.
NpcManager: Fixed getClosestLocation to restore original bank item usage after pathfinding.
AttackNpcScript: Improved antiban logic for combat state handling.
FlickerScript: Fixed lastAttack logic by including NPC animation checks for force reset condition.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor