Skip to content

Fix/network spam#40

Open
mCodex wants to merge 7 commits into
mainfrom
fix/networkSpam
Open

Fix/network spam#40
mCodex wants to merge 7 commits into
mainfrom
fix/networkSpam

Conversation

@mCodex

@mCodex mCodex commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features

    • Added action rate limiting to prevent duplicate client actions across gameplay
    • Introduced lookahead targeting for smoother waypoint-based walking paths
    • Added shrine detection caching for imbuing operations
    • Implemented waypoint deduplication during route recording
  • Improvements

    • Enhanced pathfinding with alternative recovery when routes become unreachable
    • Optimized equipment decision logic to reduce excessive calls
    • Added dynamic corridor adjustment based on route density
    • Improved tile searches with bounded viewport scanning
  • Documentation

    • Added performance guide for outbound action rate limiting

mCodex added 6 commits May 18, 2026 18:04
Introduce a small outbound action rate limiter (core/bot_core/action_rate_limiter.lua) and wire it into CaveBot modules to prevent tight retry loops from spamming client actions. Throttling calls were added to clear_tile, depositor, doors, imbuing and stand_lure (with path caching and shrine lookup caching) to gate goto/attack/use/open/move/stash/say operations. Refactor Conditions to use a single unified tick handler and route spell/talk casts through the limiter; update Equipper to use a context fingerprint and inventory generation for smarter throttled equip checks. Also: minor attack_state_machine timing tweak, add .vscode to .gitignore, update PERFORMANCE.md with limiter docs, and bump version to 3.6.3.
Integrate BotCore.ActionRateLimiter across multiple modules to prevent action spam and client API overuse. Added small helper throttle/allow wrappers and explicit limiter.allow checks in cavebot (actions, say/npcsay, antistuck, push player), DWithdraw and InWithdraw (throttled container opens/moves with short delays), Containers (throttle forced opens and sort moves; replaced findDestinationForItem with buildDestinationMap for lookup efficiency), Equipper (throttle equip/unequip moves and equip API calls), BotCore attack/items (throttle item use/drop/useOn), quiver_manager (throttle quiver fills/clears), and TargetBot (throttle chase-mode changes, container opens, nested opens, food use and moves). Also adjusted loot retry behavior (increase lootTries to 3) and added small delays where appropriate. These changes aim to make action requests more reliable and reduce client/server rate issues.
Major refactor to reduce startup cost, prevent redundant work, and harden runtime behavior:

- Add cached ACL loader (loadAclCached) and use it for inline client detection and auto-detection retries to avoid re-parsing /core/acl/init.lua.
- Introduce singleton guards for constants modules (Directions, FloorItems, FoodItems) to skip expensive re-execution on subsequent dofile() calls.
- Defer UI window creation in core/analyzer.lua via schedule(50, ...) so windows are created hidden asynchronously to keep startup snappy.
- Lower slow-startup module threshold from 100ms to 75ms for more sensitive warnings.
- Add ClientService.getTilesAroundPlayer(radius) and replace several full-floor g_map.getTiles(posz()) scans with bounded viewport scans (used in Items.findOnGround and Position.getBestTileByPattern) to limit work to reachable tiles.
- Fix recursion/bug in attack_system isInPz by delegating to SafeCall.isInPz() instead of shadowing the global.
- Add a reusable throttleComboAction limiter in core/combo.lua and apply rate-limits across combo triggers, chat/missile handlers and server listen hooks to avoid spamming actions; increase several throttle intervals.
- Add re-entry guard and conservative scheduling to core/eat_food.lua (State.eatingInProgress and stepDelay) to prevent overlapping eat cycles.
- Increase throttles for cavebot action disintegrate and TargetBot loot move operations to reduce conflicting operations.
- Remove legacy core/cavebot.lua and add core/targetbot_init.lua which defers loading of cavebot and targetbot modules in a controlled asynchronous order to improve perceived boot time and keep targetbot UI/init work grouped.

Overall these changes reduce startup latency, prevent duplicate/expensive module work, bound map scans to the viewport, and make action dispatching more robust by adding rate-limits and guards.
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mCodex, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 38 minutes and 5 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d4682ac7-da13-4ed3-b061-137cb1134fdf

📥 Commits

Reviewing files that changed from the base of the PR and between 70ad9da and 18dfe9e.

