Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 Walkthrough📝 Walkthrough🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
targetbot/looting.lua (1)
628-628: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the “ensure loot space” fallback.
After removing the final
ContainerOpener.ensureLootContainerSpace(...)phase, the no-space path now returns an empty list even when a space-management fallback could open/prepare another loot container. Reintroduce that fallback or move the responsibility to a clearly covered caller.🤖 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/looting.lua` at line 628, The no-space path in `looting.lua` now exits too early because the final `ContainerOpener.ensureLootContainerSpace(...)` fallback was removed, so `lootContainers` can be returned empty even when another loot container could still be opened or prepared. Restore that fallback in the loot collection flow, or shift it to a caller that is guaranteed to run, and make sure the logic around `lootContainers` still reaches the space-management step before returning.core/heal_engine.lua (1)
558-588: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDerive a cooldown key for raw friend spell lists.
HealBotpassesevaluateAllyspells withoutkey, soready(nil)always succeeds andstamp(nil)does nothing on that path. Normalizespell.keybefore cooldown checks or populate it in the builder.🤖 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 `@core/heal_engine.lua` around lines 558 - 588, The raw friend spell list path in HealBot’s healing flow is using spells without a cooldown key, so the cooldown guard in `planFriend`/`ready` can never track them correctly. Normalize or derive a stable `spell.key` before the `ready(spell.key, ...)` check in `core/heal_engine.lua`, or populate the key earlier in the spell builder used by `evaluateAlly`, so cooldown stamping works for those entries too.core/smart_hunt.lua (1)
414-424: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse coin values when accumulating per-item loot buckets.
entry.totalValuecounts coins viaCOIN_VALUES, butanalytics.lootItems[nameKey].valueuses onlyitm.price, so top-valued loot can show coins as zero-valued.Proposed fix
local bucket = analytics.lootItems[nameKey] or {count = 0, value = 0} bucket.count = bucket.count + (itm.count or 0) - local itemValue = (itm.price or 0) * (itm.count or 0) + local unitValue = COIN_VALUES[nameKey] or (itm.price or 0) + local itemValue = unitValue * (itm.count or 0) bucket.value = bucket.value + itemValue🤖 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 `@core/smart_hunt.lua` around lines 414 - 424, The per-item loot aggregation in smart_hunt.lua is not using coin valuation, so coin items can be recorded with zero value in analytics. Update the loot bucket logic inside the entry.items loop to use COIN_VALUES[nameKey] when available instead of relying only on itm.price, and keep the existing gold tracking behavior in sync. Use the analytics.lootItems and COIN_VALUES symbols to locate the block and ensure bucket.value reflects coin value for top-valued loot.targetbot/priority_engine.lua (1)
97-103: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the existing safe position helper in
getPlayer().Line 98 bypasses
cPos()and callsSC.getPositiondirectly, so partialSafeCreatureinitialization can crash the priority engine.Proposed fix
local function getPlayer() - if not player or not SC.getPosition(player) then + if not player or not cPos(player) then local C = getClient() player = (C and C.getLocalPlayer and C.getLocalPlayer()) or (g_game and g_game.getLocalPlayer and g_game.getLocalPlayer())🤖 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/priority_engine.lua` around lines 97 - 103, The getPlayer() helper is bypassing the existing safe position check and calling SC.getPosition directly, which can fail during partial SafeCreature initialization. Update getPlayer() to use the existing cPos() helper for the validity check instead of SC.getPosition, while keeping the rest of the player lookup logic in getClient() and g_game.getLocalPlayer() unchanged.targetbot/monster_ai_core.lua (1)
107-127: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRoute
isValidAliveMonsterthrough the new SafeCreature predicates.The exported helper still calls
creature:isMonster(),creature:isDead(), andcreature:isRemoved()directly, so callers do not benefit from the SafeCreature migration.🛡️ Proposed fix
local function isValidAliveMonster(creature) if not creature then return false end - - local ok, result = pcall(function() - return creature:isMonster() and not creature:isDead() and not creature:isRemoved() - end) - - return ok and result or false + return safeIsMonster(creature) and not safeIsDead(creature) and not safeIsRemoved(creature) 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 `@targetbot/monster_ai_core.lua` around lines 107 - 127, The isValidAliveMonster helper still bypasses the SafeCreature migration by calling creature:isMonster(), creature:isDead(), and creature:isRemoved() directly. Update isValidAliveMonster to use the existing safeIsMonster, safeIsDead, and safeIsRemoved predicates instead of the raw creature methods, and keep the nil/false handling consistent with the surrounding SafeCreature wrappers.
🟡 Minor comments (4)
core/AttackBot.lua-801-802 (1)
801-802: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDisable or hide inactive source navigation controls.
Empty click handlers leave enabled UI controls that silently do nothing. If source switching is intentionally unavailable, hide/disable these buttons or keep a user-visible warning.
🤖 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 `@core/AttackBot.lua` around lines 801 - 802, The source navigation controls in AttackBot are left with empty onClick handlers, which makes them appear enabled while doing nothing. Update the panel.previousSource and panel.nextSource handling to either disable or hide these controls when source switching is unavailable, or wire them to a user-visible warning. Use the existing panel.previousSource and panel.nextSource setup in AttackBot to locate the change.core/Containers.lua-805-806 (1)
805-806: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHonor the
prioritizeflag when queueing BFS items.Line 790 accepts
prioritize, and Line 351 passestrue, but Line 805 always appends. Prioritized nested containers will still wait behind the existing queue.Proposed fix
- ContainerBFS.queue[`#ContainerBFS.queue` + 1] = entry + if prioritize then + table.insert(ContainerBFS.queue, ContainerBFS.queueIdx, entry) + else + ContainerBFS.queue[`#ContainerBFS.queue` + 1] = entry + end return true🤖 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 `@core/Containers.lua` around lines 805 - 806, The BFS queueing logic in ContainerBFS is ignoring the prioritize flag, so prioritized nested containers are always appended behind existing entries. Update the queue insertion path in the ContainerBFS logic to branch on prioritize and place prioritized entries ahead of normal queued items while preserving the existing behavior for non-prioritized cases. Use the ContainerBFS queueing code and the prioritize parameter accepted by the BFS helper to locate the fix.core/bot_database.lua-36-47 (1)
36-47: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the BotDB cache for macro state.
registerMacro()andmigrateOldData()still read/write the legacystoragetable, so the newStorageEnginedata never participates in macro restore or migration. Move this path ontoBotDB(or amacrosnamespace inside it) to keep macro state in one persistence layer.🤖 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 `@core/bot_database.lua` around lines 36 - 47, BotDB.registerMacro still uses the legacy storage table for macro state, so the new StorageEngine cache is bypassed during restore and migration. Update registerMacro and migrateOldData to read/write macro state through BotDB (or a BotDB.macros namespace) instead of storage, and keep the existing behavior in setOn/setOff and the saved/onEnable flow while switching the persistence source to the BotDB cache.core/combo.lua-264-282 (1)
264-282: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClear stale follow targets when no current rule matches.
If
config.followchanges orLEADER TARGETis no longer a player,toFollowkeeps the previous value and follow continues.Proposed fix
+ local nextFollow = nil if config.follow == "LEADER TARGET" and leaderTarget and leaderTarget:isPlayer() then - toFollow = leaderTarget:getName() + nextFollow = leaderTarget:getName() elseif config.follow == "LEADER" then if config.onSayEnabled and config.sayLeader and config.sayLeader:len() ~= 0 then - toFollow = config.sayLeader + nextFollow = config.sayLeader elseif config.onCastEnabled and config.castLeader and config.castLeader:len() ~= 0 then - toFollow = config.castLeader + nextFollow = config.castLeader elseif config.onShootEnabled and config.shootLeader and config.shootLeader:len() ~= 0 then - toFollow = config.shootLeader + nextFollow = config.shootLeader end end + toFollow = nextFollow if not toFollow then return 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 `@core/combo.lua` around lines 264 - 282, The follow target state in followLeaderHandler can stay stale when no current rule matches, causing an old target to persist after config.follow changes or leaderTarget is no longer a player. Update followLeaderHandler so it always clears toFollow before evaluating the current follow rules, and only assigns a new value when a valid LEADER TARGET, sayLeader, castLeader, or shootLeader matches. Ensure the final early return still leaves toFollow nil when nothing matches.
🧹 Nitpick comments (1)
core/AttackBot.lua (1)
1215-1254: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPersist Absolute Sweep rotation attempts across ticks.
_attackCacheis recreated everyattackBotMaintick, and Lines 1633-1635 reset attempts whenever the sweep matches. Ifturn(desired)does not update direction,MAX_ROTATE_ATTEMPTSnever trips and casting can be starved indefinitely.Also applies to: 1632-1635, 1749-1749
🤖 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 `@core/AttackBot.lua` around lines 1215 - 1254, Persist the Absolute Sweep rotation attempt state outside the per-tick _attackCache so it survives across attackBotMain cycles. In the rotation block that uses bestSweepDir, rotationAttempts, and MAX_ROTATE_ATTEMPTS, store attempts/start time in a longer-lived cache or context field instead of resetting them whenever the sweep matches, so repeated failed turn(desired) calls can eventually stop retrying and allow casting to proceed.
🤖 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 `@core/Containers.lua`:
- Line 1239: The cached container lookup is being used before its local
declaration, so `isContainerOpen` and `openConfiguredContainer` end up resolving
`getCachedContainers` as a global and `pairs(getCachedContainers())` can fail at
runtime. Move or hoist the `getCachedContainers` local function above both
helpers, or forward-declare it before `isContainerOpen` so those functions close
over the local symbol correctly.
In `@targetbot/attack_coordinator.lua`:
- Around line 319-583: TargetBot.Creature.walk now uses combat state that is
never initialized in this scope, so recompute the missing locals before the
decision branches that reference them. Define pathLen, creatureHealth,
targetIsLowHealth, and isTrapped from the current creature/player/combat context
near the top of TargetBot.Creature.walk, then use those values in the
finish-kill, rePosition, and chase checks so pathLen > 1 and the
trapped/low-health logic cannot evaluate nil. Ensure the new assignments are
placed before the first use of those symbols and remain consistent with the
existing MovementCoordinator and nExBot flow.
---
Outside diff comments:
In `@core/heal_engine.lua`:
- Around line 558-588: The raw friend spell list path in HealBot’s healing flow
is using spells without a cooldown key, so the cooldown guard in
`planFriend`/`ready` can never track them correctly. Normalize or derive a
stable `spell.key` before the `ready(spell.key, ...)` check in
`core/heal_engine.lua`, or populate the key earlier in the spell builder used by
`evaluateAlly`, so cooldown stamping works for those entries too.
In `@core/smart_hunt.lua`:
- Around line 414-424: The per-item loot aggregation in smart_hunt.lua is not
using coin valuation, so coin items can be recorded with zero value in
analytics. Update the loot bucket logic inside the entry.items loop to use
COIN_VALUES[nameKey] when available instead of relying only on itm.price, and
keep the existing gold tracking behavior in sync. Use the analytics.lootItems
and COIN_VALUES symbols to locate the block and ensure bucket.value reflects
coin value for top-valued loot.
In `@targetbot/looting.lua`:
- Line 628: The no-space path in `looting.lua` now exits too early because the
final `ContainerOpener.ensureLootContainerSpace(...)` fallback was removed, so
`lootContainers` can be returned empty even when another loot container could
still be opened or prepared. Restore that fallback in the loot collection flow,
or shift it to a caller that is guaranteed to run, and make sure the logic
around `lootContainers` still reaches the space-management step before
returning.
In `@targetbot/monster_ai_core.lua`:
- Around line 107-127: The isValidAliveMonster helper still bypasses the
SafeCreature migration by calling creature:isMonster(), creature:isDead(), and
creature:isRemoved() directly. Update isValidAliveMonster to use the existing
safeIsMonster, safeIsDead, and safeIsRemoved predicates instead of the raw
creature methods, and keep the nil/false handling consistent with the
surrounding SafeCreature wrappers.
In `@targetbot/priority_engine.lua`:
- Around line 97-103: The getPlayer() helper is bypassing the existing safe
position check and calling SC.getPosition directly, which can fail during
partial SafeCreature initialization. Update getPlayer() to use the existing
cPos() helper for the validity check instead of SC.getPosition, while keeping
the rest of the player lookup logic in getClient() and g_game.getLocalPlayer()
unchanged.
---
Major comments:
In `@cavebot/cavebot.lua`:
- Around line 893-904: The fallback selection in cavebot.lua is clearing
startupFocusTarget too early because getFocusedChild() may return a stale child
even when focusChild failed. Update the currentAction resolution near the
focusedChild/startupFocusTarget logic so startupFocusTarget is only cleared
after the intended recovery target is actually used or successfully focused, and
not simply because uiList:getFocusedChild() returned something old. Apply the
same fix to the duplicate selection logic elsewhere in the file that uses
startupFocusTarget and focused child handling.
In `@cavebot/recorder.lua`:
- Around line 121-132: Force-record the landing tile in recorder.lua’s staircase
handling branch: after addStairs(oldPos) updates the recorder state, ensure the
subsequent addPosition(newPos) cannot be skipped by the collinearity fast-path.
Adjust the logic around addPosition, addStairs, prevRecorded, and lastPos so the
post-floor-change landing position is always appended to the route.
In `@core/bot_core/creatures.lua`:
- Around line 41-49: getCachedSpectators currently caches only by the first
argument, so different spectator queries can reuse the wrong result; update the
cache key logic in getCachedSpectators to include all arguments passed to
getSpectators, not just the first one. Make sure the cache comparison and
assignment use a stable composite key derived from the full argument list so
calls like getCachedSpectators(centerPos, patternA) and
getCachedSpectators(centerPos, patternB) do not collide.
In `@core/bot_core/friend_healer.lua`:
- Around line 463-478: The ally heal spell selection in friend_healer.lua is
assigning hardcoded prio values, which ignores the UI’s configured order. Update
the spellList construction in the HealEngine.evaluateAlly path to use the ally
priority order from config instead of fixed numbers. Reference the
config/customSpell, useGranSio, useTioSio, and useSio entries when deriving each
spell’s prio so the user’s reordered priorities are preserved.
- Around line 481-486: The HealEngine spell path is double-appending the target
because `HealEngine.evaluateAlly()` already returns a spell identifier that
includes the friend target, and `executeAction()` then sends it through
`castHealSpellOnFriend()` again. Update the `executeAction()` flow in
`friend_healer.lua` so the `HealEngine.evaluateAlly()` result is treated as the
final castable spell string, or adjust `HealEngine.evaluateAlly()` to return
only the bare spell name and let the legacy formatter handle targeting, but do
not do both. Keep the fix scoped to the `engineAction`/`castHealSpellOnFriend`
path to avoid changing other heal behaviors.
In `@core/bot_core/init.lua`:
- Line 34: The nil fallback for zChanging still dereferences nExBot directly, so
init.lua can crash when nExBot is nil. Update the lookup around zChanging to use
a nil-safe access pattern before falling back to the default function, and keep
the change localized to the initialization logic that defines zChanging.
In `@core/cavebot_control_panel.lua`:
- Around line 3-14: The cavebot control panel still continues into widget setup
even when the OTUI resource fails to load, which can lead to nil-widget crashes.
Update the initialization flow in the cavebot_control_panel module so that after
the g_resources.readFileContents/g_ui.loadUIFromString check, the rest of the
panel/button setup only runs when content is successfully loaded; otherwise exit
or return early after the warn path.
In `@core/character_db.lua`:
- Around line 34-36: The CharacterDB initialization in engine setup only runs
when g_game.getLocalPlayer() is already present, so add a retry path that calls
engine.load() once the local player becomes available after login. Update the
CharacterDB/engine wiring so the load happens both on module startup and on the
player-available event or equivalent hook, ensuring CharacterDB.isReady()
becomes true for cold boots and downstream consumers like follow/target logic
can restore state.
In `@core/client_service.lua`:
- Around line 43-52: `makeDelegate` currently hardcodes the ACL lookup to
`acl.game`, so delegates created for non-game sources never consult their
matching ACL section. Update `makeDelegate` to derive the default `aclPath` from
the delegated `source` (while still allowing `opts.aclPath` to override it), and
verify the map-related delegates such as `getTile` and `getSpectatorsInRange`
now resolve against `acl.map` without requiring per-entry configuration.
In `@core/combo.lua`:
- Around line 232-238: The leaderTarget assignment in combo.lua is happening
before verifying that c1 is the configured shooter, which can let any on-screen
missile overwrite the leader target. Move the leaderTarget update into the same
conditional path in the combo handling logic, after c1:getName():lower() matches
config.shootLeader:lower(), so only the validated shooter can set the target.
Use the existing symbols leaderTarget, c1, t1, and the shooter check in the
combo/leaderTargetHandler flow to locate the fix.
- Around line 211-212: The combo trigger in combo.lua is too broad because
`EventBus.emit("combo:trigger")` fires for any attack spell whenever
`config.enabled` is true. Update the logic in the attack-spell handling path so
it also checks the configured cast leader and respects `onCastEnabled`, matching
only the intended leader’s cast events before emitting `combo:trigger`. Use the
existing `isAttSpell`/`config` flow in the combo handler to scope the trigger
correctly.
- Around line 187-205: The combo command parsing in the text handler is too
loose: `string.find()` in the `ue` and `sd` branches can match unrelated words
like “blue,” and those branches ignore the `attackSpellEnabled` and
`attackItemEnabled` toggles. Update the logic in `core/combo.lua` to require
exact command matches for the `ue`/`sd`/`att` handlers, and gate the
`say(config.spell)`, `useWith(config.item, creature)`, and
`AttackStateMachine.requestAttack(...)` calls behind their corresponding
action-enable flags so only explicitly issued commands trigger actions.
In `@core/Containers.lua`:
- Around line 868-884: The retry logic in ContainerBFS’s timeout handler is
marking a slot as opened too early, which prevents reopening the same container
item when it remains in the same slot after a timeout. Update the safety-timeout
path in ContainerBFS.scanContainer so the retry for entry.parentId/entry.itemId
reuses the same slot without being blocked by ContainerBFS.opened, and ensure
the slot is only recorded as opened after a successful open rather than before
queueing the retry.
In `@core/creature_cache.lua`:
- Around line 441-464: CreatureCache.getSpectators currently reuses
cache.creatures based only on TTL, so different rangeX/rangeY queries can return
stale spectators from a previous range. Update the caching logic in
getSpectators (and any helper it relies on, such as updateFromSpectators/cache
metadata) so the cached result is keyed or validated by the requested range as
well as ttl. Make sure a change in rangeX or rangeY forces a refresh instead of
returning the previous query’s view.
In `@core/Dropper.lua`:
- Around line 76-78: The local aliasing in Dropper configuration setup is
masking a missing SharedHelpers dependency by falling back to an empty table,
which leaves getProfileSetting and setProfileSetting nil and causes
saveDropperConfig() to crash later. Update the SharedHelpers handling so
Dropper.lua either preserves the existing inline storage fallback or fails fast
with an explicit assertion when nExBot.SharedHelpers is unavailable, and make
sure saveDropperConfig() still has a valid setProfileSetting path.
In `@core/equip.lua`:
- Around line 9-11: The SharedHelpers setup in equip.lua can leave
getProfileSetting and setProfileSetting nil when nExBot.SharedHelpers is
missing, causing call-to-nil failures on config read/write. Update the
initialization around SharedHelpers, getProfileSetting, and setProfileSetting to
use the same guard/fail-fast pattern as elsewhere in the codebase, or restore
the previous inline fallback behavior so these helpers are always available
before equip logic uses them.
In `@core/event_bus.lua`:
- Around line 721-728: The current throttling in EventBus.emit for creature:move
drops intermediate updates and can miss the final move, so move consumers fall
behind. Update the creature move throttle logic to coalesce events by keeping
the latest pending creature/oldPos for each cId and scheduling a delayed emit
when calls arrive inside CREATURE_MOVE_THROTTLE_MS. Use the existing
_creatureMoveLastEmit, cId, and nowMs5 flow to preserve throttling while
ensuring the most recent move is eventually emitted instead of discarded.
In `@core/extras.lua`:
- Around line 181-192: The tick handler in extras.lua rethrows failures from the
g_window.setTitle path after pcall, which turns a title-update problem into a
hard error. Update the setTitle logic in the settings.title block to log the
failure once instead of calling error, and then disable further title updates so
the Extras loop can continue. Use the existing ok/err handling around
g_window.setTitle and keep the fix localized to the title update path.
In `@core/follow.lua`:
- Around line 264-310: The follow logic in the main follow loop is still
performing local movement immediately after `registerFollowIntent`, which
bypasses MovementCoordinator’s priority gating. Update the catch-up paths in
`follow.lua` so the follow intent is only registered and movement is deferred to
the coordinator instead of calling `walkStep()` directly, especially in the
`isAttacking()` / `followWhileAttacking` and non-attacking catch-up branches.
Make sure the `startFollow(leader)` fallback does not reintroduce local movement
and that follow only advances when the movement priority system allows it.
- Around line 235-236: The guard in follow logic is checking the WaypointEngine
function itself instead of its runtime state, so Follow exits even when walking
is not active. Update the condition in the follow flow to call
WaypointEngine.isWalking() alongside the existing CaveBot check, using the same
symbols in the branch that currently returns early.
In `@core/HealBot.lua`:
- Around line 671-672: Guard the fallback local-player lookup in HealBot before
calling ClientService.getLocalPlayer(), since BotCore/ClientService may be
unavailable and direct access can crash; update the affected stats-return paths
in HealBot’s fallback logic to first check that ClientService exists, then
safely return the default hp/mp table when no local player is available. Apply
the same protection to the other fallback block referenced in the HealBot code
so both lookup paths behave consistently.
- Around line 1206-1208: The ally-filter branch in HealBot.lua reads
storage.extras.checkPlayer without verifying that storage.extras exists, which
can crash when extras storage is not initialized. Update the category == 2
condition to guard storage.extras before accessing checkPlayer, and keep the
existing label:setColor and label:setTooltip behavior in the same block. Use the
HealBot logic around the category check to ensure the checkPlayer flag is only
read when extras storage is present.
- Around line 401-406: `ui.allySetup.onClick` currently closes over
`friendHealerWindow` before it is locally declared, so it may resolve the global
instead of the intended window instance. Move the `local friendHealerWindow`
declaration above the `ui.allySetup.onClick` handler in `HealBot.lua` so the
click callback references the correct local variable when showing, raising, and
focusing the window.
- Around line 1011-1029: Make loadAllyCustomPlayers map-aware and resilient to
delayed CharacterDB readiness: the current `#customPlayers` checks treat the
name-keyed ally map as empty, so saved friends never reload, and the single
schedule(500, loadAllyCustomPlayers) call can miss when CharacterDB is not ready
yet. Update loadAllyCustomPlayers to detect populated tables using pairs/keys
rather than length, keep syncing allyConfig.customPlayers back to CharacterDB
when needed, and add a retry/polling path so the load runs again until
CharacterDB.isReady() succeeds.
- Around line 1485-1488: The spectator scan in HealBot’s healing loop is
stopping too early because the `break` inside the `isCustom` check exits the
entire scan when one custom player is healthy. Update the logic in the `curHp`,
`isCustom`, and `customPlayers` block so healthy custom players are skipped with
`continue`/equivalent flow control instead of breaking the outer loop, allowing
later spectators to still be evaluated for healing.
In `@core/unified_storage.lua`:
- Around line 142-158: The EventBus registration in schedule is currently a
one-shot check, so if EventBus is still unavailable after 100 ms the
UnifiedStorage handlers never get attached. Update the unified_storage.lua
startup flow so it keeps retrying or otherwise defers registration until
EventBus exists, and make sure the handlers for targetbot/cavebot config
changes, macro/module toggles, monsterAI updates, player:logout, and tick:slow
are all registered once EventBus is ready; keep the existing UnifiedStorage.load
initialization path intact.
In `@targetbot/attack_coordinator.lua`:
- Around line 609-613: The LURE intent registration in the attack coordinator is
effectively a no-op because it uses the player’s current position, which makes
Execute.move() short-circuit with already_at_target before the INTENT.LURE
handling in movement_coordinator.lua can run. Update the caller around
MovementCoordinator.Intent.register so it passes a real lure target instead of
player:getPosition(), or adjust Execute.move()/the LURE branch so the intent
still reaches TargetBot.allowCaveBot(150) when the player is already at the
target.
In `@targetbot/attack_spells.lua`:
- Around line 55-68: The fallback in TargetBot.sayAttackSpell should not call
say(text) directly, since that can fail on clients that only expose
g_game.say/talk APIs. Update the fallback branch to reuse doSay() instead,
keeping the existing delay check, lastAttackSpell update, and
HuntAnalytics.trackAttackSpell behavior unchanged.
In `@targetbot/attack_state_machine.lua`:
- Around line 203-204: The updatePlayer() tick path is dereferencing SC too
early, so guard the SafeCreature lookup before calling SC.getPosition and only
use it after ensureDeps() has populated it. Update the lazy-load flow in
updatePlayer() to re-check or initialize dependencies before querying the player
position, and keep the player refresh logic intact when SC is unavailable or
partially loaded.
- Around line 427-430: The hold-target reacquisition path in
attack_state_machine.lua should not call CreatureCache.getNearby directly
without a boot-safety guard. Update the pcall around the nearby-spec lookup so
it first checks that CreatureCache and CreatureCache.getNearby are available,
and fall back to the existing empty-table default when they are not. Keep the
logic in the same ASM idle/hold-target branch so attack_state_machine behavior
remains consistent during early startup.
In `@targetbot/attack_waves.lua`:
- Around line 366-372: The avoidance bookkeeping in the safePos branch is being
advanced before confirming that a move was actually issued. Update the logic in
attack_waves.lua around the safePos handling so MovementCoordinator.canMove() is
checked first, and only then call TargetBot.walkTo and update
avoidanceState.lastMove, avoidanceState.lastSafePos, and
avoidanceState.consecutiveMoves. If movement is not allowed, do not return true
or start the cooldown state, so the bot can retry avoidance on the next pass.
- Around line 140-159: The inPredPath check in attack_waves.lua is using pcall()
as the predicate, so successful false returns are being treated as hits and
incorrectly raising waveThreats and totalDanger. Update the logic around
MonsterAI.Predictor.isPositionInWavePath so pcall only guards against errors,
then use the function’s actual boolean result to decide whether the threat block
should run, preserving error safety without converting false into true.
In `@targetbot/core.lua`:
- Line 285: The visited-node key in key() is too coarse and can collide for
different coordinates, causing distinct positions to be treated as already
visited. Update the key generation in targetbot/core.lua to use a collision-free
composite representation for p.x, p.y, and p.z, and make sure all callers that
rely on this key continue to compare coordinates uniquely.
In `@targetbot/event_targeting.lua`:
- Around line 902-903: The current-target dead check still calls
currentTarget:isDead() directly in the ClientService path, which can fail for
stale or adapter-backed creatures. Update the affected targeting logic in
event_targeting.lua to wrap currentTarget with SafeCreature before checking
death, and use the SafeCreature-based dead check in both the current block and
the other matching ClientService branch around the referenced target-selection
logic.
- Around line 1811-1813: The fallback logic in
ClientService.getAttackingCreature is unsafe because it still invokes the method
when ClientService or the method may be absent. Update the currentAttack
assignment to only call getAttackingCreature after confirming ClientService
exists and the method is present, and otherwise return nil; keep the surrounding
throttling logic in event_targeting.lua unchanged.
- Around line 54-57: The fallback loader in ensurePathUtils is not guarding the
optional SharedHelpers dependency, so it can throw when nExBot.SharedHelpers or
its ensurePathUtils method is missing. Update ensurePathUtils to check that
SharedHelpers exists and that ensurePathUtils is callable before invoking it,
and otherwise return nil from this helper. Keep the existing PathUtils re-check
behavior after the guarded load so the function still resolves the global when
available.
In `@targetbot/monster_ai.lua`:
- Line 1302: Re-filter the results from CreatureCache.getNearby in
monster_ai.lua before any range-sensitive work, because cache hits can return a
previously wider set that includes out-of-range creatures. Update the logic
around the creatures local used by the damage attribution and updateAll()
processing paths so only creatures within CONST.DAMAGE.CORRELATION_RADIUS are
kept before continuing, and apply the same fix in the other affected occurrence
as well.
In `@targetbot/monster_scenario.lua`:
- Around line 432-434: The target-lock handler in monster_scenario.lua is using
a raw creature object directly, which can be stale or invalid. Update the logic
around the TargetBot.isOff check and the S.lockTarget call to use the
SafeCreature helpers instead of calling creature:getId() or
creature:getHealthPercent() directly. Make sure the callback resolves the
creature through the SafeCreature migration path before locking, and only falls
back to S.clearTargetLock() when no valid safe creature is available.
---
Minor comments:
In `@core/AttackBot.lua`:
- Around line 801-802: The source navigation controls in AttackBot are left with
empty onClick handlers, which makes them appear enabled while doing nothing.
Update the panel.previousSource and panel.nextSource handling to either disable
or hide these controls when source switching is unavailable, or wire them to a
user-visible warning. Use the existing panel.previousSource and panel.nextSource
setup in AttackBot to locate the change.
In `@core/bot_database.lua`:
- Around line 36-47: BotDB.registerMacro still uses the legacy storage table for
macro state, so the new StorageEngine cache is bypassed during restore and
migration. Update registerMacro and migrateOldData to read/write macro state
through BotDB (or a BotDB.macros namespace) instead of storage, and keep the
existing behavior in setOn/setOff and the saved/onEnable flow while switching
the persistence source to the BotDB cache.
In `@core/combo.lua`:
- Around line 264-282: The follow target state in followLeaderHandler can stay
stale when no current rule matches, causing an old target to persist after
config.follow changes or leaderTarget is no longer a player. Update
followLeaderHandler so it always clears toFollow before evaluating the current
follow rules, and only assigns a new value when a valid LEADER TARGET,
sayLeader, castLeader, or shootLeader matches. Ensure the final early return
still leaves toFollow nil when nothing matches.
In `@core/Containers.lua`:
- Around line 805-806: The BFS queueing logic in ContainerBFS is ignoring the
prioritize flag, so prioritized nested containers are always appended behind
existing entries. Update the queue insertion path in the ContainerBFS logic to
branch on prioritize and place prioritized entries ahead of normal queued items
while preserving the existing behavior for non-prioritized cases. Use the
ContainerBFS queueing code and the prioritize parameter accepted by the BFS
helper to locate the fix.
---
Nitpick comments:
In `@core/AttackBot.lua`:
- Around line 1215-1254: Persist the Absolute Sweep rotation attempt state
outside the per-tick _attackCache so it survives across attackBotMain cycles. In
the rotation block that uses bestSweepDir, rotationAttempts, and
MAX_ROTATE_ATTEMPTS, store attempts/start time in a longer-lived cache or
context field instead of resetting them whenever the sweep matches, so repeated
failed turn(desired) calls can eventually stop retrying and allow casting to
proceed.
🪄 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: 20d94c02-bda4-4551-bdbf-55af4f30173a
📒 Files selected for processing (115)
README.md_Loader.luacavebot/actions.luacavebot/bank.luacavebot/cavebot.luacavebot/clear_tile.luacavebot/config.luacavebot/d_withdraw.luacavebot/editor.luacavebot/minimap.luacavebot/recorder.luacavebot/tasker.luacavebot/walking.luaconstants/directions.luaconstants/floor_items.luaconstants/food_items.luacore/AttackBot.luacore/Conditions.luacore/Containers.luacore/Containers.otuicore/Dropper.luacore/Equipper.luacore/HealBot.luacore/acl/adapters/base.luacore/acl/adapters/opentibiabr.luacore/acl/adapters/otcv8.luacore/acl/compat.luacore/acl/init.luacore/acl/interfaces.luacore/alarms.luacore/analyzer.luacore/bot_core/actions.luacore/bot_core/analytics.luacore/bot_core/attack_system.luacore/bot_core/conditions.luacore/bot_core/cooldown.luacore/bot_core/creatures.luacore/bot_core/friend_healer.luacore/bot_core/init.luacore/bot_core/items.luacore/bot_core/position.luacore/bot_core/priority.luacore/bot_core/stats.luacore/bot_database.luacore/cavebot_control_panel.luacore/cavebot_control_panel.otuicore/character_db.luacore/client_service.luacore/combo.luacore/combo.otuicore/creature_cache.luacore/depot_withdraw.luacore/equip.luacore/event_bus.luacore/exeta.luacore/extras.luacore/follow.luacore/global_config.luacore/heal_engine.luacore/lib.luacore/new_cavebot_lib.luacore/new_healer.luacore/outfit_cloner.luacore/pushmax.luacore/quiver_label.luacore/smart_hunt.luacore/spy_level.luacore/tools.luacore/unified_storage.luacore/unified_tick.luacore/updater.luadocs/ARCHITECTURE.mddocs/CAVEBOT.mddocs/FOLLOW.mddocs/TARGETBOT.mdtargetbot/attack_coordinator.luatargetbot/attack_spells.luatargetbot/attack_state_machine.luatargetbot/attack_waves.luatargetbot/auto_tuner.luatargetbot/chase_controller.luatargetbot/combat_constants.luatargetbot/core.luatargetbot/creature_attack.luatargetbot/creature_editor.luatargetbot/event_targeting.luatargetbot/helpers.luatargetbot/looting.luatargetbot/monster_ai.luatargetbot/monster_ai_core.luatargetbot/monster_combat_feedback.luatargetbot/monster_inspector.luatargetbot/monster_patterns.luatargetbot/monster_prediction.luatargetbot/monster_reachability.luatargetbot/monster_scenario.luatargetbot/monster_spell_tracker.luatargetbot/monster_tbi.luatargetbot/monster_tracking.luatargetbot/movement_coordinator.luatargetbot/opentibiabr_targeting.luatargetbot/priority_engine.luatargetbot/target.luatargetbot/target_coordinator.luatargetbot/target_events.luatargetbot/target_pathfinding.luatargetbot/walking.luautils/path_strategy.luautils/path_utils.luautils/safe_creature.luautils/shared.luautils/shared_helpers.luautils/storage_engine.luautils/waypoint_navigator.luautils/weak_cache.lua
💤 Files with no reviewable changes (39)
- core/quiver_label.lua
- targetbot/monster_spell_tracker.lua
- constants/food_items.lua
- core/outfit_cloner.lua
- core/global_config.lua
- core/bot_core/items.lua
- cavebot/bank.lua
- targetbot/auto_tuner.lua
- cavebot/d_withdraw.lua
- constants/floor_items.lua
- targetbot/creature_editor.lua
- core/bot_core/cooldown.lua
- core/acl/compat.lua
- targetbot/monster_combat_feedback.lua
- core/bot_core/conditions.lua
- core/bot_core/position.lua
- core/alarms.lua
- constants/directions.lua
- core/bot_core/actions.lua
- cavebot/tasker.lua
- cavebot/config.lua
- core/new_healer.lua
- core/acl/init.lua
- core/new_cavebot_lib.lua
- core/bot_core/analytics.lua
- cavebot/clear_tile.lua
- core/Conditions.lua
- core/acl/adapters/otcv8.lua
- core/updater.lua
- core/acl/adapters/opentibiabr.lua
- targetbot/monster_tracking.lua
- core/acl/interfaces.lua
- core/acl/adapters/base.lua
- targetbot/monster_patterns.lua
- core/analyzer.lua
- core/depot_withdraw.lua
- targetbot/chase_controller.lua
- targetbot/monster_inspector.lua
- core/Equipper.lua
There was a problem hiding this comment.
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 (3)
targetbot/attack_spells.lua (1)
61-67: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOnly consume cooldown after
doSay()succeeds.This fallback path returns
trueeven whendoSay(text)fails, and it still advanceslastAttackSpell. A transient speech failure will therefore block the next retry for the full cooldown while reporting a cast that never happened.Suggested fix
if lastAttackSpell + delay < now then - doSay(text) + if not doSay(text) then + return false + end lastAttackSpell = now if HuntAnalytics and HuntAnalytics.trackAttackSpell then HuntAnalytics.trackAttackSpell(text, 0)🤖 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/attack_spells.lua` around lines 61 - 67, The fallback path in `attack_spells.lua` advances `lastAttackSpell` before knowing whether `doSay(text)` succeeded, so move the cooldown update and `HuntAnalytics.trackAttackSpell` call to only happen after a successful `doSay()` result. Update the `lastAttackSpell` handling in this `if lastAttackSpell + delay < now then` block so a failed speech attempt does not consume the cooldown or report a cast, and keep the return value aligned with the actual `doSay()` outcome.core/client_service.lua (1)
50-92: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCheck delegate targets are callable before invoking them.
The dynamic dispatch now calls any truthy table/global value. A non-function ACL entry, subsystem field, or global with the same name will crash the wrapper.
Proposed fix pattern
local aclSection = acl[aclPath] - if aclSection and aclSection[aclKey] then - return aclSection[aclKey](...) + local aclFn = aclSection and aclSection[aclKey] + if type(aclFn) == "function" then + return aclFn(...) end- if g_game and g_game[methodName] then - return g_game[methodName](...) + local fn = g_game and g_game[methodName] + if type(fn) == "function" then + return fn(...) endlocal globalFn = rawget(_G, methodName) - if globalFn then + if type(globalFn) == "function" then return globalFn(...) endAlso applies to: 95-108
🤖 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 `@core/client_service.lua` around lines 50 - 92, The dynamic dispatch in client_service.lua currently only checks that ACL entries and subsystem/global lookups are truthy before calling them, which can crash if the target is not callable. Update the dispatch paths in the client service wrapper (the ACL lookup and the g_game/g_map/g_ui/g_resources/g_window/g_platform/g_keyboard/g_settings branches) to verify the resolved target is a function before invoking it, and only return when the callable check passes; otherwise continue to the next source or fall through safely.cavebot/walking.lua (1)
368-380: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReturn
falsewhen no floor-change movement is dispatched.This branch returns
trueeven whenfindPathfails, the first step is blocked, or the unsafe fallback has no walkable adjacent tile. Callers treattrueas progress, so recovery can leave its guarded path without moving.Proposed fix
if fcPath and `#fcPath` > 0 then local dir = fcPath[1] local smoothed = PS().smoothDirection(dir, true) or dir if canWalkDirection(smoothed) then PS().walkStep(smoothed) + return true elseif canWalkDirection(dir) then PS().walkStep(dir) + return true end end - return true + return false else -- Far: guarded autoWalk local isSafe = PS().nativePathIsSafe(playerPos, walkDest, {ignoreNonPathable = true}) if isSafe then - PS().autoWalk(walkDest, maxDist, {precision = precision}) + return PS().autoWalk(walkDest, maxDist, {precision = precision}) ~= false else local dirToDest = getDirectionTo(playerPos, walkDest) if dirToDest and canWalkDirection(dirToDest) then local off = DIR_TO_OFFSET[dirToDest] local target = off and {x = playerPos.x + off.x, y = playerPos.y + off.y, z = playerPos.z} if target and PathUtils.isTileWalkable(target) then PS().walkStep(dirToDest) + return true end end end - return true + return falseAlso applies to: 383-396
🤖 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/walking.lua` around lines 368 - 380, The close-range movement branch in walking.lua currently returns true even when no step is actually dispatched, which makes callers think progress happened. Update the logic around the path/fallback handling in the movement routine (the branch using findPath, smoothDirection, canWalkDirection, and walkStep) so it only returns true after a walkStep is issued, and returns false when findPath fails or both the smoothed and raw directions are blocked. Apply the same return-value fix to the analogous fallback branch referenced by the comment so guarded recovery only advances when a real floor-change move occurs.
🧹 Nitpick comments (2)
docs/CONTAINERS.md (2)
192-195: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake troubleshooting guidance more actionable.
Instead of "check that you don't have too many backpacks assigned," specify a concrete guideline (e.g., "keep total assigned containers under 15–18 to stay safely below the 19-container guard").
🤖 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 `@docs/CONTAINERS.md` around lines 192 - 195, The troubleshooting note under the Containers guidance is too vague; replace the “check that you don’t have too many backpacks assigned” wording with a concrete limit. Update the text in the “Containers closing immediately after opening” section to give an actionable recommendation tied to the existing 19-container guard, such as keeping total assigned containers safely under 15–18, so readers can follow a clear guideline.
56-87: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueClarify the safety margin for container limit.
The documentation states the "server-side container limit (max 19 open)" (line 64) but the troubleshooting section later describes it as "limit of ~20" (line 193). Consider explicitly stating that 19 is a deliberate safety margin below the ~20 server limit to prevent rejection, so users understand why the number differs.
🤖 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 `@docs/CONTAINERS.md` around lines 56 - 87, Clarify the container limit wording in the Container Panel docs by making the “max 19 open” limit and the later “~20” troubleshooting reference consistent. Update the sections describing the server-side limit, safety timeout, and related behavior to state that 19 is an intentional safety margin below the approximate 20-container server limit, so readers understand the discrepancy.
🤖 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 599-641: The floor-change recovery in findNearestFloorChangeTile
and the recovery block that calls CaveBot.walkTo should not complete just
because a nearby stair/ladder was found or movement started. Use targetFloors to
filter candidates so only tiles leading toward a floor with
waypointPositionCache entries are selected, and verify the player actually
changed to one of those floors before calling transitionTo("NORMAL"). If the
floor transition does not happen, keep recovery active instead of returning
success.
In `@core/analyzer.lua`:
- Around line 1602-1605: The hourVal helper currently divides by uptime without
checking whether it is nil or zero, which can break during startup/reset. Update
hourVal to guard the uptime value before the division, returning a safe fallback
(such as 0) when uptime is unset or zero. Keep the fix localized to hourVal and
preserve the existing call pattern where v defaults to 0.
- Around line 1607-1617: avgTable() currently sums entries with pairs() but
divides by `#t`, which can be wrong for keyed or sparse tables. Update avgTable in
core/analyzer.lua to count the actual number of iterated values while
accumulating, then divide by that counted total instead of using `#t`; keep the
existing zero-table guard behavior for empty or non-table inputs.
In `@core/bot_core/creatures.lua`:
- Around line 42-45: The cache key built in getCachedSpectators is too weak
because it only uses nargs and args[1], causing different multi-argument
spectator queries to collide. Update the key generation in the
getCachedSpectators logic to incorporate all relevant arguments in args (not
just the first one) so distinct patterns and positions map to different cache
entries, preserving correct results for getInArea().
- Around line 379-390: Creatures.getNearby currently only changes how spectators
are fetched, but it still returns every non-local spectator regardless of the
requested range. Update the function to apply the requested rangeX and rangeY
bounds when building the result so only creatures within the caller’s radius are
included. Use the existing Creatures.getNearby flow and the spec:isLocalPlayer()
check, but add coordinate/distance filtering before appending to result.
In `@core/Conditions.lua`:
- Line 30: Add a backward-compatible fallback for the legacy typo key so
existing HealBot.json profiles still enable poison curing. Update the config
loading/default handling around the curePoison setting in Conditions.lua to read
from both curePoison and the legacy curePosion key, preserving the current
curePoison symbol while mapping the old profile data into it.
In `@core/event_bus.lua`:
- Line 433: The creature cache lookup in event handling only guards against a
nil return, so loading/order changes can still fail if BotCore,
BotCore.Creatures, or getNearby is missing. Update the cache assignment in the
event bus logic to safely verify the BotCore.Creatures dependency before calling
getNearby, and fall back to an empty table when the dependency or method is
unavailable. Use the existing _damageAttrCachedCreatures assignment site to keep
the guard localized and non-invasive.
- Around line 686-691: The scheduled callback in EventBus.emit for
"creature:move" is recording a stale throttle timestamp by assigning
_creatureMoveLastEmit[cId] from the suppressed event time instead of the actual
delayed emit time. Update the callback to set the last-emit marker using the
current time at callback execution, and keep the logic localized in the
schedule(...) handler and _creatureMovePending/_creatureMoveLastEmit bookkeeping
so the next throttle check uses the real emission time.
In `@core/extras.lua`:
- Around line 191-193: Stop the repeated setTitle attempts after the first
unsupported call: in the extras title-update path where g_window.setTitle is
invoked, make sure the failure in the setTitle handling branch disables future
calls for that same session/state, not just settings.title. Update the logic
around the existing warn("[Extras] setTitle failed: ...") path so the code
checks the persisted flag before each tick and skips calling g_window.setTitle
once it has failed.
In `@core/quiver_manager.lua`:
- Around line 41-42: `QuiverManager` is iterating directly over
`nExBot.Shared.getContainers()` without guarding against `nExBot.Shared` being
uninitialized or the call returning nil. Add a small local helper in
`core/quiver_manager.lua` that normalizes the container list to an empty table
when unavailable, then use that helper at both `pairs(containers)` call sites so
the iteration is always safe and consistent.
In `@core/supplies.lua`:
- Line 89: Restore the empty-panel cleanup in clearEmptyPanels() so it actually
removes leftover blank rows instead of being a no-op. Update the item-clearing
flow that calls clearEmptyPanels(), and verify the add-item path still appends a
fresh blank panel only when needed. Check the supply-panel logic around
clearEmptyPanels() and the related item insertion/removal behavior to ensure
there is only one trailing "blank" panel after edits.
- Line 118: The duplicate check in the supplies setup is looking at
config[tostring(id)] instead of the same collection used for storage, so it
misses existing entries. Update the logic in the supplies-loading flow (around
the duplicate guard and the insert into config.items[tostring(id)]) to check
config.items[tostring(id)] before creating a new panel, and keep the storage key
consistent so repeated ids are properly rejected.
---
Outside diff comments:
In `@cavebot/walking.lua`:
- Around line 368-380: The close-range movement branch in walking.lua currently
returns true even when no step is actually dispatched, which makes callers think
progress happened. Update the logic around the path/fallback handling in the
movement routine (the branch using findPath, smoothDirection, canWalkDirection,
and walkStep) so it only returns true after a walkStep is issued, and returns
false when findPath fails or both the smoothed and raw directions are blocked.
Apply the same return-value fix to the analogous fallback branch referenced by
the comment so guarded recovery only advances when a real floor-change move
occurs.
In `@core/client_service.lua`:
- Around line 50-92: The dynamic dispatch in client_service.lua currently only
checks that ACL entries and subsystem/global lookups are truthy before calling
them, which can crash if the target is not callable. Update the dispatch paths
in the client service wrapper (the ACL lookup and the
g_game/g_map/g_ui/g_resources/g_window/g_platform/g_keyboard/g_settings
branches) to verify the resolved target is a function before invoking it, and
only return when the callable check passes; otherwise continue to the next
source or fall through safely.
In `@targetbot/attack_spells.lua`:
- Around line 61-67: The fallback path in `attack_spells.lua` advances
`lastAttackSpell` before knowing whether `doSay(text)` succeeded, so move the
cooldown update and `HuntAnalytics.trackAttackSpell` call to only happen after a
successful `doSay()` result. Update the `lastAttackSpell` handling in this `if
lastAttackSpell + delay < now then` block so a failed speech attempt does not
consume the cooldown or report a cast, and keep the return value aligned with
the actual `doSay()` outcome.
---
Nitpick comments:
In `@docs/CONTAINERS.md`:
- Around line 192-195: The troubleshooting note under the Containers guidance is
too vague; replace the “check that you don’t have too many backpacks assigned”
wording with a concrete limit. Update the text in the “Containers closing
immediately after opening” section to give an actionable recommendation tied to
the existing 19-container guard, such as keeping total assigned containers
safely under 15–18, so readers can follow a clear guideline.
- Around line 56-87: Clarify the container limit wording in the Container Panel
docs by making the “max 19 open” limit and the later “~20” troubleshooting
reference consistent. Update the sections describing the server-side limit,
safety timeout, and related behavior to state that 19 is an intentional safety
margin below the approximate 20-container server limit, so readers understand
the discrepancy.
🪄 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: 18505b21-d502-4560-9cf2-c12c5c4edbfd
📒 Files selected for processing (66)
README.mdcavebot/actions.luacavebot/bank.luacavebot/buy_supplies.luacavebot/cavebot.luacavebot/d_withdraw.luacavebot/recorder.luacavebot/walking.luacore/Conditions.luacore/Containers.luacore/Dropper.luacore/HealBot.luacore/analyzer.luacore/bot_core/attack_system.luacore/bot_core/creatures.luacore/bot_core/init.luacore/cavebot_control_panel.luacore/character_db.luacore/client_service.luacore/combo.luacore/combo.otuicore/configs.luacore/container_opener.luacore/creature_cache.luacore/eat_food.luacore/equip.luacore/event_bus.luacore/exeta.luacore/extras.luacore/follow.luacore/heal_engine.luacore/lib.luacore/new_cavebot_lib.luacore/quiver_manager.luacore/smart_hunt.luacore/spy_level.luacore/supplies.luacore/tools.luacore/unified_storage.luacore/unified_tick.luacore/updater.luadocs/ARCHITECTURE.mddocs/CAVEBOT.mddocs/CONTAINERS.mddocs/EXTRAS.mddocs/FOLLOW.mddocs/PERFORMANCE.mddocs/TARGETBOT.mdtargetbot/attack_coordinator.luatargetbot/attack_spells.luatargetbot/attack_state_machine.luatargetbot/attack_waves.luatargetbot/core.luatargetbot/event_targeting.luatargetbot/looting.luatargetbot/monster_ai.luatargetbot/monster_ai_core.luatargetbot/monster_inspector.luatargetbot/monster_scenario.luatargetbot/monster_tbi.luatargetbot/movement_coordinator.luatargetbot/priority_engine.luatargetbot/target_coordinator.luatargetbot/target_events.luautils/shared.luautils/storage_engine.lua
💤 Files with no reviewable changes (8)
- README.md
- core/container_opener.lua
- core/spy_level.lua
- core/combo.otui
- core/updater.lua
- core/lib.lua
- core/unified_tick.lua
- core/heal_engine.lua
✅ Files skipped from review due to trivial changes (5)
- docs/FOLLOW.md
- docs/EXTRAS.md
- docs/ARCHITECTURE.md
- docs/PERFORMANCE.md
- docs/CAVEBOT.md
🚧 Files skipped from review as they are similar to previous changes (18)
- core/Dropper.lua
- core/equip.lua
- core/cavebot_control_panel.lua
- cavebot/bank.lua
- core/character_db.lua
- core/tools.lua
- cavebot/recorder.lua
- targetbot/attack_state_machine.lua
- core/bot_core/attack_system.lua
- core/bot_core/init.lua
- core/follow.lua
- core/HealBot.lua
- core/unified_storage.lua
- core/combo.lua
- targetbot/attack_waves.lua
- targetbot/attack_coordinator.lua
- core/Containers.lua
- core/smart_hunt.lua
Comment on lines
41
to
42
| local containers = nExBot.Shared.getContainers() | ||
| for _, container in pairs(containers) do |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Normalize container lookup before iterating.
pairs(containers) will throw if nExBot.Shared is not initialized or getContainers() returns nil. Keep the previous fallback/empty-table behavior in one local helper and use it in both call sites.
Proposed fix
+ local function getOpenContainers()
+ if nExBot and nExBot.Shared and nExBot.Shared.getContainers then
+ return nExBot.Shared.getContainers() or {}
+ end
+ local Client = getClient and getClient()
+ if Client and Client.getContainers then
+ return Client.getContainers() or {}
+ end
+ if g_game and g_game.getContainers then
+ return g_game.getContainers() or {}
+ end
+ return {}
+ end
+
local function findAmmoItem(ammoIds)
- local containers = nExBot.Shared.getContainers()
+ local containers = getOpenContainers() local function findDestContainer(quiverContainer)
- local containers = nExBot.Shared.getContainers()
+ local containers = getOpenContainers()Also applies to: 104-105
🤖 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 `@core/quiver_manager.lua` around lines 41 - 42, `QuiverManager` is iterating
directly over `nExBot.Shared.getContainers()` without guarding against
`nExBot.Shared` being uninitialized or the call returning nil. Add a small local
helper in `core/quiver_manager.lua` that normalizes the container list to an
empty table when unavailable, then use that helper at both `pairs(containers)`
call sites so the iteration is always safe and consistent.
Comment on lines
+116
to
+133
| function TargetCore.isAdjacent(pos1, pos2) | ||
| local dx = math.abs(pos1.x - pos2.x) | ||
| local dy = math.abs(pos1.y - pos2.y) | ||
| return dx <= 1 and dy <= 1 and (dx + dy) > 0 | ||
| end | ||
|
|
||
| function TargetCore.getDirection(pos1, pos2) | ||
| local dx = pos2.x - pos1.x | ||
| local dy = pos2.y - pos1.y | ||
| if dx == 0 and dy < 0 then return 0 end | ||
| if dx > 0 and dy == 0 then return 1 end | ||
| if dx == 0 and dy > 0 then return 2 end | ||
| if dx < 0 and dy == 0 then return 3 end | ||
| if dx > 0 and dy < 0 then return 4 end | ||
| if dx > 0 and dy > 0 then return 5 end | ||
| if dx < 0 and dy > 0 then return 6 end | ||
| if dx < 0 and dy < 0 then return 7 end | ||
| return nil |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject cross-floor positions in geometry helpers.
isAdjacent() and getDirection() currently ignore z, so positions on different floors can be treated as adjacent or produce a walk direction.
Proposed fix
function TargetCore.isAdjacent(pos1, pos2)
+ if not pos1 or not pos2 or pos1.z ~= pos2.z then return false end
local dx = math.abs(pos1.x - pos2.x)
local dy = math.abs(pos1.y - pos2.y)
return dx <= 1 and dy <= 1 and (dx + dy) > 0
end
function TargetCore.getDirection(pos1, pos2)
+ if not pos1 or not pos2 or pos1.z ~= pos2.z then return nil end
local dx = pos2.x - pos1.x
local dy = pos2.y - pos1.y📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function TargetCore.isAdjacent(pos1, pos2) | |
| local dx = math.abs(pos1.x - pos2.x) | |
| local dy = math.abs(pos1.y - pos2.y) | |
| return dx <= 1 and dy <= 1 and (dx + dy) > 0 | |
| end | |
| function TargetCore.getDirection(pos1, pos2) | |
| local dx = pos2.x - pos1.x | |
| local dy = pos2.y - pos1.y | |
| if dx == 0 and dy < 0 then return 0 end | |
| if dx > 0 and dy == 0 then return 1 end | |
| if dx == 0 and dy > 0 then return 2 end | |
| if dx < 0 and dy == 0 then return 3 end | |
| if dx > 0 and dy < 0 then return 4 end | |
| if dx > 0 and dy > 0 then return 5 end | |
| if dx < 0 and dy > 0 then return 6 end | |
| if dx < 0 and dy < 0 then return 7 end | |
| return nil | |
| function TargetCore.isAdjacent(pos1, pos2) | |
| if not pos1 or not pos2 or pos1.z ~= pos2.z then return false end | |
| local dx = math.abs(pos1.x - pos2.x) | |
| local dy = math.abs(pos1.y - pos2.y) | |
| return dx <= 1 and dy <= 1 and (dx + dy) > 0 | |
| end | |
| function TargetCore.getDirection(pos1, pos2) | |
| if not pos1 or not pos2 or pos1.z ~= pos2.z then return nil end | |
| local dx = pos2.x - pos1.x | |
| local dy = pos2.y - pos1.y | |
| if dx == 0 and dy < 0 then return 0 end | |
| if dx > 0 and dy == 0 then return 1 end | |
| if dx == 0 and dy > 0 then return 2 end | |
| if dx < 0 and dy == 0 then return 3 end | |
| if dx > 0 and dy < 0 then return 4 end | |
| if dx > 0 and dy > 0 then return 5 end | |
| if dx < 0 and dy > 0 then return 6 end | |
| if dx < 0 and dy < 0 then return 7 end | |
| return nil |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation