Feature/player piloted summons#1375
Open
ifroshty wants to merge 20 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
the
size:L
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds controlled-actor routing for piloted summons, updates summon lifecycle and turn control handling, wires the combat UI to the active actor, and adds tests covering piloting, masking, AI behavior, and summon spawning/orphan cleanup. ChangesPiloted Summon Combat System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/src/layout/Combat.tsx`:
- Around line 154-166: The precomputedActions variable computed via useMemo is
render-scoped but gets consumed by long-lived callbacks (interval handlers and
renderer click/highlight paths at lines 501-503 and 734-747) that run outside
the React render cycle, causing them to access stale action data after battle
updates. Follow the same pattern already established with userIdRef: create a
new useRef to store precomputedActions, add a useEffect hook that updates this
ref whenever the precomputedActions value changes (with dependencies on
battleRef.current?.version and controlledActorId), and then update all the
long-lived callback code at those referenced line ranges to read from the ref
instead of the closure variable. This ensures the interval and click handlers
always access the current precomputedActions data rather than stale cached
values.
🪄 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: 584f4ea3-f9ac-4caf-beea-9b91cd1670ad
📒 Files selected for processing (19)
app/src/app/combat/page.tsxapp/src/layout/Combat.tsxapp/src/libs/combat/actions.tsapp/src/libs/combat/ai_v2.tsapp/src/libs/combat/constants.tsapp/src/libs/combat/tags.tsapp/src/libs/combat/types.tsapp/src/libs/combat/util.tsapp/src/libs/threejs/combat.tsapp/src/server/api/routers/combat.tsapp/tests/libs/combat/actions.test.tsapp/tests/libs/combat/ai_v2_piloted_guard.test.tsapp/tests/libs/combat/orphan_summon.test.tsapp/tests/libs/combat/piloted_actionset.test.tsapp/tests/libs/combat/piloted_masking.test.tsapp/tests/libs/combat/piloted_summon.test.tsapp/tests/libs/combat/resolve_controlled_actor.test.tsapp/tests/libs/combat/summon_template_lookup.test.tsapp/tests/libs/combat/turn_control.test.ts
Contributor
Confidence Score: 4/5Safe to merge for interactive battle types; all previous review-thread concerns are addressed and the piloted-summon turn routing is correctly gated on both server and client. The implementation is thorough and the 140-test suite covers every predicate, splice edge case, masking, and turn-control scenario. The previous reviewer concern about the AI interval auto-driving another player's piloted summon is fully fixed: getTurnControl gates on actor.isAi && !actor.isPiloted, which is false for any piloted summon regardless of which client is watching. The complexity of coordinating React refs, render-cycle effects, and 1-second intervals still warrants a close look at Combat.tsx — the brief window between a battle-state update and the interval's re-registration only causes a spurious changedActor refetch rather than incorrect action submission. app/src/layout/Combat.tsx — the interaction between the reactive controlledActorId, precomputedActionsRef, and the 1-second AI-interval closure is subtle and worth a manual smoke-test of the turn handoff between a player and their piloted summon in a live multiplayer battle. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant C as Client (Player)
participant S as Server (performAction)
participant AI as AI Loop
Note over C,AI: Player's own turn
C->>S: "performAction(userId=suid, actionId)"
S->>S: "getTurnControl(actor, suid) isUserTurn=true isAITurn=false"
S-->>C: battleUpdate (summon now active)
Note over C,AI: Piloted summon's turn
C->>C: resolveControlledActorId() → summon.userId
C->>C: availableUserActions(summon.userId) → summon jutsu
C->>S: "performAction(userId=summon.userId, jutsu)"
S->>S: "getTurnControl(pilotedSummon, suid) isUserTurn=true isAITurn=false"
S->>S: "availableUserActions(actor.userId=summon)"
S-->>C: battleUpdate
Note over AI: AI loop on any client — piloted summon turn
AI->>AI: "getTurnControl(actor, suid) isAITurn = isAi && !isPiloted = false"
AI->>AI: No auto-drive fired
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant C as Client (Player)
participant S as Server (performAction)
participant AI as AI Loop
Note over C,AI: Player's own turn
C->>S: "performAction(userId=suid, actionId)"
S->>S: "getTurnControl(actor, suid) isUserTurn=true isAITurn=false"
S-->>C: battleUpdate (summon now active)
Note over C,AI: Piloted summon's turn
C->>C: resolveControlledActorId() → summon.userId
C->>C: availableUserActions(summon.userId) → summon jutsu
C->>S: "performAction(userId=summon.userId, jutsu)"
S->>S: "getTurnControl(pilotedSummon, suid) isUserTurn=true isAITurn=false"
S->>S: "availableUserActions(actor.userId=summon)"
S-->>C: battleUpdate
Note over AI: AI loop on any client — piloted summon turn
AI->>AI: "getTurnControl(actor, suid) isAITurn = isAi && !isPiloted = false"
AI->>AI: No auto-drive fired
Reviews (11): Last reviewed commit: "fix: typescript ci" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app/tests/libs/combat/summon_turn_order.test.ts (1)
22-27: ⚡ Quick winReplace process-tracking prose with purpose-focused test commentary
The comment references task/review history instead of the test’s behavior contract. Please rewrite it to only describe why this fixture proves
calcActiveUserexcludes non-live templates.Suggested edit
- // curHealth:0 is what Task 1 Step 8 (the AutoBattle !user.isSummon guard) - // GUARANTEES at load for templates in every battle type. This test locks the - // CONSUMER behavior — that calcActiveUser excludes a curHealth:0 template — - // so it exercises the stillInBattle exclusion path, not the loader itself - // (which needs a DB-backed processUsersForBattle; covered by dev-boot smoke). + // This template is intentionally non-live (curHealth: 0), so calcActiveUser + // must exclude it through stillInBattle-based actor selection.As per coding guidelines, comments should describe code purpose and should not include review/task tracking markers.
🤖 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 `@app/tests/libs/combat/summon_turn_order.test.ts` around lines 22 - 27, The multi-line comment block preceding the template creation references process-tracking details like task history and implementation concerns rather than the test's purpose. Replace this comment with a clear explanation focused solely on describing why this fixture with curHealth:0 proves that calcActiveUser excludes non-live templates from active user calculations. Remove all references to Task identifiers, loader implementation details, and smoke test coverage, keeping only the behavioral contract this test verifies.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@app/tests/libs/combat/summon_turn_order.test.ts`:
- Around line 22-27: The multi-line comment block preceding the template
creation references process-tracking details like task history and
implementation concerns rather than the test's purpose. Replace this comment
with a clear explanation focused solely on describing why this fixture with
curHealth:0 proves that calcActiveUser excludes non-live templates from active
user calculations. Remove all references to Task identifiers, loader
implementation details, and smoke test coverage, keeping only the behavioral
contract this test verifies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cb13694b-1ef2-404d-826f-d1a63ab2e673
📒 Files selected for processing (11)
app/src/libs/combat/ai_v2.tsapp/src/libs/combat/summon.tsapp/src/libs/combat/tags.tsapp/src/libs/combat/types.tsapp/src/libs/combat/util.tsapp/src/server/api/routers/combat.tsapp/tests/libs/combat/orphan_summon.test.tsapp/tests/libs/combat/piloted_summon.test.tsapp/tests/libs/combat/summon.test.tsapp/tests/libs/combat/summon_template_lookup.test.tsapp/tests/libs/combat/summon_turn_order.test.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/tests/libs/combat/piloted_summon.test.ts
- app/tests/libs/combat/orphan_summon.test.ts
- app/src/libs/combat/types.ts
- app/src/server/api/routers/combat.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@app/src/validators/combat.ts`:
- Around line 740-742: The playerControlled validator currently uses
z.coerce.boolean(), which relies on JavaScript truthiness and can turn the
string "false" into true. Update the playerControlled field in the combat
validator to use a proper string-to-boolean parser such as z.stringbool() or an
explicit boolean parsing schema, while keeping the same default behavior and
surrounding validation structure intact.
In `@app/tests/libs/combat/summon_template_lookup.test.ts`:
- Around line 308-310: The summon tests are too loose because `toBeFalsy()`
allows `undefined` and won’t catch regressions in `summon()` initialization.
Update the non-piloted assertions in `summon_template_lookup.test.ts` to use
exact boolean checks on `findSpawn(...)?.isPiloted`, especially in the
`summon()` paths that rely on `shouldPilotSummon(...)`, so the tests fail if
`isPiloted` is no longer explicitly set to `false`.
- Around line 365-369: The clone assertion in summon_template_lookup.test should
verify that clone() removes the isPiloted flag entirely, not just that it is
falsy. Update the expectation on the cloned summon found via usersState.find to
use toBeUndefined() for cloned?.isPiloted so this test catches regressions where
the property remains set to false instead of being cleared.
🪄 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: 06822444-690c-4e80-845e-83cc0074c021
📒 Files selected for processing (5)
app/src/libs/combat/summon.tsapp/src/libs/combat/tags.tsapp/src/validators/combat.tsapp/tests/libs/combat/piloted_summon.test.tsapp/tests/libs/combat/summon_template_lookup.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- app/tests/libs/combat/piloted_summon.test.ts
- app/src/libs/combat/summon.ts
- app/src/libs/combat/tags.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/layout/Combat.tsx (1)
429-466: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAvoid stale
suid/timeDiffin the once-registered key handler.The listener is registered with
[], but still closes over render-scopesuidandtimeDiff. If user data or clock sync arrives after mount, thewshortcut can compare against a stale session id or compute the active actor with stale timing.Proposed fix
const suid = userData?.userId; + const suidRef = useRef<string | undefined>(suid); + const timeDiffRef = useRef(timeDiff); + + useEffect(() => { + suidRef.current = suid; + timeDiffRef.current = timeDiff; + }, [suid, timeDiff]); + @@ const onDocumentKeyDown = (event: KeyboardEvent) => { if (battleRef.current) { @@ - const { actor } = calcActiveUser(battleRef.current, suid, timeDiff); + const currentSuid = suidRef.current; + const { actor } = calcActiveUser( + battleRef.current, + currentSuid, + timeDiffRef.current, + ); @@ - if (actor.controllerId === suid) { + if (actor.controllerId === currentSuid) {🤖 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 `@app/src/layout/Combat.tsx` around lines 429 - 466, The once-registered keyboard handler in onDocumentKeyDown is closing over stale suid and timeDiff, so the “w” action can use outdated session/clock state after mount. Update the handler to read the latest values at event time, either by moving suid and timeDiff into refs or by wiring the effect so the listener is refreshed when those values change, while keeping calcActiveUser and the battleRef.current lookup aligned with the live state.
🤖 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.
Outside diff comments:
In `@app/src/layout/Combat.tsx`:
- Around line 429-466: The once-registered keyboard handler in onDocumentKeyDown
is closing over stale suid and timeDiff, so the “w” action can use outdated
session/clock state after mount. Update the handler to read the latest values at
event time, either by moving suid and timeDiff into refs or by wiring the effect
so the listener is refreshed when those values change, while keeping
calcActiveUser and the battleRef.current lookup aligned with the live state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49007cbb-351a-4951-9dd7-fc80f44e2d20
📒 Files selected for processing (3)
app/src/layout/Combat.tsxapp/src/validators/combat.tsapp/tests/libs/combat/summon_template_lookup.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/validators/combat.ts
- app/tests/libs/combat/summon_template_lookup.test.ts
MathiasGruber
force-pushed
the
feature/player-piloted-summons
branch
from
55071f9 to
ce7bb35
Compare
Collaborator
|
Rebased this branch onto the latest Conflict resolved:
No migration conflicts were involved. The 16 branch commits replayed cleanly otherwise, and the PR is now mergeable. |
Collaborator
|
/tnr-review-now |
Contributor
Scope covered
Test steps executed
Findings (pass/fail, bugs, risks)
Screenshot index
RecommendationApprove. Core PR behavior worked in live preview testing. The only remaining gap is narrower UI coverage around the duplicate-cast rejection message while a live summon already exists. Screenshots01 profile after login 02 summon cast 03 piloted summon turn Run: View workflow run |
MathiasGruber
left a comment
MathiasGruber
left a comment
Collaborator
There was a problem hiding this comment.
Logic / readability / DRY review.
Contributor
"Completed" label removedThis PR has 3 unresolved review thread(s). The "Completed" label has been automatically removed. Please resolve all review threads before re-applying the label. Unresolved threads:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/libs/combat/tags.ts (1)
3120-3129: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude clones when selecting the summon to unsummon.
clone()creates clones withisSummon = true,controllerId = user.userId, andisOriginal = false, so this lookup can remove a dead/expired clone before the actual summon effect owner. Keep summon cleanup scoped to original summons.Proposed fix
- let idx = usersState.findIndex( - (u) => - u.isSummon && - u.controllerId === effect.creatorId && - !isLiveSummon(u, usersState, userEffects), - ); + const isOriginalSummonForEffect = (u: BattleUserState) => + u.isSummon && u.isOriginal !== false && u.controllerId === effect.creatorId; + + let idx = usersState.findIndex( + (u) => isOriginalSummonForEffect(u) && !isLiveSummon(u, usersState, userEffects), + ); if (idx === -1) { - idx = usersState.findIndex( - (u) => u.isSummon && u.controllerId === effect.creatorId, - ); + idx = usersState.findIndex(isOriginalSummonForEffect); }🤖 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 `@app/src/libs/combat/tags.ts` around lines 3120 - 3129, The summon cleanup in tags.ts is matching any isSummon unit for effect.creatorId, which can pick a clone instead of the real summon. Update the lookup around the unsummon selection logic to exclude non-original summons by checking isOriginal (and/or equivalent clone markers) in both the primary findIndex and the fallback path, so only the actual summon owner is removed. Use the existing isLiveSummon selection block as the reference point for the fix.
🤖 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 `@app/src/libs/combat/tags.ts`:
- Around line 11-16: The summon cleanup fallback in tags.ts can mistakenly pick
a clone because clone() entities also satisfy the summon-like checks. Update the
target selection logic to explicitly exclude isClone(u) before choosing the
entity, or switch to the shared summon helper used by hasLiveSummon/isLiveSummon
so the cleanup only matches real summons. Use the existing summon-related
symbols in this module to keep the filter aligned with the rest of the combat
logic.
---
Duplicate comments:
In `@app/src/libs/combat/tags.ts`:
- Around line 3120-3129: The summon cleanup in tags.ts is matching any isSummon
unit for effect.creatorId, which can pick a clone instead of the real summon.
Update the lookup around the unsummon selection logic to exclude non-original
summons by checking isOriginal (and/or equivalent clone markers) in both the
primary findIndex and the fallback path, so only the actual summon owner is
removed. Use the existing isLiveSummon selection block as the reference point
for the fix.
🪄 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: 90680522-c61d-48eb-9d5a-f9179dee3a62
📒 Files selected for processing (22)
app/src/app/combat/page.tsxapp/src/layout/Combat.tsxapp/src/libs/combat/actions.tsapp/src/libs/combat/ai_v2.tsapp/src/libs/combat/constants.tsapp/src/libs/combat/summon.tsapp/src/libs/combat/tags.tsapp/src/libs/combat/types.tsapp/src/libs/combat/util.tsapp/src/libs/threejs/combat.tsapp/src/server/api/routers/combat.tsapp/src/validators/combat.tsapp/tests/libs/combat/ai_v2_piloted_guard.test.tsapp/tests/libs/combat/orphan_summon.test.tsapp/tests/libs/combat/piloted_actionset.test.tsapp/tests/libs/combat/piloted_masking.test.tsapp/tests/libs/combat/piloted_summon.test.tsapp/tests/libs/combat/resolve_controlled_actor.test.tsapp/tests/libs/combat/summon.test.tsapp/tests/libs/combat/summon_template_lookup.test.tsapp/tests/libs/combat/summon_turn_order.test.tsapp/tests/libs/combat/turn_control.test.ts
💤 Files with no reviewable changes (9)
- app/tests/libs/combat/piloted_actionset.test.ts
- app/tests/libs/combat/resolve_controlled_actor.test.ts
- app/tests/libs/combat/piloted_masking.test.ts
- app/tests/libs/combat/piloted_summon.test.ts
- app/tests/libs/combat/turn_control.test.ts
- app/tests/libs/combat/summon_turn_order.test.ts
- app/tests/libs/combat/summon.test.ts
- app/tests/libs/combat/orphan_summon.test.ts
- app/tests/libs/combat/summon_template_lookup.test.ts
✅ Files skipped from review due to trivial changes (3)
- app/src/libs/combat/constants.ts
- app/src/libs/threejs/combat.ts
- app/src/validators/combat.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- app/src/libs/combat/types.ts
- app/src/libs/combat/ai_v2.ts
- app/tests/libs/combat/ai_v2_piloted_guard.test.ts
- app/src/app/combat/page.tsx
- app/src/libs/combat/summon.ts
- app/src/libs/combat/actions.ts
- app/src/libs/combat/util.ts
- app/src/server/api/routers/combat.ts
- app/src/layout/Combat.tsx
github-actions
Bot
added
size:XL
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
3 participants
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.
Pull Request
Summary
Adds player-piloted summons: when a human player summons a creature in an interactive battle, they now drive that summon themselves on its turn (its own jutsu, items, action points and targeting) instead of handing it to the rule-based AI. Along the way the summon system was rewritten to fix long-standing issues (re-cast, one-per-controller, masterless summons), two regressions found during review were fixed, and the summon lifecycle logic was consolidated into a single tested module so it stops breaking on each change. Summons are now also correctly disabled in auto-resolved battle types (
KAGE_AI/CLAN_CHALLENGE).What was implemented
Player-piloted summons
isPilotedflag on the battle user state. A summon is piloted when its controller is a live human in a non-auto battle type (shouldPilotSummon).isAideliberately staystrue, so reward attribution, drop tables and the AI accounting path are unchanged —isPilotedonly affects turn routing and UI.getTurnControl/wantsHumanActionSet: on a piloted summon's turn the action is taken by the human controller, not the AI. The AI dispatch loop never auto-drives a piloted summon, and a defense-in-depth guard prevents the AI exhaustion path from self-killing a piloted summon.isPilotedto all viewers (added to the public allow-list) while the summon's private stats remain visible only to its controller (unchangedcontrollerId-based masking).resolveControlledActorIdhelper makes the combat UI follow the controlled actor — normally the player, transparently the summon on its turn — so the action menu, timer, AP, targeting, camera and keyboard end-turn all act for the summon without any extra UI. AI auto-play is suppressed for the player's own piloted summon.Summon system rewrite / hardening
hasLiveSummon) with a clear "already has a summon!" message; clears once the prior summon is dead/fled/left so re-summon works.isOrphanedSummon/spliceOrphanedSummons): removes a live masterless summon when its controller dies/flees/leaves, before actor selection, so the turn order never lands on or names a controllerless summon.Bug fixes (found in review)
userId, but battle-loaded AI templates carry their DB id incontrollerId(theiruserIdis a fresh nanoid). Restored thecontrollerIdmatch sosummon()finds the template again. Fixes "Failed to create summon!" for all summons.curHealth: 0), so subsequent casts had no template.isOrphanedSummonnow only treats live summons as orphans, leaving templates (and dead summons) in place. Both first-cast and re-cast now work.Hardening — single source of truth
app/src/libs/combat/summon.tsand replaced the hand-re-derived "template vs active summon" checks with one tested predicate set (isSummonTemplate/isActiveSummon/isLiveSummon), routed through the cap, orphan cleanup and AI gate. The summon-template lookup and teardown stay raw (controllerId-based) by design — routing them through the predicate would regress legacy lookups and dead-summon removal.isSummonTemplatemarker (set at load, cleared on spawn/clone), and enforced the templatecurHealth:0invariant in all battle types by guarding the AutoBattle pool reset with!user.isSummon— so a hidden off-grid template can never be selected as the active actor.spliceOrphanedSummonsnow classifies against a pre-mutation snapshot (order-independent).KAGE_AI/CLAN_CHALLENGE—summon()now refuses to spawn in auto-resolved battle types, where there are no human turns. Previously an AI participant there could spawn an AI-driven summon.Why
The summon feature previously ran entirely under AI, giving players no control over creatures they summoned. Piloting makes summons a real tactical tool. The rewrite also resolves re-cast and masterless-summon edge cases that were awkward in the old implementation.
Screenshots
TODO: add a combat screenshot showing the action panel switching to the summon's jutsu/AP on the summon's turn.
Breaking changes
No schema/API breaks; one intentional behavior change.
isPiloted(public) andisSummonTemplate(server-internal) are transient fields inside the battle-state JSON, not new columns.KAGE_AI,CLAN_CHALLENGE). Previously an AI participant there could spawn an AI-driven summon;summon()now refuses to spawn in those types, matching the design that those battles have no human turns. (Interactive battle types are unaffected.)Testing
bun test tests/libs/combat/→ 140 passed / 0 failed (14 files). Coverage spans the piloting suites (turn control, action-set selection, controlled-actor resolution, AI piloted-guard, masking, one-per-controller cap, orphan cleanup), thesummon()template-lookup regression tests (first-cast, re-cast-through-cleanup, template-kept, live-orphan-removed, turn-order placement, missing-AI message, auto-battle block), and the newsummon.tspredicate module (full template/spawned/clone matrix, legacy fallback, masking-absence, splice determinism,summonsAllowedInBattle).bun run typecheck→ exit 0; import cycle (summon ↔ actions ↔ tags) verified runtime-safe (combat route serves 200).Notes / follow-ups
aiId, which must exist as aUserDatarow to be loaded as a template. Only jutsu and item summon effects are scanned at battle init (not bloodline/skill effects). If a referenced AI is missing from the database, the summon now fails with a clearer message ("…'s summon creature could not be found.") rather than a generic error — this is a content/data concern, not a code issue (e.g. most summon AIs are unseeded in local dev DBs; production has them).License
By making this pull request, I confirm that I have the right to waive copyright and related rights to my contribution, and agree that all copyright and related rights in my contributions are waived, and I acknowledge that the Studie-Tech ApS organization has the copyright to use and modify my contribution for perpetuity.
Summary by CodeRabbit
New Features
Bug Fixes