📒 Files selected for processing (15)
  • cavebot/cavebot.lua
  • cavebot/clear_tile.lua
  • cavebot/d_withdraw.lua
  • cavebot/doors.lua
  • cavebot/stand_lure.lua
  • cavebot/walking.lua
  • core/Conditions.lua
  • core/Containers.lua
  • core/Equipper.lua
  • core/bot_core/action_rate_limiter.lua
  • core/client_service.lua
  • core/combo.lua
  • core/targetbot_init.lua
  • targetbot/attack_state_machine.lua
  • targetbot/creature_attack.lua
📝 Walkthrough

Walkthrough

This PR introduces action rate-limiting system-wide to prevent duplicate client interactions, reorganizes bot module initialization, optimizes equipment decision logic, enhances waypoint recovery pathfinding, unifies condition spell handling, and updates version to 3.7.0. The ActionRateLimiter module is integrated across cavebot, targetbot, and core combat/resource systems.

Changes

Action Rate Limiting and System Foundation

Layer / File(s) Summary
ActionRateLimiter core module and BotCore integration
core/bot_core/action_rate_limiter.lua, core/bot_core/init.lua
Introduces per-key action throttling via timestamp/counter tracking. Exports allow(), run(), castSpell(), mark(), isReady(), and stat helpers. Registers on BotCore.ActionRateLimiter and nExBot.ActionRateLimiter.
ACL caching and lastDetection storage
_Loader.lua, core/acl/init.lua
Adds loadAclCached() helper to avoid repeated dofile("/core/acl/init.lua") calls during client detection. Stores detected type/name/signals into ACL.lastDetection and nExBot.clientDetection for later inspection.
Performance documentation for action rate limiting
docs/PERFORMANCE.md
Documents BotCore.ActionRateLimiter behavior, default per-action intervals, and usage of getStats() for debugging suppressed/sent metrics.

Boot Sequence and Module Initialization Reorganization

Layer / File(s) Summary
Loader refactor to use targetbot_init
_Loader.lua
Updates legacy features phase to load targetbot_init instead of cavebot. Uses loadAclCached() for repeated client detection. Reduces slow-module reporting threshold from 100ms to 75ms. Updates documentation to clarify TargetBot/CaveBot loading via targetbot_init.lua.
New targetbot_init.lua bootstrap module
core/targetbot_init.lua
Immediately loads core cavebot UI/styles and foundational modules, establishes CaveBot.Extensions, defines safeDofile() wrapper, and defers auxiliary cavebot + all TargetBot modules via ordered list. Incremental schedule() loading sets nExBot.bootReady = true on completion.

Constant Module Singleton Guards

Layer / File(s) Summary
Singleton load guards for constant modules
constants/directions.lua, constants/floor_items.lua, constants/food_items.lua
Adds _loaded flag and early return check to each constant file, so repeated dofile() calls return cached table without re-initializing constants and helpers.

Rate-Limiting Integration Across Core and Combat Systems

Layer / File(s) Summary
Equipper rate-limiting and context caching
core/Equipper.lua
Adds throttleEquipAction() to gate equip/unequip/move via ActionRateLimiter. Introduces inventoryGeneration counter and contextFingerprint (from combat state: HP/MP buckets, target, protection, danger) to avoid redundant equipment rechecks when context and inventory match. Updates unequipSlot(), equipSlot(), throttledEquipCheck() to use throttling and fingerprinting. Adds EventBus handlers for container events to invalidate cache and trigger re-checks.
Container sorting and item action rate-limiting
core/Containers.lua, core/bot_core/items.lua, core/client_service.lua
Containers.lua precomputes buildDestinationMap() to avoid per-tick findDestinationForItem() scanning, rate-limits sort-move and force-open via ActionRateLimiter. Items.lua adds allowItemAction() helper to gate useSelf/useOn/drop. ClientService adds getTilesAroundPlayer(radius) (default 10) to constrain tile searches to viewport.
Combo, quiver, and food re-entry guards
core/combo.lua, core/quiver_manager.lua, core/eat_food.lua
Combo.lua adds throttleComboAction() to rate-limit spell/useWith/attack across chat triggers, missile handlers, leader-target, and websocket listeners. quiver_manager.lua adds throttleMove() to gate clear/fill operations. eat_food.lua adds State.eatingInProgress guard to prevent overlapping scheduled chains and computes bounded stepDelay.
Item-on-target attack rate-limiting
core/bot_core/attack_system.lua
Gates useItemOnTarget() via ActionRateLimiter using per-target key (from getId/getName). Replaces self-referential isInPz behavior with SafeCall.isInPz() delegation.

