add bar battle animation to possession pitch visualization#26
Merged
Conversation
Animated "bar battle" plays on the pitch after each round to visualize how points affect possession. When players earn points, +N text appears near their avatar, then flies into their zone and spawns bars (1 bar = 10 points). Bars march toward the center, cancel one-by-one, and remaining bars push the possession line. Works for all question types (MC, countdown, clues, putInOrder). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR adds a multi-phase bar-battle animation system to possession match UI. A new ChangesBar Battle Feature
Sequence DiagramsequenceDiagram
participant Controller as Realtime Controller
participant Hook as useBarBattle
participant Pitch as PitchVisualization
participant Overlay as BarBattleOverlay
Controller->>Hook: answerAck & opponent score events
Hook->>Hook: track shown scores, update to<br/>both-score phase if ready
Controller->>Hook: roundResult arrives with point deltas
Hook->>Hook: compute bar counts & cancellation
Hook->>Hook: init battle state, schedule transitions
loop Phase transitions (80ms → 120ms → 80ms → 400ms → 100ms)
Hook->>Pitch: barBattle.phase updated
Pitch->>Overlay: render with current phase
Overlay->>Overlay: animate bars per phase<br/>(spawn → march → collision → push)
end
Hook->>Hook: cleanup & reset after done phase
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
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 docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 3
🧹 Nitpick comments (2)
src/features/possession/hooks/__tests__/usePossessionScoreSplashes.test.ts (1)
42-137: ⚡ Quick winAdd a test for the new non-MC
myRoundsplash path.Current updates only pass
myRound: null; they don’t assert the new behavior added in the hook (roundResult + myRound,selectedAnswer === null). Please add one case to verify splash appears withmyRound.pointsEarned > 0and is deduped perqIndex.🤖 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 `@src/features/possession/hooks/__tests__/usePossessionScoreSplashes.test.ts` around lines 42 - 137, Add a new test in usePossessionScoreSplashes.test.ts that passes a non-null myRound (with myRound.pointsEarned > 0) and a non-null roundResult into usePossessionScoreSplashes while selectedAnswer is null and selectedAnswerQIndex set (e.g. 5), then assert showPlayerSplash is true and playerSplashPoints equals the combination of roundResult and myRound.pointsEarned; also verify deduping by calling the hook twice with the same selectedAnswerQIndex (splash only once) and then with a different selectedAnswerQIndex to confirm a new splash is produced. Ensure the new test references usePossessionScoreSplashes, myRound, roundResult, selectedAnswer, and selectedAnswerQIndex so reviewers can locate the relevant logic.src/features/possession/components/BarBattleOverlay.tsx (1)
68-76: ⚡ Quick winUse shared game typography token for score labels
Line 75 hardcodes Poppins. Prefer the shared
font-funstyle for in-game text consistency.As per coding guidelines,
src/{components,features,app}/**/*.{ts,tsx}: “Usefont-funclass for game typography (Nunito font)”.🤖 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 `@src/features/possession/components/BarBattleOverlay.tsx` around lines 68 - 76, The SVG text element in BarBattleOverlay (motion.text) hardcodes Poppins via fontFamily; replace that with the shared game typography by removing the inline fontFamily and applying the `font-fun` token (e.g. add className="font-fun" to the motion.text) so score labels use the project's Nunito game font; keep the other props (x, y, textAnchor, fill, fontSize, fontWeight, style) intact.
🤖 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 `@src/features/possession/hooks/useBarBattle.ts`:
- Around line 125-131: The effect in useBarBattle.ts computes qIndex only from
roundResult or answerAck so when the opponent answers first qIndex can be null
and the effect returns early; update the hook signature (useBarBattle) to accept
a currentQuestionIndex param and use it as a final fallback when deriving qIndex
(e.g. qIndex = roundResult?.qIndex ?? answerAck?.qIndex ?? currentQuestionIndex
?? null) so the effect runs for opponent-first answers, and update all call
sites to pass the current question index into useBarBattle.
- Around line 31-35: BAR_BATTLE_TOTAL_MS is set too low relative to the declared
phase timings and MAX_BARS (12), causing orchestrator locks to expire early;
update BAR_BATTLE_TOTAL_MS so it reflects the true worst-case timeline (≈4700ms)
or replace the hardcoded export with a computed value derived from the per-phase
durations and MAX_BARS (eg. add fixed overhead + per-bar durations * MAX_BARS)
and export that computed value or a getter (reference symbols:
BAR_BATTLE_TOTAL_MS, MAX_BARS, POINTS_PER_BAR, useBarBattle) so
callers/orchestrator consume an accurate bound.
In `@src/features/possession/hooks/usePossessionScoreSplashes.ts`:
- Around line 90-105: This branch can display a splash for a stale question
because it never verifies roundResult.qIndex is the active question; add a guard
(similar to the MC path) that checks roundResult.qIndex matches the current
active question index/state before queuing the splash and updating
shownSplashQRef; update the useEffect to return early if roundResult.qIndex !==
<the component's active question index/state> (use the same variable used in the
MC path) so that setPlayerSplashVariant, setPlayerSplashPoints,
setShowPlayerSplash and shownSplashQRef.current.player only run for the current
question.
---
Nitpick comments:
In `@src/features/possession/components/BarBattleOverlay.tsx`:
- Around line 68-76: The SVG text element in BarBattleOverlay (motion.text)
hardcodes Poppins via fontFamily; replace that with the shared game typography
by removing the inline fontFamily and applying the `font-fun` token (e.g. add
className="font-fun" to the motion.text) so score labels use the project's
Nunito game font; keep the other props (x, y, textAnchor, fill, fontSize,
fontWeight, style) intact.
In `@src/features/possession/hooks/__tests__/usePossessionScoreSplashes.test.ts`:
- Around line 42-137: Add a new test in usePossessionScoreSplashes.test.ts that
passes a non-null myRound (with myRound.pointsEarned > 0) and a non-null
roundResult into usePossessionScoreSplashes while selectedAnswer is null and
selectedAnswerQIndex set (e.g. 5), then assert showPlayerSplash is true and
playerSplashPoints equals the combination of roundResult and
myRound.pointsEarned; also verify deduping by calling the hook twice with the
same selectedAnswerQIndex (splash only once) and then with a different
selectedAnswerQIndex to confirm a new splash is produced. Ensure the new test
references usePossessionScoreSplashes, myRound, roundResult, selectedAnswer, and
selectedAnswerQIndex so reviewers can locate the relevant logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 65bf5b3d-03da-4c27-a9a7-a77abeb6890e
📒 Files selected for processing (7)
src/features/possession/components/BarBattleOverlay.tsxsrc/features/possession/components/PitchVisualization.tsxsrc/features/possession/hooks/__tests__/usePossessionScoreSplashes.test.tssrc/features/possession/hooks/useBarBattle.tssrc/features/possession/hooks/usePossessionScoreSplashes.tssrc/features/possession/hooks/useRealtimePossessionMatchController.tssrc/features/possession/realtimePossession.helpers.ts
Comment on lines
+31
to
+35
| /** Total time from convert start. Export so orchestrator can size its lock window. */ | ||
| export const BAR_BATTLE_TOTAL_MS = 2800; // Conservative upper bound | ||
|
|
||
| const POINTS_PER_BAR = 10; | ||
| const MAX_BARS = 12; |
Contributor
There was a problem hiding this comment.
BAR_BATTLE_TOTAL_MS is below the configured worst-case timeline
Line 32 sets 2800ms, but the declared phase timings with MAX_BARS = 12 can exceed that by a lot (~4.7s end-to-end). Any orchestrator lock using this bound can release early and desync/clip the animation.
Suggested fix
-/** Total time from convert start. Export so orchestrator can size its lock window. */
-export const BAR_BATTLE_TOTAL_MS = 2800; // Conservative upper bound
-
const POINTS_PER_BAR = 10;
const MAX_BARS = 12;
+
+/** Worst-case total timeline (includes hold/cleanup). */
+export const BAR_BATTLE_TOTAL_MS =
+ BOTH_SCORE_HOLD_MS +
+ CONVERT_DURATION +
+ (BARS_SPAWN_BASE_MS + MAX_BARS * BARS_PER_STAGGER_MS) +
+ (BATTLE_BASE_MS + MAX_BARS * BATTLE_PER_BAR_MS) +
+ RESULT_HOLD_MS +
+ DONE_LINGER_MS;📝 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
| /** Total time from convert start. Export so orchestrator can size its lock window. */ | |
| export const BAR_BATTLE_TOTAL_MS = 2800; // Conservative upper bound | |
| const POINTS_PER_BAR = 10; | |
| const MAX_BARS = 12; | |
| const POINTS_PER_BAR = 10; | |
| const MAX_BARS = 12; | |
| /** Worst-case total timeline (includes hold/cleanup). */ | |
| export const BAR_BATTLE_TOTAL_MS = | |
| BOTH_SCORE_HOLD_MS + | |
| CONVERT_DURATION + | |
| (BARS_SPAWN_BASE_MS + MAX_BARS * BARS_PER_STAGGER_MS) + | |
| (BATTLE_BASE_MS + MAX_BARS * BATTLE_PER_BAR_MS) + | |
| RESULT_HOLD_MS + | |
| DONE_LINGER_MS; |
🤖 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 `@src/features/possession/hooks/useBarBattle.ts` around lines 31 - 35,
BAR_BATTLE_TOTAL_MS is set too low relative to the declared phase timings and
MAX_BARS (12), causing orchestrator locks to expire early; update
BAR_BATTLE_TOTAL_MS so it reflects the true worst-case timeline (≈4700ms) or
replace the hardcoded export with a computed value derived from the per-phase
durations and MAX_BARS (eg. add fixed overhead + per-bar durations * MAX_BARS)
and export that computed value or a getter (reference symbols:
BAR_BATTLE_TOTAL_MS, MAX_BARS, POINTS_PER_BAR, useBarBattle) so
callers/orchestrator consume an accurate bound.
Comment on lines
+125
to
+131
| useEffect(() => { | ||
| if (!opponentAnswered && !roundResult) return; | ||
|
|
||
| // Determine qIndex from whatever source is available | ||
| const qIndex = roundResult?.qIndex ?? answerAck?.qIndex ?? null; | ||
| if (qIndex === null) return; | ||
|
|
Contributor
There was a problem hiding this comment.
Opponent-first answers can miss immediate score rendering
At Line 129, qIndex falls back only to roundResult/answerAck. If opponent answers first, both can be null, so the effect exits at Line 130 and opponent score text is delayed.
Pass current question index into the hook and use it as a fallback when deriving qIndex.
🤖 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 `@src/features/possession/hooks/useBarBattle.ts` around lines 125 - 131, The
effect in useBarBattle.ts computes qIndex only from roundResult or answerAck so
when the opponent answers first qIndex can be null and the effect returns early;
update the hook signature (useBarBattle) to accept a currentQuestionIndex param
and use it as a final fallback when deriving qIndex (e.g. qIndex =
roundResult?.qIndex ?? answerAck?.qIndex ?? currentQuestionIndex ?? null) so the
effect runs for opponent-first answers, and update all call sites to pass the
current question index into useBarBattle.
Comment on lines
+90
to
+105
| useEffect(() => { | ||
| if (!roundResult || !myRound) return; | ||
| const resolvedPhaseKind = roundResult.phaseKind ?? phaseKind; | ||
| if (resolvedPhaseKind !== 'normal' && resolvedPhaseKind !== 'last_attack') return; | ||
| if (shownSplashQRef.current.player === roundResult.qIndex) return; | ||
| // Only for non-MC: if selectedAnswer was set, the MC path above already handled it | ||
| if (selectedAnswer !== null) return; | ||
| if (!myRound.isCorrect || myRound.pointsEarned <= 0) return; | ||
|
|
||
| queueMicrotask(() => { | ||
| setPlayerSplashVariant('points'); | ||
| setPlayerSplashPoints(myRound.pointsEarned); | ||
| setShowPlayerSplash(true); | ||
| }); | ||
| shownSplashQRef.current.player = roundResult.qIndex; | ||
| }, [roundResult, myRound, phaseKind, selectedAnswer]); |
Contributor
There was a problem hiding this comment.
Guard non-MC splash against stale roundResult events.
Line [94]-[104] can show a player splash for an old question because this branch never verifies that roundResult.qIndex is still the active question (unlike the MC path). Late realtime packets can therefore animate points on the wrong round.
Suggested fix
useEffect(() => {
if (!roundResult || !myRound) return;
const resolvedPhaseKind = roundResult.phaseKind ?? phaseKind;
if (resolvedPhaseKind !== 'normal' && resolvedPhaseKind !== 'last_attack') return;
+ const activeQIndex = localQuestion?.qIndex ?? roundResult.qIndex;
+ if (activeQIndex !== roundResult.qIndex) return;
if (shownSplashQRef.current.player === roundResult.qIndex) return;
// Only for non-MC: if selectedAnswer was set, the MC path above already handled it
if (selectedAnswer !== null) return;
if (!myRound.isCorrect || myRound.pointsEarned <= 0) return;
@@
- }, [roundResult, myRound, phaseKind, selectedAnswer]);
+ }, [localQuestion?.qIndex, roundResult, myRound, phaseKind, selectedAnswer]);🤖 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 `@src/features/possession/hooks/usePossessionScoreSplashes.ts` around lines 90
- 105, This branch can display a splash for a stale question because it never
verifies roundResult.qIndex is the active question; add a guard (similar to the
MC path) that checks roundResult.qIndex matches the current active question
index/state before queuing the splash and updating shownSplashQRef; update the
useEffect to return early if roundResult.qIndex !== <the component's active
question index/state> (use the same variable used in the MC path) so that
setPlayerSplashVariant, setPlayerSplashPoints, setShowPlayerSplash and
shownSplashQRef.current.player only run for the current question.
This was referenced May 16, 2026
Merged
Merged
Merged
Merged
Merged
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.
Animated "bar battle" plays on the pitch after each round to visualize how points affect possession. When players earn points, +N text appears near their avatar, then flies into their zone and spawns bars (1 bar = 10 points). Bars march toward the center, cancel one-by-one, and remaining bars push the possession line. Works for all question types (MC, countdown, clues, putInOrder).
Summary by CodeRabbit
New Features
Chores