Autopilot enhancements: targeted poison, quest mode, hook extraction #134
Conversation
Re-adds targeted poison (per-player and per-beast with custom amounts), quest mode with filter-based beast prioritization, advanced beast selection via selectOptimalBeasts, and between-batch halt conditions in attack loop. Adds streak urgency scoring for "Consistency is Key" quest so beasts with higher streaks or expiring streaks (approaching 48h reset) are prioritized. Fixes isOwnerTargetedForPoison type signature (IgnoredPlayer -> TargetedPoisonPlayer). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…or hook Moves ~340 lines of autopilot logic (beast selection, attack batching, poison targeting, extra life management, all useEffects) from ActionBar into a dedicated hook. ActionBar is now UI-only. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@spaghettiOnToast is attempting to deploy a commit to the Provable Games Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the autopilot system by introducing targeted poison capabilities and a quest-driven prioritization mode for beast selection. It also refactors core autopilot logic into a dedicated React hook, enhancing modularity and maintainability. These changes aim to provide users with more strategic control over their autopilot's behavior and improve the overall efficiency of automated gameplay. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughExtracts autopilot orchestration from ActionBar into a new useAutopilotOrchestrator hook; adds targeted-poison (players & beasts), quest mode/filters, and new beast-selection/urgency utilities; updates store, UI, tests, and tests/hooks to use the orchestrator. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant ActionBar as ActionBar
participant Orchestrator as useAutopilotOrchestrator
participant Store as Autopilot Store
participant BeastUtils as Beast Utils
participant GameDir as Game Director
User->>ActionBar: Click startAutopilot
ActionBar->>Orchestrator: startAutopilot()
activate Orchestrator
Orchestrator->>Store: read config (quests, targets, budgets)
Store-->>Orchestrator: config
loop for each attack batch
Orchestrator->>BeastUtils: selectOptimalBeasts(collection, summit, config)
BeastUtils-->>Orchestrator: ranked beasts
Orchestrator->>GameDir: execute batch attacks (attackUntilCapture)
GameDir-->>Orchestrator: batch results
alt targeted poison required
Orchestrator->>GameDir: apply targeted poison
end
alt diplomacy/ignore check
Orchestrator->>GameDir: handle diplomacy / skip owners
end
Orchestrator->>Orchestrator: update log & potion counts
end
User->>ActionBar: Click stopAutopilot
ActionBar->>Orchestrator: stopAutopilot()
deactivate Orchestrator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59598243a1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
client/src/utils/beasts.ts
Outdated
|
|
||
| export function hasDiplomacyMatch(playerBeasts: Beast[], summitBeast: Beast): boolean { | ||
| return playerBeasts.some( | ||
| (beast) => beast.prefix === summitBeast.prefix && beast.suffix === summitBeast.suffix |
There was a problem hiding this comment.
hasDiplomacyMatch now treats any prefix/suffix name match as diplomacy, but the previous behavior (and the skipSharedDiplomacy setting semantics) required beast.diplomacy to be true. In all_out mode this helper is used between batches, so autopilot can halt against players that merely share names with one of your beasts even when none of your beasts has diplomacy unlocked.
Useful? React with 👍 / 👎.
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(beastAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); |
There was a problem hiding this comment.
Between-batch targeted poison is dispatched as fire-and-forget, and the loop immediately sends the next attack_until_capture transaction. That means the next batch is not actually guaranteed to run after poison is applied, and both writes can race each other (including nonce contention) when targeted poison is enabled during all_out attacks.
Useful? React with 👍 / 👎.
| poisonedTokenIdRef.current = summit.beast.token_id; | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [summit?.beast?.token_id]); |
There was a problem hiding this comment.
This effect only depends on summit?.beast?.token_id, so enabling autopilot, adding a targeted player/beast, or changing poison limits while the same summit holder remains active will not run targeted poison logic. In practice, targeted poison can stay idle indefinitely unless the summit token changes or an attack loop happens.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to the autopilot system, including a targeted poison feature, a quest-aware mode for beast selection, and a major refactoring of the autopilot logic into a dedicated useAutopilotOrchestrator hook. However, a security audit identified two logic-related vulnerabilities in the useAutopilotOrchestrator hook that could lead to unintended token spending or incorrect targeting due to stale state in asynchronous operations. Additionally, a minor issue was found in the new configuration UI.
| const handleAttackUntilCapture = async (extraLifePotions: number) => { | ||
| const { attackInProgress: alreadyAttacking, applyingPotions: alreadyApplying } = useGameStore.getState(); | ||
| if (!enableAttack || alreadyAttacking || alreadyApplying) return; | ||
|
|
||
| setBattleEvents([]); | ||
| setAttackInProgress(true); | ||
|
|
||
| const allBeasts: [Beast, number, number][] = collectionWithCombat.map((beast: Beast) => [beast, 1, beast.combat?.attackPotions || 0]); | ||
|
|
||
| const batches: [Beast, number, number][][] = []; | ||
| for (let i = 0; i < allBeasts.length; i += MAX_BEASTS_PER_ATTACK) { | ||
| batches.push(allBeasts.slice(i, i + MAX_BEASTS_PER_ATTACK)); | ||
| } | ||
|
|
||
| for (const batch of batches) { | ||
| // Between batches: check if summit changed to an ignored or diplomacy-matched player | ||
| const currentSummit = useGameStore.getState().summit; | ||
| if (currentSummit) { | ||
| const { ignoredPlayers: ig, skipSharedDiplomacy: skipDip, targetedPoisonPlayers: tpp } = useAutopilotStore.getState(); | ||
| const currentCollection = useGameStore.getState().collection; | ||
| const isMyBeast = currentCollection.some((b: Beast) => b.token_id === currentSummit.beast.token_id); | ||
|
|
||
| if (isMyBeast) { | ||
| setAutopilotLog('Summit captured — halting attack'); | ||
| break; | ||
| } | ||
| if (isOwnerIgnored(currentSummit.owner, ig)) { | ||
| setAutopilotLog('Halted: ignored player took summit'); | ||
| break; | ||
| } | ||
| if (skipDip && hasDiplomacyMatch(currentCollection, currentSummit.beast)) { | ||
| setAutopilotLog('Halted: shared diplomacy'); | ||
| break; | ||
| } | ||
|
|
||
| // Fire targeted poison between batches if applicable | ||
| const { poisonTotalMax: ptm, poisonPotionsUsed: ppu, targetedPoisonBeasts: tpb } = useAutopilotStore.getState(); | ||
| const isBeastTarget = tpb.length > 0 && isBeastTargetedForPoison(currentSummit.beast.token_id, tpb); | ||
| if (isBeastTarget) { | ||
| const beastAmount = getTargetedBeastPoisonAmount(currentSummit.beast.token_id, tpb); | ||
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(beastAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| } else if (tpp.length > 0 && isOwnerTargetedForPoison(currentSummit.owner, tpp)) { | ||
| const playerAmount = getTargetedPoisonAmount(currentSummit.owner, tpp); | ||
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(playerAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| } | ||
| } | ||
|
|
||
| const result = await executeGameAction({ | ||
| type: 'attack_until_capture', | ||
| beasts: batch, | ||
| extraLifePotions | ||
| }); | ||
|
|
||
| if (!result) { | ||
| setAttackInProgress(false); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| setAttackInProgress(false); | ||
| }; |
There was a problem hiding this comment.
The handleAttackUntilCapture function is an asynchronous function that executes multiple batches of attacks. It captures the summit and tokenBalances variables from the render cycle in which it was created. Because it awaits multiple transactions, the actual state in the game store or controller context can change between batches (e.g., another player takes the summit, or token balances are updated). However, the function continues to use the stale captured values for logic checks and when calling handleApplyPoison. This can result in applying poison to a beast that is no longer on the summit or using incorrect balance information for spending decisions.
Recommendation: Use useGameStore.getState().summit to access the most recent summit state within the async loop. Additionally, refactor handleApplyPoison to accept the target beast ID as an explicit argument instead of relying on the captured summit variable.
| <TextField | ||
| type="number" | ||
| size="small" | ||
| value={defaultAmount} | ||
| onChange={(e) => { | ||
| let v = Number.parseInt(e.target.value, 10); | ||
| if (Number.isNaN(v)) v = 1; | ||
| setDefaultAmount(Math.max(1, Math.min(v, poisonAvailable || 9999))); | ||
| }} | ||
| inputProps={{ min: 1, max: poisonAvailable || 9999, step: 1 }} | ||
| sx={{ ...styles.numberField, width: 80 }} | ||
| /> |
There was a problem hiding this comment.
The max value for the poison amount TextField is incorrectly calculated when poisonAvailable is 0. The expression poisonAvailable || 9999 evaluates to 9999, allowing the user to set an amount of poison they do not have. This could lead to confusion and unexpected behavior.
The input should be disabled when no poison is available, and the max value should be correctly bound to poisonAvailable.
This issue is also present in other amount fields within TargetedPoisonSection and TargetedPoisonBeastSection.
| <TextField | |
| type="number" | |
| size="small" | |
| value={defaultAmount} | |
| onChange={(e) => { | |
| let v = Number.parseInt(e.target.value, 10); | |
| if (Number.isNaN(v)) v = 1; | |
| setDefaultAmount(Math.max(1, Math.min(v, poisonAvailable || 9999))); | |
| }} | |
| inputProps={{ min: 1, max: poisonAvailable || 9999, step: 1 }} | |
| sx={{ ...styles.numberField, width: 80 }} | |
| /> | |
| <TextField | |
| type="number" | |
| size="small" | |
| value={defaultAmount} | |
| disabled={poisonAvailable === 0} | |
| onChange={(e) => { | |
| let v = Number.parseInt(e.target.value, 10); | |
| if (Number.isNaN(v)) v = 1; | |
| setDefaultAmount(Math.max(1, Math.min(v, poisonAvailable))); | |
| }} | |
| inputProps={{ min: 1, max: poisonAvailable, step: 1 }} | |
| sx={{ ...styles.numberField, width: 80 }} | |
| /> |
| // Fire targeted poison between batches if applicable | ||
| const { poisonTotalMax: ptm, poisonPotionsUsed: ppu, targetedPoisonBeasts: tpb } = useAutopilotStore.getState(); | ||
| const isBeastTarget = tpb.length > 0 && isBeastTargetedForPoison(currentSummit.beast.token_id, tpb); | ||
| if (isBeastTarget) { | ||
| const beastAmount = getTargetedBeastPoisonAmount(currentSummit.beast.token_id, tpb); | ||
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(beastAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| } else if (tpp.length > 0 && isOwnerTargetedForPoison(currentSummit.owner, tpp)) { | ||
| const playerAmount = getTargetedPoisonAmount(currentSummit.owner, tpp); | ||
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(playerAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| } |
There was a problem hiding this comment.
Within the handleAttackUntilCapture loop, the logic checks for targeted poison players or beasts between every batch of attacks. If a target is identified, handleApplyPoison is called. If an attack sequence requires multiple batches to capture the summit, and the target remains on the summit between batches, poison will be applied repeatedly (once per batch). This can lead to spending significantly more poison tokens than the user intended when they configured the "amount" for a specific target, as there is no mechanism to track if poison has already been applied to the current target during the current attack sequence.
Recommendation: Implement a tracking mechanism (e.g., a local variable or a Set of beast IDs) within handleAttackUntilCapture to ensure that targeted poison is only applied once per target per attack sequence.
…ilot - handleApplyPoison accepts explicit beastId to avoid stale summit closure - Track poisoned targets per attack sequence to prevent duplicate spending - Add config deps to targeted poison effect so it triggers on setting changes - Disable poison amount inputs when balance is 0, fix max fallback to 9999 - Require beast.diplomacy flag in hasDiplomacyMatch Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
client/src/hooks/useAutopilotOrchestrator.ts (1)
7-12: Use the repo alias forsrcimports.This new hook reaches
src/utils/beasts.tsvia a relative path instead of@/*, which makes future file moves more brittle than the rest of the client code. As per coding guidelines, use path alias@/*to referencesrc/*in imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 7 - 12, The import in useAutopilotOrchestrator.ts currently references src utilities via a relative path; change the import of calculateRevivalRequired, isOwnerIgnored, isOwnerTargetedForPoison, getTargetedPoisonAmount, isBeastTargetedForPoison, getTargetedBeastPoisonAmount, hasDiplomacyMatch, selectOptimalBeasts to use the repository alias (e.g., '@/utils/beasts') instead of '../utils/beasts' so the hook uses the src path alias and remains resilient to file moves.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/components/dialogs/AutopilotConfigModal.tsx`:
- Around line 164-170: The onChange handlers and inputProps in
AutopilotConfigModal are incorrectly clamping persistent targeting amounts to
the current poison balance (poisonAvailable); update the onChange logic (e.g.,
the handler that calls setDefaultAmount and the similar handlers at the other
ranges) to only clamp to sensible persistent limits (e.g., min 1 and a high max
like 9999) instead of using poisonAvailable, and update inputProps.max to the
same persistent max rather than poisonAvailable; leave runtime
capping/validation to the execution path that applies rules. Locate the onChange
callbacks and inputProps near setDefaultAmount and the analogous setXAmount
functions and remove references to poisonAvailable for clamping.
- Around line 114-133: The effect should track each lookup with a monotonically
incremented request ID stored in a ref to ignore stale responses: add a
requestIdRef (useRef<number>) and increment it before calling lookupUsernames
inside the debounce timeout, capture the current id in the async closure, and
before any setResolved/setError/setLoading calls verify the captured id ===
requestIdRef.current; also ensure the cleanup (return) does not accidentally
change requestIdRef so only completed newer requests can update state; update
the existing debounceRef/lookupUsernames logic to use this request-id guard
around all state updates.
In `@client/src/hooks/useAutopilotOrchestrator.ts`:
- Around line 133-176: The loop in useAutopilotOrchestrator never re-reads the
autopilot stop flag so clicking “Stop Autopilot” won’t halt remaining batches;
before dispatching each batch (inside the for (const batch of batches) loop,
just before calling executeGameAction), read the current stop flag from
useAutopilotStore (e.g. const autopilotEnabled =
useAutopilotStore.getState().autopilotEnabled) and if it is false call
setAutopilotLog('Autopilot stopped') and break out of the loop to avoid queueing
further executeGameAction('attack_until_capture', beasts: batch, ...). Ensure
you reference the existing symbols useAutopilotStore.getState() and
executeGameAction so the check is colocated with the batch dispatch.
- Around line 216-224: The memoized summitSharesDiplomacy currently requires
beast.diplomacy which diverges from the attack loop predicate; change
summitSharesDiplomacy to mirror hasDiplomacyMatch (and the between-batch halt)
by removing the beast.diplomacy truthiness check and only comparing prefix and
suffix against summit.beast (so it returns true when any collection beast has
matching prefix and suffix), ensuring shouldSkipSummit and the actual halt
condition agree.
- Around line 252-303: The effect attached in useAutopilotOrchestrator (the
useEffect that reacts to summit?.beast?.token_id) reads many external values but
only lists summit?.beast?.token_id as a dependency, so it can miss updates;
update the dependency array for that useEffect to include all referenced values
(autopilotEnabled, targetedPoisonBeasts, targetedPoisonPlayers, poisonStrategy,
poisonTotalMax, poisonPotionsUsed, tokenBalances, shouldSkipSummit,
poisonMinPower, poisonMinHealth, poisonAggressiveAmount, collection, and any
refs or handlers used such as poisonedTokenIdRef and handleApplyPoison) so the
effect reruns whenever any of those change and preserves current behavior (keep
the eslint-disable comment removed once deps are complete).
- Around line 92-117: Both handlers call async executeGameAction but immediately
treat the action as successful; change handleApplyExtraLife and
handleApplyPoison to await executeGameAction, wrap the call in try/catch, and
only return true or consider the action successful after the awaited call
resolves; ensure setApplyingPotions(true) stays before the await to disable UI
and that you reset setApplyingPotions(false) in a finally block, and update
setAutopilotLog on error inside the catch so failures are handled correctly (use
the existing function names handleApplyExtraLife, handleApplyPoison,
executeGameAction, setApplyingPotions, setAutopilotLog to locate the changes).
- Around line 69-88: The useMemo block that computes collectionWithCombat calls
selectOptimalBeasts but only depends on collection.length, which misses updates
to beast properties; update the dependency array for the useMemo (the useMemo
declaration that defines collectionWithCombat) to include the full collection
object (replace or add collection alongside collection.length) so the memo
recomputes when individual beast fields change, while keeping the other
dependencies like summit?.beast?.token_id, summit?.beast?.extra_lives,
summit?.beast?.current_health, revivePotionsUsed, attackPotionsUsed,
useRevivePotions, useAttackPotions, questMode, questFilters, maxBeastsPerAttack,
attackStrategy, and autopilotEnabled.
---
Nitpick comments:
In `@client/src/hooks/useAutopilotOrchestrator.ts`:
- Around line 7-12: The import in useAutopilotOrchestrator.ts currently
references src utilities via a relative path; change the import of
calculateRevivalRequired, isOwnerIgnored, isOwnerTargetedForPoison,
getTargetedPoisonAmount, isBeastTargetedForPoison, getTargetedBeastPoisonAmount,
hasDiplomacyMatch, selectOptimalBeasts to use the repository alias (e.g.,
'@/utils/beasts') instead of '../utils/beasts' so the hook uses the src path
alias and remains resilient to file moves.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 029b03c6-9b00-463d-84d9-a8abf783c121
📒 Files selected for processing (7)
client/src/components/ActionBar.test.tsxclient/src/components/ActionBar.tsxclient/src/components/dialogs/AutopilotConfigModal.tsxclient/src/hooks/useAutopilotOrchestrator.tsclient/src/stores/autopilotStore.tsclient/src/utils/beasts.test.tsclient/src/utils/beasts.ts
| React.useEffect(() => { | ||
| setResolved(null); | ||
| setError(null); | ||
| const username = input.trim(); | ||
| if (!username) { setLoading(false); return; } | ||
| setLoading(true); | ||
| if (debounceRef.current) clearTimeout(debounceRef.current); | ||
| debounceRef.current = setTimeout(async () => { | ||
| try { | ||
| const result = await lookupUsernames([username]); | ||
| const address = result.get(username); | ||
| if (input.trim() !== username) return; | ||
| if (address) { setResolved(address); setError(null); } | ||
| else { setResolved(null); setError('Player not found'); } | ||
| } catch { setResolved(null); setError('Lookup failed'); } | ||
| finally { setLoading(false); } | ||
| }, 400); | ||
| return () => { if (debounceRef.current) clearTimeout(debounceRef.current); }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [input]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Get file size first
wc -l client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 124
🏁 Script executed:
# Read the specific sections mentioned in the review
sed -n '100,140p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1851
🏁 Script executed:
# Also read the second section mentioned
sed -n '380,435p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1860
🏁 Script executed:
# Check around lines 164-170, 199-205, 266-272, 299-305 that were mentioned in the scratchpad
sed -n '160,175p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 768
🏁 Script executed:
# Get broader context to understand lookupUsernames and the overall structure
sed -n '1,50p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1583
🏁 Script executed:
# Search for all debounceRef declarations in the file
rg "debounceRef" client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 602
🏁 Script executed:
# Get the full context around both effect declarations to see the structure
sed -n '108,145p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1548
🏁 Script executed:
# Also check around the second debounceRef to see if it's a redeclaration
sed -n '375,410p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1253
🏁 Script executed:
# Verify the effect cleanup by checking both effects and their dependency arrays carefully
sed -n '113,145p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1212
🏁 Script executed:
# Also check the second effect's dependency array
sed -n '389,435p' client/src/components/dialogs/AutopilotConfigModal.tsxRepository: Provable-Games/summit
Length of output: 1596
Both username lookups need explicit stale-response guards using request IDs or refs.
The current stale checks (input.trim() !== username / ignoredInput.trim() !== username) compare the current input value at callback time, but in-flight requests aren't canceled when input changes. While the stale check technically prevents state updates from older requests, this approach is fragile and relies on closure semantics and timing. Use a request ID or useRef to track the latest request and explicitly ignore older responses before setting state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/dialogs/AutopilotConfigModal.tsx` around lines 114 -
133, The effect should track each lookup with a monotonically incremented
request ID stored in a ref to ignore stale responses: add a requestIdRef
(useRef<number>) and increment it before calling lookupUsernames inside the
debounce timeout, capture the current id in the async closure, and before any
setResolved/setError/setLoading calls verify the captured id ===
requestIdRef.current; also ensure the cleanup (return) does not accidentally
change requestIdRef so only completed newer requests can update state; update
the existing debounceRef/lookupUsernames logic to use this request-id guard
around all state updates.
| onChange={(e) => { | ||
| let v = Number.parseInt(e.target.value, 10); | ||
| if (Number.isNaN(v)) v = 1; | ||
| setDefaultAmount(Math.max(1, Math.min(v, poisonAvailable || 9999))); | ||
| }} | ||
| inputProps={{ min: 1, max: poisonAvailable || 9999, step: 1 }} | ||
| sx={{ ...styles.numberField, width: 80 }} |
There was a problem hiding this comment.
Don’t bind targeted-poison amounts to the current balance.
These are persistent targeting rules, but the inputs clamp them to poisonAvailable whenever the user already holds any poison. That means someone with 5 poison today cannot configure “apply 100 when available later”, even though runtime already caps by current balance.
Also applies to: 199-205, 266-272, 299-305
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/components/dialogs/AutopilotConfigModal.tsx` around lines 164 -
170, The onChange handlers and inputProps in AutopilotConfigModal are
incorrectly clamping persistent targeting amounts to the current poison balance
(poisonAvailable); update the onChange logic (e.g., the handler that calls
setDefaultAmount and the similar handlers at the other ranges) to only clamp to
sensible persistent limits (e.g., min 1 and a high max like 9999) instead of
using poisonAvailable, and update inputProps.max to the same persistent max
rather than poisonAvailable; leave runtime capping/validation to the execution
path that applies rules. Locate the onChange callbacks and inputProps near
setDefaultAmount and the analogous setXAmount functions and remove references to
poisonAvailable for clamping.
| const collectionWithCombat = useMemo<Beast[]>(() => { | ||
| if (summit && collection.length > 0) { | ||
| return selectOptimalBeasts(collection, summit, { | ||
| useRevivePotions, | ||
| revivePotionMax, | ||
| revivePotionMaxPerBeast, | ||
| revivePotionsUsed, | ||
| useAttackPotions, | ||
| attackPotionMax, | ||
| attackPotionMaxPerBeast, | ||
| attackPotionsUsed, | ||
| autopilotEnabled, | ||
| questMode, | ||
| questFilters, | ||
| }); | ||
| } | ||
|
|
||
| return []; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [summit?.beast?.token_id, summit?.beast?.extra_lives, summit?.beast?.current_health, collection.length, revivePotionsUsed, attackPotionsUsed, useRevivePotions, useAttackPotions, questMode, questFilters, maxBeastsPerAttack, attackStrategy, autopilotEnabled]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the useAutopilotOrchestrator.ts file
fd "useAutopilotOrchestrator" --type fRepository: Provable-Games/summit
Length of output: 1870
🏁 Script executed:
# Also find the selectOptimalBeasts function
rg "selectOptimalBeasts" --type ts --type tsx -B 2 -A 10Repository: Provable-Games/summit
Length of output: 92
🏁 Script executed:
# Look at the Beast type definition
rg "type Beast|interface Beast" --type ts --type tsxRepository: Provable-Games/summit
Length of output: 92
🏁 Script executed:
# Try to list repository files without cloning issues
git ls-files | head -20Repository: Provable-Games/summit
Length of output: 462
🏁 Script executed:
# Search for useAutopilotOrchestrator in repo
find . -name "useAutopilotOrchestrator*" 2>/dev/nullRepository: Provable-Games/summit
Length of output: 111
🏁 Script executed:
# Try to find selectOptimalBeasts
grep -r "selectOptimalBeasts" --include="*.ts" --include="*.tsx" 2>/dev/null | head -20Repository: Provable-Games/summit
Length of output: 1034
🏁 Script executed:
# Get selectOptimalBeasts implementation
cat -n client/src/utils/beasts.ts | head -150Repository: Provable-Games/summit
Length of output: 6658
🏁 Script executed:
# Get more context around the memo in useAutopilotOrchestrator
cat -n client/src/hooks/useAutopilotOrchestrator.ts | head -120Repository: Provable-Games/summit
Length of output: 5172
🏁 Script executed:
# Find Beast type definition
grep -r "type Beast" client/src --include="*.ts" --include="*.tsx" -A 5Repository: Provable-Games/summit
Length of output: 568
🏁 Script executed:
# Get Beast type definition
cat -n client/src/types/game.ts 2>/dev/null | grep -A 30 "type Beast\|interface Beast" | head -50Repository: Provable-Games/summit
Length of output: 963
🏁 Script executed:
# Get selectOptimalBeasts function signature and beginning of implementation
grep -n "export function selectOptimalBeasts" client/src/utils/beasts.ts -A 100 | head -120Repository: Provable-Games/summit
Length of output: 4788
🏁 Script executed:
# Check if collection items are mutated directly in gameStore
grep -n "collection" client/src/stores/gameStore.ts -B 2 -A 2 | head -60Repository: Provable-Games/summit
Length of output: 2041
🏁 Script executed:
# Check how collection is updated in gameStore and if items are mutated
grep -n "setCollection\|collection =" client/src/stores/gameStore.ts -B 2 -A 2 | head -80Repository: Provable-Games/summit
Length of output: 1332
🏁 Script executed:
# Look for where collection items are mutated or updated
rg "collection\[" client/src --type ts --type tsx -B 2 -A 2 | head -50Repository: Provable-Games/summit
Length of output: 92
🏁 Script executed:
# Check what properties selectOptimalBeasts actually reads from Beast objects
grep -n "beast\." client/src/utils/beasts.ts | grep -E "current_health|last_death|revival|extra_lives|attack_streak|captured_summit|max_attack" | head -30Repository: Provable-Games/summit
Length of output: 1981
🏁 Script executed:
# Check how collection gets updated - look for setCollection calls
rg "setCollection" client/src --type ts --type tsx -B 2 -A 2 | head -80Repository: Provable-Games/summit
Length of output: 92
🏁 Script executed:
# Look for places where beasts are updated individually (mutation risk)
rg "collection.*current_health\|collection.*last_death\|collection.*revival" client/src --type ts --type tsx -B 2 -A 2Repository: Provable-Games/summit
Length of output: 92
🏁 Script executed:
# Check if there are any sync hooks that update beast properties
grep -rn "useEffect\|useLayoutEffect" client/src/hooks/useAutopilotOrchestrator.ts -A 10 | head -50Repository: Provable-Games/summit
Length of output: 2145
🏁 Script executed:
# Search for where collection gets synced or updated from backend
rg "syncCollection\|loadCollection\|updateCollection" client/src --type ts -B 3 -A 3Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Look for WebSocket or external data updates to collection
rg "websocket\|useWebSocket\|addEventListener" client/src/hooks --type ts -A 5 | head -60Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Check the memo behavior - look at other useMemo patterns in the codebase to see if they include collection
rg "useMemo.*collection" client/src --type ts -B 2 -A 8 | head -60Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Look for where beast properties are updated (health, streak, etc)
rg "current_health\s*=" client/src --type ts -B 2 -A 2 | head -40Repository: Provable-Games/summit
Length of output: 2598
🏁 Script executed:
# Check BeastCollection.tsx (from learning) to understand how collection hydration works
find client/src -name "BeastCollection*" -type fRepository: Provable-Games/summit
Length of output: 106
🏁 Script executed:
# Look at how collection state changes are handled
grep -rn "setCollection\|updateCollection" client/src/contexts --include="*.ts" -B 2 -A 5 | head -60Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Search for API/sync calls that might update individual beasts
rg "PUT\|PATCH\|POST.*beast" client/src --type ts -B 2 -A 2 | head -40Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Check BeastCollection component to understand data flow (from learning)
cat -n client/src/components/BeastCollection.tsx | head -80Repository: Provable-Games/summit
Length of output: 4661
🏁 Script executed:
# Look for where beasts are fetched/updated
rg "fetch.*beast\|queryBeast" client/src --type ts -B 2 -A 3 | head -40Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Check if there's real-time sync that updates collection properties
grep -rn "useCallback\|useEffect" client/src/contexts --include="*.ts" | grep -i "collection\|beast" | head -20Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Check if collection is ever updated with the same length (e.g., refreshing beast properties)
grep -n "setCollection" client/src --include="*.ts" -B 5 -A 5 -r | head -100Repository: Provable-Games/summit
Length of output: 3432
🏁 Script executed:
# Look at GameDirector to see how battle results update collection
cat -n client/src/contexts/GameDirector.ts 2>/dev/null | grep -A 10 "collection\|beast.*health" | head -60Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Check if there are any real-time updates or polling that might update beasts
rg "interval\|timeout\|setInterval" client/src/hooks --type ts -B 2 -A 5 | head -40Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Check the actual dependency list more carefully - look for any collection-related dependencies
grep -n "collection\|Beast\[\]" client/src/hooks/useAutopilotOrchestrator.ts | head -30Repository: Provable-Games/summit
Length of output: 1503
🏁 Script executed:
# Search for where collection properties might be updated (health, streak, etc)
rg "\.current_health\s*=\|\.attack_streak\s*=\|\.revival_count\s*=" client/src --type ts -B 3 -A 1 | head -60Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Look at quest-related functions to see if they also read mutable beast state
rg "questNeedsPredicate\|questUrgencyScore" client/src/utils/beasts.ts -B 2 -A 15 | head -80Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Verify the exact dependency array one more time with context
sed -n '69,89p' client/src/hooks/useAutopilotOrchestrator.tsRepository: Provable-Games/summit
Length of output: 864
🏁 Script executed:
# Check if collection reference itself (not just length) is ever used as a dependency elsewhere
rg "useMemo.*\[.*collection[^.]" client/src/hooks --type ts -B 2 -A 2 | head -40Repository: Provable-Games/summit
Length of output: 47
🏁 Script executed:
# Look at how bestial properties change over time (e.g., health after battle)
rg "battle\|attack.*execute\|setCollection" client/src/contexts/GameDirector.ts -B 2 -A 5 | head -80Repository: Provable-Games/summit
Length of output: 140
Include collection in the memo dependency array to ensure recomputation when beast properties change.
selectOptimalBeasts() reads mutable state like current health, revival counters, and attack streaks from individual beast objects. Since the dependency array only tracks collection.length, a collection update that modifies beast properties while preserving length will leave combat scoring and quest ordering stale until an unrelated dependency changes.
Suggested fix
Add collection to the dependency array (replacing or alongside collection.length):
}, [collection, summit?.beast?.token_id, summit?.beast?.extra_lives, summit?.beast?.current_health, revivePotionsUsed, attackPotionsUsed, useRevivePotions, useAttackPotions, questMode, questFilters, maxBeastsPerAttack, attackStrategy, autopilotEnabled]);
Note: This may increase recomputation frequency if the collection reference changes often; consider memoizing the collection reference or using a secondary effect if performance becomes an issue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 69 - 88, The
useMemo block that computes collectionWithCombat calls selectOptimalBeasts but
only depends on collection.length, which misses updates to beast properties;
update the dependency array for the useMemo (the useMemo declaration that
defines collectionWithCombat) to include the full collection object (replace or
add collection alongside collection.length) so the memo recomputes when
individual beast fields change, while keeping the other dependencies like
summit?.beast?.token_id, summit?.beast?.extra_lives,
summit?.beast?.current_health, revivePotionsUsed, attackPotionsUsed,
useRevivePotions, useAttackPotions, questMode, questFilters, maxBeastsPerAttack,
attackStrategy, and autopilotEnabled.
| const handleApplyExtraLife = (amount: number) => { | ||
| if (!summit?.beast || !isSavage || applyingPotions || amount === 0) return; | ||
|
|
||
| setApplyingPotions(true); | ||
| setAutopilotLog('Adding extra lives...'); | ||
|
|
||
| executeGameAction({ | ||
| type: 'add_extra_life', | ||
| beastId: summit.beast.token_id, | ||
| extraLifePotions: amount, | ||
| }); | ||
| }; | ||
|
|
||
| const handleApplyPoison = (amount: number): boolean => { | ||
| if (!summit?.beast || applyingPotions || amount === 0) return false; | ||
|
|
||
| setApplyingPotions(true); | ||
| setAutopilotLog('Applying poison...'); | ||
|
|
||
| executeGameAction({ | ||
| type: 'apply_poison', | ||
| beastId: summit.beast.token_id, | ||
| count: amount, | ||
| }); | ||
| return true; | ||
| }; |
There was a problem hiding this comment.
Await potion writes before treating them as successful.
executeGameAction() is async, but these handlers update UI state immediately and handleApplyPoison() returns true before the write settles. Callers then mark the summit as already poisoned or continue batching even if the action rejects or returns false.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 92 - 117, Both
handlers call async executeGameAction but immediately treat the action as
successful; change handleApplyExtraLife and handleApplyPoison to await
executeGameAction, wrap the call in try/catch, and only return true or consider
the action successful after the awaited call resolves; ensure
setApplyingPotions(true) stays before the await to disable UI and that you reset
setApplyingPotions(false) in a finally block, and update setAutopilotLog on
error inside the catch so failures are handled correctly (use the existing
function names handleApplyExtraLife, handleApplyPoison, executeGameAction,
setApplyingPotions, setAutopilotLog to locate the changes).
| for (const batch of batches) { | ||
| // Between batches: check if summit changed to an ignored or diplomacy-matched player | ||
| const currentSummit = useGameStore.getState().summit; | ||
| if (currentSummit) { | ||
| const { ignoredPlayers: ig, skipSharedDiplomacy: skipDip, targetedPoisonPlayers: tpp } = useAutopilotStore.getState(); | ||
| const currentCollection = useGameStore.getState().collection; | ||
| const isMyBeast = currentCollection.some((b: Beast) => b.token_id === currentSummit.beast.token_id); | ||
|
|
||
| if (isMyBeast) { | ||
| setAutopilotLog('Summit captured — halting attack'); | ||
| break; | ||
| } | ||
| if (isOwnerIgnored(currentSummit.owner, ig)) { | ||
| setAutopilotLog('Halted: ignored player took summit'); | ||
| break; | ||
| } | ||
| if (skipDip && hasDiplomacyMatch(currentCollection, currentSummit.beast)) { | ||
| setAutopilotLog('Halted: shared diplomacy'); | ||
| break; | ||
| } | ||
|
|
||
| // Fire targeted poison between batches if applicable | ||
| const { poisonTotalMax: ptm, poisonPotionsUsed: ppu, targetedPoisonBeasts: tpb } = useAutopilotStore.getState(); | ||
| const isBeastTarget = tpb.length > 0 && isBeastTargetedForPoison(currentSummit.beast.token_id, tpb); | ||
| if (isBeastTarget) { | ||
| const beastAmount = getTargetedBeastPoisonAmount(currentSummit.beast.token_id, tpb); | ||
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(beastAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| } else if (tpp.length > 0 && isOwnerTargetedForPoison(currentSummit.owner, tpp)) { | ||
| const playerAmount = getTargetedPoisonAmount(currentSummit.owner, tpp); | ||
| const remainingCap = Math.max(0, ptm - ppu); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(playerAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| } | ||
| } | ||
|
|
||
| const result = await executeGameAction({ | ||
| type: 'attack_until_capture', | ||
| beasts: batch, | ||
| extraLifePotions | ||
| }); |
There was a problem hiding this comment.
Honor stopAutopilot() between attack batches.
Once this loop starts, it never re-checks autopilotEnabled, so clicking “Stop Autopilot” does not prevent the remaining batches from being queued. The stop flag needs to be read here before dispatching the next batch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 133 - 176, The
loop in useAutopilotOrchestrator never re-reads the autopilot stop flag so
clicking “Stop Autopilot” won’t halt remaining batches; before dispatching each
batch (inside the for (const batch of batches) loop, just before calling
executeGameAction), read the current stop flag from useAutopilotStore (e.g.
const autopilotEnabled = useAutopilotStore.getState().autopilotEnabled) and if
it is false call setAutopilotLog('Autopilot stopped') and break out of the loop
to avoid queueing further executeGameAction('attack_until_capture', beasts:
batch, ...). Ensure you reference the existing symbols
useAutopilotStore.getState() and executeGameAction so the check is colocated
with the batch dispatch.
| const summitSharesDiplomacy = useMemo(() => { | ||
| if (!skipSharedDiplomacy || !summit?.beast) return false; | ||
| return collection.some( | ||
| (beast: Beast) => | ||
| beast.diplomacy && | ||
| beast.prefix === summit.beast.prefix && | ||
| beast.suffix === summit.beast.suffix, | ||
| ); | ||
| }, [skipSharedDiplomacy, summit?.beast?.token_id, collection.length]); |
There was a problem hiding this comment.
Use the same diplomacy predicate as the attack loop.
This memo requires beast.diplomacy, but hasDiplomacyMatch() and the between-batch halt only compare prefix/suffix. That makes shouldSkipSummit disagree with the actual halt condition and can still spend attacks or poison on a summit the UI says should be skipped.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 216 - 224, The
memoized summitSharesDiplomacy currently requires beast.diplomacy which diverges
from the attack loop predicate; change summitSharesDiplomacy to mirror
hasDiplomacyMatch (and the between-batch halt) by removing the beast.diplomacy
truthiness check and only comparing prefix and suffix against summit.beast (so
it returns true when any collection beast has matching prefix and suffix),
ensuring shouldSkipSummit and the actual halt condition agree.
| // Targeted + aggressive poison on summit change | ||
| useEffect(() => { | ||
| if (!autopilotEnabled || !summit?.beast) return; | ||
|
|
||
| const { attackInProgress: attacking, applyingPotions: applying } = useGameStore.getState(); | ||
| if (attacking || applying) return; | ||
|
|
||
| const myBeast = collection.find((beast: Beast) => beast.token_id === summit.beast.token_id); | ||
| if (myBeast) return; | ||
|
|
||
| // Beast-level targeted poison (highest priority) | ||
| const isBeastTarget = targetedPoisonBeasts.length > 0 && isBeastTargetedForPoison(summit.beast.token_id, targetedPoisonBeasts); | ||
| if (isBeastTarget) { | ||
| const beastAmount = getTargetedBeastPoisonAmount(summit.beast.token_id, targetedPoisonBeasts); | ||
| const remainingCap = Math.max(0, poisonTotalMax - poisonPotionsUsed); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(beastAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| return; | ||
| } | ||
|
|
||
| // Player-level targeted poison | ||
| const isTargeted = targetedPoisonPlayers.length > 0 && isOwnerTargetedForPoison(summit.owner, targetedPoisonPlayers); | ||
| if (isTargeted) { | ||
| const playerAmount = getTargetedPoisonAmount(summit.owner, targetedPoisonPlayers); | ||
| const remainingCap = Math.max(0, poisonTotalMax - poisonPotionsUsed); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(playerAmount, pb, remainingCap); | ||
| if (amount > 0) handleApplyPoison(amount); | ||
| return; | ||
| } | ||
|
|
||
| if (poisonStrategy !== 'aggressive') return; | ||
| if (shouldSkipSummit) return; | ||
|
|
||
| // Reset tracked token when summit beast changes | ||
| if (poisonedTokenIdRef.current !== summit.beast.token_id) { | ||
| poisonedTokenIdRef.current = null; | ||
| } | ||
| if (poisonedTokenIdRef.current === summit.beast.token_id) return; | ||
|
|
||
| if (poisonMinPower > 0 && summit.beast.power < poisonMinPower) return; | ||
| if (poisonMinHealth > 0 && summit.beast.current_health < poisonMinHealth) return; | ||
|
|
||
| const remainingCap = Math.max(0, poisonTotalMax - poisonPotionsUsed); | ||
| const pb = tokenBalances?.["POISON"] || 0; | ||
| const amount = Math.min(poisonAggressiveAmount, pb, remainingCap); | ||
| if (amount > 0 && handleApplyPoison(amount)) { | ||
| poisonedTokenIdRef.current = summit.beast.token_id; | ||
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [summit?.beast?.token_id]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "useAutopilotOrchestrator.ts" --type fRepository: Provable-Games/summit
Length of output: 1876
Missing dependencies in poison effect listener.
The useEffect at lines 252-303 only depends on summit?.beast?.token_id, but reads autopilotEnabled, targetedPoisonBeasts, targetedPoisonPlayers, poisonStrategy, poisonTotalMax, poisonPotionsUsed, tokenBalances, shouldSkipSummit, poisonMinPower, poisonMinHealth, poisonAggressiveAmount, and collection. The effect won't rerun when these variables change, causing missed poison opportunities (e.g., when a targeted player already holds the summit or autopilot is enabled with targets).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 252 - 303, The
effect attached in useAutopilotOrchestrator (the useEffect that reacts to
summit?.beast?.token_id) reads many external values but only lists
summit?.beast?.token_id as a dependency, so it can miss updates; update the
dependency array for that useEffect to include all referenced values
(autopilotEnabled, targetedPoisonBeasts, targetedPoisonPlayers, poisonStrategy,
poisonTotalMax, poisonPotionsUsed, tokenBalances, shouldSkipSummit,
poisonMinPower, poisonMinHealth, poisonAggressiveAmount, collection, and any
refs or handlers used such as poisonedTokenIdRef and handleApplyPoison) so the
effect reruns whenever any of those change and preserves current behavior (keep
the eslint-disable comment removed once deps are complete).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
client/src/components/dialogs/AutopilotConfigModal.tsx (1)
230-237: Consider preventing duplicate beast entries.The
handleAddfunction doesn't check if thetokenIdalready exists inbeastsbefore callingonAdd. Adding the same beast twice could cause UI confusion or duplicate poison applications.♻️ Proposed fix
const handleAdd = () => { const tokenId = Number.parseInt(tokenIdInput.trim(), 10); if (!Number.isFinite(tokenId) || tokenId <= 0) return; + if (beasts.some((b) => b.tokenId === tokenId)) return; // already added const name = nameInput.trim() || `Beast #${tokenId}`; onAdd({ tokenId, name, amount: defaultAmount }); setTokenIdInput(''); setNameInput(''); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/components/dialogs/AutopilotConfigModal.tsx` around lines 230 - 237, In handleAdd, prevent adding a duplicate beast by checking the existing beasts array for the parsed tokenId before calling onAdd; use the tokenId computed from tokenIdInput and if beasts.some(b => b.tokenId === tokenId) return early (or call a provided setError/ui feedback) so duplicate entries are ignored, otherwise proceed to construct name and call onAdd, then clear tokenIdInput and nameInput as before.client/src/utils/beasts.ts (2)
709-727: Therevival_potionquest check may incorrectly skip adding candidates.Line 712 checks if
result.some((b) => needsQuest(b) && b.current_health === 0), but after the beast is revived,current_healthbecomes positive. This check only works if the dead beast was already in the selection, not if it was revived duringselectOptimalBeasts.♻️ Proposed fix - track revived beasts explicitly
Consider tracking which beasts were added via revive and checking
used_revival_potioninstead of relying oncurrent_health === 0:if (quest === 'revival_potion') { // Special: include a dead beast that hasn't used revival potion if (config.useRevivePotions) { - const alreadyIncluded = result.some((b) => needsQuest(b) && b.current_health === 0); + const alreadyIncluded = result.some((b) => needsQuest(b)); if (!alreadyIncluded) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/utils/beasts.ts` around lines 709 - 727, The revival check in selectOptimalBeasts uses result.some((b) => needsQuest(b) && b.current_health === 0) which fails once a dead beast is revived (current_health > 0); update the logic to track revived candidates explicitly (e.g., mark a flag like used_revival_potion or maintain a revivedIds set) when you add a revival candidate so the presence check uses that marker instead of current_health; adjust the selection block that finds questCandidate (deadPool.find(...)) and the place that pushes beastCopy (and selectedIds) to also mark the beast as revived so subsequent checks correctly detect it; use existing symbols needsQuest, deadPool, selectedIds, calculateBattleResult, result and config to implement this change.
620-637: Mutation of candidate during selection could cause issues.Lines 623-627 mutate
candidate.attackPotionsandcandidate.damagewhen attack budget is exceeded. If the same candidate is considered again (e.g., if selection logic changes), the mutated values persist.♻️ Proposed fix - use local variables instead of mutating
for (const candidate of candidates) { if (selected.length >= maxBeasts) break; if (candidate.reviveCost > reviveBudget) continue; + + let effectivePotions = candidate.attackPotions; + let effectiveDamage = candidate.damage; if (candidate.attackPotions > attackBudget - usedAttackBudget) { // Try without attack potions - candidate.attackPotions = 0; - candidate.damage = candidate.beast.combat?.estimatedDamage ?? 0; + effectivePotions = 0; + effectiveDamage = candidate.beast.combat?.estimatedDamage ?? 0; } reviveBudget -= candidate.reviveCost; - usedAttackBudget += candidate.attackPotions; + usedAttackBudget += effectivePotions; const beastCopy = { ...candidate.beast }; - if (candidate.attackPotions > 0) { - beastCopy.combat = calculateBattleResult(beastCopy, summit, candidate.attackPotions); + if (effectivePotions > 0) { + beastCopy.combat = calculateBattleResult(beastCopy, summit, effectivePotions); } selected.push(beastCopy); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/src/utils/beasts.ts` around lines 620 - 637, The loop over candidates mutates candidate.attackPotions and candidate.damage which can leak into future iterations; instead, keep local variables (e.g., attackPotionsToUse and damageToUse) derived from candidate.attackPotions and candidate.damage when enforcing the attackBudget cap, do not assign back to candidate, decrement reviveBudget by candidate.reviveCost and increment usedAttackBudget by attackPotionsToUse, and when creating beastCopy use attackPotionsToUse to decide whether to call calculateBattleResult(beastCopy, summit, attackPotionsToUse) so candidate objects remain immutable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/src/hooks/useAutopilotOrchestrator.ts`:
- Around line 369-377: The guaranteed attack path calls executeGameAction(...)
without awaiting it, which can cause race conditions if the effect re-runs
before completion; update the effect callback that contains the guaranteed
strategy to await executeGameAction (e.g., make the effect callback async or use
an async IIFE) so the call is awaited just like handleAttackUntilCapture() does,
ensuring executeGameAction completes before the effect can re-trigger.
---
Nitpick comments:
In `@client/src/components/dialogs/AutopilotConfigModal.tsx`:
- Around line 230-237: In handleAdd, prevent adding a duplicate beast by
checking the existing beasts array for the parsed tokenId before calling onAdd;
use the tokenId computed from tokenIdInput and if beasts.some(b => b.tokenId ===
tokenId) return early (or call a provided setError/ui feedback) so duplicate
entries are ignored, otherwise proceed to construct name and call onAdd, then
clear tokenIdInput and nameInput as before.
In `@client/src/utils/beasts.ts`:
- Around line 709-727: The revival check in selectOptimalBeasts uses
result.some((b) => needsQuest(b) && b.current_health === 0) which fails once a
dead beast is revived (current_health > 0); update the logic to track revived
candidates explicitly (e.g., mark a flag like used_revival_potion or maintain a
revivedIds set) when you add a revival candidate so the presence check uses that
marker instead of current_health; adjust the selection block that finds
questCandidate (deadPool.find(...)) and the place that pushes beastCopy (and
selectedIds) to also mark the beast as revived so subsequent checks correctly
detect it; use existing symbols needsQuest, deadPool, selectedIds,
calculateBattleResult, result and config to implement this change.
- Around line 620-637: The loop over candidates mutates candidate.attackPotions
and candidate.damage which can leak into future iterations; instead, keep local
variables (e.g., attackPotionsToUse and damageToUse) derived from
candidate.attackPotions and candidate.damage when enforcing the attackBudget
cap, do not assign back to candidate, decrement reviveBudget by
candidate.reviveCost and increment usedAttackBudget by attackPotionsToUse, and
when creating beastCopy use attackPotionsToUse to decide whether to call
calculateBattleResult(beastCopy, summit, attackPotionsToUse) so candidate
objects remain immutable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f713b2d-7f1f-4c67-ae91-ad6e91606c18
📒 Files selected for processing (3)
client/src/components/dialogs/AutopilotConfigModal.tsxclient/src/hooks/useAutopilotOrchestrator.tsclient/src/utils/beasts.ts
| executeGameAction({ | ||
| type: 'attack', | ||
| beasts: beasts.map((beast: Beast) => ([beast, 1, beast.combat?.attackPotions || 0])), | ||
| safeAttack: false, | ||
| vrf: true, | ||
| extraLifePotions: extraLifePotions, | ||
| attackPotions: beasts[0]?.combat?.attackPotions || 0 | ||
| }); | ||
| } |
There was a problem hiding this comment.
Await the executeGameAction call in the guaranteed attack path.
The 'guaranteed' strategy calls executeGameAction() without await, unlike handleAttackUntilCapture() which properly awaits each batch. This could cause race conditions if the effect re-runs before the action completes.
🐛 Proposed fix
} else if (attackStrategy === 'guaranteed') {
const beasts = collectionWithCombat.slice(0, maxBeastsPerAttack);
const totalSummitHealth = ((summit.beast.health + summit.beast.bonus_health) * summit.beast.extra_lives) + summit.beast.current_health;
const totalEstimatedDamage = beasts.reduce((acc, beast) => acc + (beast.combat?.estimatedDamage ?? 0), 0);
if (totalEstimatedDamage < (totalSummitHealth * 1.1)) {
return;
}
- executeGameAction({
+ await executeGameAction({
type: 'attack',
beasts: beasts.map((beast: Beast) => ([beast, 1, beast.combat?.attackPotions || 0])),
safeAttack: false,
vrf: true,
extraLifePotions: extraLifePotions,
attackPotions: beasts[0]?.combat?.attackPotions || 0
});
}Note: This requires making the effect callback async or wrapping in an IIFE.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/src/hooks/useAutopilotOrchestrator.ts` around lines 369 - 377, The
guaranteed attack path calls executeGameAction(...) without awaiting it, which
can cause race conditions if the effect re-runs before completion; update the
effect callback that contains the guaranteed strategy to await executeGameAction
(e.g., make the effect callback async or use an async IIFE) so the call is
awaited just like handleAttackUntilCapture() does, ensuring executeGameAction
completes before the effect can re-trigger.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
2 participants
Summary
targeted player/beast holds it.
What changed
Autopilot enhancements: targeted poison, quest mode, hook extraction
useAutopilotOrchestratorhook for cleaner separationScope
Client-only, 7 files:
Validation
Commands run
# cd client && pnpm lint && pnpm buildIf
client/**changedcd client && pnpm lintcd client && pnpm buildRisk and Rollout
Risk level
Rollout / rollback plan
Breaking Changes
Merged to upstream main prior to PR
Summary by CodeRabbit
New Features
Refactor
Tests