CaveBot Action Rate-Limiting

Layer / File(s) Summary
CaveBot actions throttling helpers and basic actions
cavebot/actions.lua, cavebot/bank.lua
actions.lua adds actionLimiter()/throttleCavebotAction() helpers. Throttles antistuck disintegrate/move and say/npcsay by message content. Modifies pushPlayer() to return move result (or false when throttled). bank.lua adds null-check guard for CaveBot/isOff.
Goto pure pursuit lookahead target
cavebot/actions.lua
Extends goto action to compute WaypointNavigator.getLookaheadTarget() when route is built, validating same-floor and distance constraints, using that target as walk destination instead of original waypoint (preserves floor-change behavior).
Clear tile operations rate-limiting
cavebot/clear_tile.lua
Wraps goto-stand/goto, attack, move-item, push-player, and use-door with throttleRetry(...), returning "retry" when throttled and applying appropriate post-action delays (100-250ms per operation).
Container interaction rate-limiting
cavebot/d_withdraw.lua, cavebot/depositor.lua, cavebot/doors.lua, cavebot/inbox_withdraw.lua
d_withdraw throttles open-next/open-full/move-item. depositor throttles open-next (300ms) and stash (250ms per item). doors throttles all use/useWith by position. inbox_withdraw throttles stackable/non-stackable moves. All use throttleRetry() to return "retry" when throttled.
Imbuing shrine caching and stand_lure path caching with throttling
cavebot/imbuing.lua, cavebot/stand_lure.lua
imbuing.lua adds findShrine() with time-window caching, throttles equip-off/shrine-movement/shrine-use paths. stand_lure.lua adds getCachedPaths() for pathfinding result caching (with/without monsters), throttles attack/chase-mode/walk actions, replaces direct findPath calls with cached lookups.

TargetBot Rate-Limiting

Layer / File(s) Summary
Attack state machine same-target reaffirm rate-limiting
targetbot/attack_state_machine.lua
Adds state.lastReaffirmAt to track reaffirm timing during ENGAGING. Same-target reaffirm checks COMMAND_COOLDOWN window; if within cooldown returns early, otherwise updates lastReaffirmAt. Reset path clears lastReaffirmAt back to 0.
Creature attack chase mode rate-limiting
targetbot/creature_attack.lua
Gates setChaseMode() and lastChaseMode caching via ActionRateLimiter (key "targetbot:chase-mode", 300ms interval). Only performs switch and cache when limiter allows.
Looting container/item operations rate-limiting
targetbot/looting.lua
Adds throttleLootAction() helper using ActionRateLimiter and updating waitTill when throttled. Applies throttling to open-corpse, open-queue (requeues when throttled), open-inventory (prioritized + fallback), use-food, open-nested (two variants), move-stack, and move-item operations.

Navigation and Pathfinding Improvements

Layer / File(s) Summary
Waypoint recovery path validation and fallback
cavebot/cavebot.lua
executeRecovery now validates A* path to suggested waypoint via ps.findPath() when available; if no path found, scans forward through navigator indices for same-floor alternate that passes blacklist and path checks. Falls back to original candidate if no alternate exists.
Walking no-path recovery strategy
cavebot/walking.lua
When findWalkablePath() returns no path, attempts PathUtils.findEveryPath() to locate closest reachable tile near dest (same z) by Chebyshev distance, then re-runs findWalkablePath() to that candidate. Only proceeds to tryKeyboardNudge() if this still fails.
Recorder Chebyshev distance and waypoint deduplication
cavebot/recorder.lua
Switches distFromLast, cornerDist, and final waypoint distance validation from Euclidean to Chebyshev. Adds deduplicateWaypoints() helper to remove consecutive same-floor goto entries within 1 tile (Chebyshev). Calls deduplication in disable().
Waypoint navigator corridor adjustment and path strategy relaxation
utils/waypoint_navigator.lua, utils/path_strategy.lua
waypoint_navigator computes average segment length and dynamically adjusts corridor widths based on route density. path_strategy removes far-destination early-exit check, evaluating attempts 4-5 (unseen tiles, ignore fields) for all distances.
Position tile search bounded to viewport
core/bot_core/position.lua
getBestTileByPattern() now uses ClientService.getTilesAroundPlayer(maxDist) when available to constrain tile candidate set to viewport; falls back to g_map.getTiles(posz()) when unavailable.

Condition and Spell System Unification

Layer / File(s) Summary
Conditions handler unification and spell casting
core/Conditions.lua
Adds castConditionSpell() using ActionRateLimiter.castSpell or falling back to say(). Adds healingGroupReady() gate checking group cooldown. Replaces separate cure (500ms) and hold-spells (50ms) handlers with single conditionSpellsHandler() at 100ms that gates curing (HP > 95, group ready, individual flags, mana/status), then selects one hold spell per tick with constraints (Utura < 90 HP, Utana 120s interval, Utamo no shield, Haste requirements, Paralyse checks).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • mCodex/nExBot#36: Both PRs modify CaveBot's same-floor/Z-change recovery logic; this PR enhances it with A* path validation and fallback, while retrieved PR simplifies floor-change handling.
  • mCodex/nExBot#35: This PR's pure-pursuit lookahead target computation in cavebot/actions.lua via WaypointNavigator.getLookaheadTarget() directly builds on the retrieved PR's WaypointNavigator refactor.
  • mCodex/nExBot#38: Both PRs modify CaveBot's walking pathfinding fallback in cavebot/walking.lua, adding recovery strategies around findWalkablePath() and PathUtils handling.

Poem

🐰 A rabbit's joy—the limiter tames the chaos,
Each action gently paced, no frantic blast.
The pathfinder finds its way through waypoints,
Equipment caches thoughts, and spells now sing in time,
All systems dance in rhythm—what harmony at last! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.42% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title "Fix/network spam" is vague and uses generic phrasing that doesn't clearly convey the main changes in the changeset, which involve implementing action rate limiting across multiple bot systems and refactoring cavebot initialization. Use a more descriptive title that captures the primary change, such as 'Implement action rate limiting for outbound bot commands' or 'Add ActionRateLimiter to prevent network spam across bot actions'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/networkSpam

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
cavebot/d_withdraw.lua (1)

92-99: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the matched container item here instead of foundNextContainer.

Line 97 references foundNextContainer, but this file never assigns it. Once the destination container fills up, this branch will try to open nil and stall the withdraw flow instead of opening the nested container you just matched in the loop.

Suggested fix
-					if Client and Client.openContainer then Client.openContainer(foundNextContainer, destContainer) elseif g_game then g_game.open(foundNextContainer, destContainer) end
+					if Client and Client.openContainer then Client.openContainer(item, destContainer) elseif g_game then g_game.open(item, destContainer) end
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cavebot/d_withdraw.lua` around lines 92 - 99, The code calls openContainer
using the undefined variable foundNextContainer when a destContainer is full;
replace that with the matched loop variable (item) so the agent opens the actual
nested container that was found. Specifically, in the block checking
containerIsFull(destContainer) inside the for i, item in
pairs(destContainer:getItems()) loop, pass item (the matched container item) to
Client.openContainer or g_game.open instead of foundNextContainer; keep the
existing throttleRetry, delay, and return "retry" behavior.
cavebot/doors.lua (1)

52-58: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Throttle the GlobalConfig.openDoor path too.

This branch still executes before any throttleDoor(pos) check. If GlobalConfig.openDoor performs the use/useWith call, the new spam guard is bypassed on the main door-opening path and only the fallback implementation is protected.

Suggested fix
       -- Use GlobalConfig for door handling if available
       if GlobalConfig and GlobalConfig.openDoor then
+        if not throttleDoor(pos) then return "retry" end
         if GlobalConfig.openDoor(doorTile, key) then
           delay(200)
           return "retry"
         end
       end
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cavebot/doors.lua` around lines 52 - 58, The GlobalConfig.openDoor branch
runs before the spam guard, allowing bypass of throttleDoor; change the logic so
you call or check throttleDoor(doorTile:getPosition() or doorTile) before
invoking GlobalConfig.openDoor(doorTile, key) (i.e., if throttleDoor(...)
returns true, delay/return "retry" as you do for the fallback path), ensuring
both the custom GlobalConfig.openDoor and the local fallback are protected by
the same throttle check; update references around GlobalConfig.openDoor,
throttleDoor, doorTile and key accordingly.
cavebot/clear_tile.lua (1)

27-46: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix the ClearTile input validation.

Line 44 never rejects bad input: pos is a keyed table, and not #pos == 3 evaluates to false == 3. Invalid coordinates therefore fall through into getDistanceBetween, Client.getTile, and CaveBot.GoTo with nil fields.

Suggested fix
-    local pos = {x=tonumber(data[1]), y=tonumber(data[2]), z=tonumber(data[3])}
+    local pos = {x=tonumber(data[1]), y=tonumber(data[2]), z=tonumber(data[3])}
@@
-    if not `#pos` == 3 then
+    if `#data` < 3 or not pos.x or not pos.y or not pos.z then
       warn("CaveBot[ClearTile]: invalid value. It should be position (x,y,z), is: " .. value)
       return false
     end
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cavebot/clear_tile.lua` around lines 27 - 46, The input validation for
ClearTile is wrong because pos is a keyed table and the condition `not `#pos` ==
3` never catches invalid input; update the validation to check the original data
array length and numeric conversion before using pos: verify `#data == 3` (use
`#data ~= 3` to reject) and ensure `tonumber(data[1])`, `tonumber(data[2])`, and
`tonumber(data[3])` are all non-nil (or check after parsing that pos.x, pos.y,
pos.z are numbers) before calling getDistanceBetween, Client.getTile, or
CaveBot.GoTo; move pos creation or bail out if those checks fail and keep the
existing warn(...) and return false behavior.
targetbot/creature_attack.lua (1)

1537-1557: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep TargetBot.usingNativeChase aligned with the effective client mode.

If the limiter blocks the switch on Lines 1541-1542, the client stays in currentMode, but Line 1557 still publishes useNativeChase. That desynchronizes the movement layer from the real chase mode and can make later ticks skip custom walking while still in stand mode, or vice versa.

Suggested fix
   elseif (Client and Client.setChaseMode) or (g_game and g_game.setChaseMode) then
     -- Fallback: direct chase mode control
     local desiredMode = useNativeChase and 1 or 0
     local currentMode = (Client and Client.getChaseMode) and Client.getChaseMode() or (g_game and g_game.getChaseMode and g_game.getChaseMode()) or -1
+    local effectiveMode = currentMode
     if currentMode ~= desiredMode then
       local limiter = BotCore and BotCore.ActionRateLimiter
       local canSetChaseMode = not limiter or not limiter.allow or limiter.allow("targetbot:chase-mode", 300, "default")
       if canSetChaseMode then
         if Client and Client.setChaseMode then
           Client.setChaseMode(desiredMode)
         elseif g_game and g_game.setChaseMode then
           g_game.setChaseMode(desiredMode)
         end
+        effectiveMode = desiredMode
         -- Cache the mode for other modules
         if TargetCore and TargetCore.Native then
           TargetCore.Native.lastChaseMode = desiredMode
         end
       end
     end
+    TargetBot.usingNativeChase = (effectiveMode == 1)
+  else
+    TargetBot.usingNativeChase = useNativeChase
   end
-  
-  -- Store whether we're using native chase for the walk function
-  TargetBot.usingNativeChase = useNativeChase
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@targetbot/creature_attack.lua` around lines 1537 - 1557, The code currently
sets TargetBot.usingNativeChase to useNativeChase regardless of whether the
client chase mode was actually changed; adjust it so TargetBot.usingNativeChase
reflects the effective client mode: after attempting to change the mode (the
block using currentMode, desiredMode, limiter.allow,
Client.setChaseMode/g_game.setChaseMode and caching
TargetCore.Native.lastChaseMode), compute the effectiveMode (if the switch
succeeded then desiredMode else currentMode) and set TargetBot.usingNativeChase
= (effectiveMode == 1); also update TargetCore.Native.lastChaseMode when
appropriate so both the movement layer and the cached value stay consistent.
🧹 Nitpick comments (1)
cavebot/stand_lure.lua (1)

120-153: ⚡ Quick win

Severely inconsistent indentation reduces readability.

The elseif on line 120 should align with the if on line 116, but is indented much deeper. The nested blocks also have inconsistent indentation levels, making the control flow difficult to follow.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cavebot/stand_lure.lua` around lines 120 - 153, The block beginning with
"elseif pathWithoutMonsters and not pathWithMonsters then" has
inconsistent/deeper indentation than its matching "if" and inner statements;
reformat this entire elseif branch so its "elseif" lines up with the
corresponding "if" and all nested statements use the repository's standard
indentation level (match the surrounding file style), including the for loop,
nextPos updates, the getClient()/getTile() check, the hasCreature/creature
checks, the findPath call, and the subsequent
attack/throttleRetry/setChaseMode/resetRetries/delay/return lines (references:
modPos, getClient, Client.getTile, tile:hasCreature, creature:getHealthPercent,
creature:isMonster, findPath, throttleRetry, attack,
Client.setChaseMode/g_game.setChaseMode). Ensure consistent indentation for all
if/elseif/for/if-end blocks so control flow is visually aligned and readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cavebot/cavebot.lua`:
- Around line 589-631: The forward-scan loop in executeRecovery() uses an
undefined variable maxDist (causing a runtime error); replace uses of maxDist
with a call to CaveBot.getMaxGotoDistance() (or compute maxDist once before the
loop) so distance checks like "if fwdDist <= maxDist" use the correct value;
update the block that references maxDist (around
WaypointNavigator.getGotoIndices(), waypointPositionCache,
isWaypointBlacklisted, and ps.findPath) to either define local maxDist =
CaveBot.getMaxGotoDistance() before the loop or substitute calls to
CaveBot.getMaxGotoDistance() inline.

In `@cavebot/stand_lure.lua`:
- Around line 44-45: pathCache.withMonsters calls findPath using an undefined
variable maxDist (nil) causing inconsistent behavior vs
pathCache.withoutMonsters; fix by either declaring and initializing maxDist
before use (e.g., set maxDist = 30 or to the intended distance) or replace
maxDist in the pathCache.withMonsters call with the explicit numeric value used
for withoutMonsters (30) so both findPath calls use the correct distance; update
the symbol pathCache.withMonsters (and/or define maxDist near fromPos/toPos)
accordingly.
- Around line 7-13: In nowMs(), the branch "if now then return now end" returns
the function object instead of a timestamp; update it to call the function (use
now()) so nowMs() returns the current timestamp when the global now function is
available—modify the nowMs function accordingly (reference: nowMs and the
conditional that checks `if now then ...`).

In `@cavebot/walking.lua`:
- Around line 406-434: The use of posKey:split(",") depends on OTClient's
extended string API rather than a local helper; update the block around
posKey:split(",") (inside the PathUtils.findEveryPath fallback in walking.lua
that builds bestPos and bestCheb and later calls findWalkablePath) to either add
a short clarifying comment stating this relies on OTClient's string.split
implementation or replace posKey:split(",") with a small local split helper if
you want to remove the runtime dependency—make the change near the loop that
iterates everyResult so readers know the dependency (or that a local fallback
exists).

In `@core/bot_core/action_rate_limiter.lua`:
- Around line 135-138: The resetStats function currently clears state.sent and
state.suppressed but leaves state.last intact, causing old cooldowns to continue
throttling after a reset; update ActionRateLimiter.resetStats to also clear
state.last (e.g., assign an empty table or nil) so that all limiter tracking is
truly reset, referencing the existing function ActionRateLimiter.resetStats and
the state fields state.sent, state.suppressed, and state.last.

In `@core/bot_core/items.lua`:
- Around line 21-25: The current allowItemAction wrapper is collapsing
caller-provided per-target limiter keys into a generic "items:"..key gate and
thus undoes the per-target windows created by the combat layer; change
allowItemAction to accept an optional fullLimiterKey (or flag) and, when
provided, call limiter.allow with that full key instead of always prepending
"items:", and update Items.useOn to pass the full per-target limiter key (e.g.
"items:use-on:<itemId>:<targetId>") from the caller (or remove the extra
allowItemAction call in Items.useOn entirely) so actionLimiter().allow receives
the exact per-target key the combat layer expects.

In `@core/client_service.lua`:
- Around line 2039-2045: ClientService.getTilesAroundPlayer currently calls
g_game.getLocalPlayer() directly, bypassing ACL logic; change it to call
ClientService.getLocalPlayer() instead so ACL-backed access is honored—update
the function ClientService.getTilesAroundPlayer to obtain the player via
ClientService.getLocalPlayer(), keep the existing nil checks (if not player then
return {}), then continue to use ClientService.getTilesInRange(pos, radius,
radius, false) as before.

In `@core/combo.lua`:
- Around line 374-377: The equality check mistakenly compares target:getName()
to the creature object serverTarget; update the condition in the
SafeCall.getTarget() branch so it compares names to names or objects to objects
(e.g. use serverTarget:getName() in the right-hand side or compare target ~=
serverTarget) to avoid re-sending g_game.attack(serverTarget) when you're
already targeting the same creature; keep the
throttleComboAction("server-target", 350, "attack") logic intact and ensure the
check uses the existing variables target and serverTarget consistently.

In `@core/Conditions.lua`:
- Around line 287-290: Replace reliance on the global now for Utana timing by
using a local nowMs = nExBot.Shared.nowMs() and compare/set utanaCast against
that (ensure utanaCast is numeric or default 0 before subtracting) so the
condition in the utana block (the expression involving utanaCast and the 120000
ms throttle and the assignment utanaCast = ...) uses nowMs instead of now;
additionally, gate cureParalyse with healingGroupReady() (same as other cures in
the hppercent() > 95 block) so cureParalyse only runs when healingGroupReady()
is true.

In `@core/Containers.lua`:
- Around line 1645-1655: buildDestinationMap currently overwrites previous
assignments for the same item ID, changing behavior to "last one wins"; modify
buildDestinationMap (the local function buildDestinationMap and its use of
destinationByItemId) to preserve the first matching container by only setting
destinationByItemId[id] when it is nil (i.e., if destinationByItemId[id] == nil
then destinationByItemId[id] = entry.itemId), so duplicate item IDs keep the
original scan-order priority; optionally, collect or log duplicates if you want
to surface conflicts but do not change the assignment logic.

In `@core/Equipper.lua`:
- Around line 1189-1208: The current buildContextFingerprint coarsens
HP/MP/danger via bucketPercent and omits dynamic inputs (e.g. hotkey), letting
throttledEquipCheck skip valid rule transitions; update buildContextFingerprint
(and if needed bucketPercent) to use finer-grained values for hp/mp/danger (e.g.
raw numeric or 1% buckets instead of 5-point bands) and include any dynamic rule
inputs such as the hotkey state (and other rule-trigger flags used by the rules
engine) so that changes like 82→80 hp or a hotkey press produce a different
fingerprint and force throttledEquipCheck to re-evaluate. Ensure you reference
bucketPercent and buildContextFingerprint when making the change so
throttledEquipCheck observes the new fingerprint shape.

In `@core/targetbot_init.lua`:
- Around line 11-19: safeDofile currently returns the pcall error string on
failure because the final "return res" executes even when ok is false; change
the failure path to return nil instead of the error string by replacing the
final "return res" with "return nil" (or move "return nil" into the else branch)
so that safeDofile(path) returns the loaded value on success and nil on failure;
keep the warn(...) log as-is and do not propagate the raw error string.

In `@targetbot/attack_state_machine.lua`:
- Around line 687-695: The code may return early from the STATE.ENGAGING branch
before updating state.creature, leaving a stale handle; to fix, refresh the
creature reference (assign state.creature = creature) before the cooldown
early-return check (i.e., move the state.creature = creature assignment above
the if (t - state.lastReaffirmAt) < CC.COMMAND_COOLDOWN return) so
handleEngaging() and handleLocked() always see the fresh handle; keep the rest
of the logic (time check using nowMs() and state.lastReaffirmAt, and updating
state.lastReaffirmAt) unchanged.

---

Outside diff comments:
In `@cavebot/clear_tile.lua`:
- Around line 27-46: The input validation for ClearTile is wrong because pos is
a keyed table and the condition `not `#pos` == 3` never catches invalid input;
update the validation to check the original data array length and numeric
conversion before using pos: verify `#data == 3` (use `#data ~= 3` to reject)
and ensure `tonumber(data[1])`, `tonumber(data[2])`, and `tonumber(data[3])` are
all non-nil (or check after parsing that pos.x, pos.y, pos.z are numbers) before
calling getDistanceBetween, Client.getTile, or CaveBot.GoTo; move pos creation
or bail out if those checks fail and keep the existing warn(...) and return
false behavior.

In `@cavebot/d_withdraw.lua`:
- Around line 92-99: The code calls openContainer using the undefined variable
foundNextContainer when a destContainer is full; replace that with the matched
loop variable (item) so the agent opens the actual nested container that was
found. Specifically, in the block checking containerIsFull(destContainer) inside
the for i, item in pairs(destContainer:getItems()) loop, pass item (the matched
container item) to Client.openContainer or g_game.open instead of
foundNextContainer; keep the existing throttleRetry, delay, and return "retry"
behavior.

In `@cavebot/doors.lua`:
- Around line 52-58: The GlobalConfig.openDoor branch runs before the spam
guard, allowing bypass of throttleDoor; change the logic so you call or check
throttleDoor(doorTile:getPosition() or doorTile) before invoking
GlobalConfig.openDoor(doorTile, key) (i.e., if throttleDoor(...) returns true,
delay/return "retry" as you do for the fallback path), ensuring both the custom
GlobalConfig.openDoor and the local fallback are protected by the same throttle
check; update references around GlobalConfig.openDoor, throttleDoor, doorTile
and key accordingly.

In `@targetbot/creature_attack.lua`:
- Around line 1537-1557: The code currently sets TargetBot.usingNativeChase to
useNativeChase regardless of whether the client chase mode was actually changed;
adjust it so TargetBot.usingNativeChase reflects the effective client mode:
after attempting to change the mode (the block using currentMode, desiredMode,
limiter.allow, Client.setChaseMode/g_game.setChaseMode and caching
TargetCore.Native.lastChaseMode), compute the effectiveMode (if the switch
succeeded then desiredMode else currentMode) and set TargetBot.usingNativeChase
= (effectiveMode == 1); also update TargetCore.Native.lastChaseMode when
appropriate so both the movement layer and the cached value stay consistent.

---

Nitpick comments:
In `@cavebot/stand_lure.lua`:
- Around line 120-153: The block beginning with "elseif pathWithoutMonsters and
not pathWithMonsters then" has inconsistent/deeper indentation than its matching
"if" and inner statements; reformat this entire elseif branch so its "elseif"
lines up with the corresponding "if" and all nested statements use the
repository's standard indentation level (match the surrounding file style),
including the for loop, nextPos updates, the getClient()/getTile() check, the
hasCreature/creature checks, the findPath call, and the subsequent
attack/throttleRetry/setChaseMode/resetRetries/delay/return lines (references:
modPos, getClient, Client.getTile, tile:hasCreature, creature:getHealthPercent,
creature:isMonster, findPath, throttleRetry, attack,
Client.setChaseMode/g_game.setChaseMode). Ensure consistent indentation for all
if/elseif/for/if-end blocks so control flow is visually aligned and readable.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9cbf142a-abbe-4a5e-a6b8-fcb54ce46bfa

📥 Commits

Reviewing files that changed from the base of the PR and between 867858d and 70ad9da.

📒 Files selected for processing (39)
  • .gitignore
  • _Loader.lua
  • cavebot/actions.lua
  • cavebot/bank.lua
  • cavebot/cavebot.lua
  • cavebot/clear_tile.lua
  • cavebot/d_withdraw.lua
  • cavebot/depositor.lua
  • cavebot/doors.lua
  • cavebot/imbuing.lua
  • cavebot/inbox_withdraw.lua
  • cavebot/recorder.lua
  • cavebot/stand_lure.lua
  • cavebot/walking.lua
  • constants/directions.lua
  • constants/floor_items.lua
  • constants/food_items.lua
  • core/Conditions.lua
  • core/Containers.lua
  • core/Equipper.lua
  • core/acl/init.lua
  • core/bot_core/action_rate_limiter.lua
  • core/bot_core/attack_system.lua
  • core/bot_core/init.lua
  • core/bot_core/items.lua
  • core/bot_core/position.lua
  • core/cavebot.lua
  • core/client_service.lua
  • core/combo.lua
  • core/eat_food.lua
  • core/quiver_manager.lua
  • core/targetbot_init.lua
  • docs/PERFORMANCE.md
  • targetbot/attack_state_machine.lua
  • targetbot/creature_attack.lua
  • targetbot/looting.lua
  • utils/path_strategy.lua
  • utils/waypoint_navigator.lua
  • version
💤 Files with no reviewable changes (3)
  • utils/path_strategy.lua
  • core/cavebot.lua
  • core/acl/init.lua

Comment thread cavebot/cavebot.lua
Comment thread cavebot/stand_lure.lua
Comment thread cavebot/stand_lure.lua Outdated
Comment thread cavebot/walking.lua
Comment thread core/bot_core/action_rate_limiter.lua
Comment thread core/Conditions.lua Outdated
Comment thread core/Containers.lua
Comment thread core/Equipper.lua
Comment thread core/targetbot_init.lua
Comment thread targetbot/attack_state_machine.lua Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant