added win streaks, Maria will be happy :)#20
added win streaks, Maria will be happy :)#20matteomekhail wants to merge 2 commits intoT3-Content:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds per-player streak tracking to the game. A Changes
Sequence DiagramsequenceDiagram
participant Game as Game Logic
participant Server as Server State
participant Client as Client State
participant UI as UI Components
Game->>Game: Process round outcome
activate Game
Game->>Game: Increment winner streak / Reset loser streak
deactivate Game
Server->>Server: Update streaks in GameState
Server->>Client: Send streaks via getClientState()
Client->>Client: Store streaks in state
Client->>UI: Pass streaks to Arena & Standings
UI->>UI: ContestantCard renders badge if streak >= 2
UI->>UI: LeaderboardSection renders badge if streak >= 2
UI->>UI: Display flame emoji with highlight
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
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 |
Add win streak tracking and render 🔥 badges for streaks ≥2 across game state, server init, and UI (Maria will be happy)Add 📍Where to StartStart in the Macroscope summarized 8bad3a2. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
frontend.css (1)
236-243:margin-left: autoon.streak-badgemay conflict with.lb-entry__score'smargin-left: auto.In
.lb-entry__top, both.streak-badgeand.lb-entry__scoreusemargin-left: auto. The first auto-margin consumes all free space, so the score will sit flush against the streak badge instead of being independently right-aligned. Visually this may be acceptable (both cluster on the right), but verify the spacing looks intentional in the leaderboard entries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.css` around lines 236 - 243, In .lb-entry__top, both .streak-badge and .lb-entry__score use margin-left: auto causing the first auto margin to consume available space; update the CSS so only the element intended to be right-aligned uses margin-left:auto (or switch the container to display:flex and use gap/justify-content to control spacing). Specifically, remove or change margin-left:auto on .streak-badge (or replace it with a fixed margin like margin-left:8px or margin-inline-start) and ensure .lb-entry__score retains margin-left:auto (or alternatively make .lb-entry__top a flex container and use gap to separate .streak-badge and .lb-entry__score) so the score and badge align and space as intended.frontend.tsx (2)
635-635: Line is quite long but functionally correct.The
?? {}fallbacks forviewerScoresandstreaksare defensive against potentially undefined server state during initial connection. Consider breaking this into multiple lines for readability.Proposed formatting
- <Standings scores={state.scores} viewerScores={state.viewerScores ?? {}} streaks={state.streaks ?? {}} activeRound={state.active} /> + <Standings + scores={state.scores} + viewerScores={state.viewerScores ?? {}} + streaks={state.streaks ?? {}} + activeRound={state.active} + />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` at line 635, The JSX line rendering the Standings component is long; split the props across multiple lines or precompute fallback values to improve readability: either break the <Standings ... /> element so each prop (scores, viewerScores, streaks, activeRound) sits on its own line, or create local constants like safeViewerScores = state.viewerScores ?? {} and safeStreaks = state.streaks ?? {} then pass those into <Standings scores={state.scores} viewerScores={safeViewerScores} streaks={safeStreaks} activeRound={state.active}>, keeping the same defensive ?? {} behavior.
432-434: Non-null assertionstreaks![name]is safe here, but could be cleaner.The guard
(streaks?.[name] || 0) >= 2ensuresstreaksis defined and the value is ≥ 2, sostreaks![name]won't beundefined. Consider usingstreaks?.[name]for consistency to avoid the!assertion.Proposed fix
{(streaks?.[name] || 0) >= 2 && ( - <span className="streak-badge">🔥{streaks![name]}</span> + <span className="streak-badge">🔥{streaks?.[name]}</span> )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend.tsx` around lines 432 - 434, Replace the non-null assertion in the JSX span content with a safe optional access to match the guard: in the conditional that uses (streaks?.[name] || 0) >= 2, change the displayed value from streaks![name] to streaks?.[name] (or streaks?.[name] ?? 0) so you avoid the `!` assertion while preserving the same behavior for the streak-badge span.quipslop.tsx (1)
227-227: Minor inconsistency:streaksinitialized differently fromscoresandviewerScores.
scoresandviewerScoresare pre-populated with all model names keyed to0, butstreaksstarts as{}. This works because consumers use|| 0fallbacks, but it's inconsistent. Consider initializing it the same way for uniformity.Proposed fix
- streaks: {}, + streaks: Object.fromEntries(MODELS.map((m) => [m.name, 0])),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@quipslop.tsx` at line 227, streaks is initialized as an empty object while scores and viewerScores are pre-populated with all model names set to 0; change the initialization of streaks to use the same pre-population pattern (e.g., use the same helper or mapping used for scores/viewerScores or iterate MODEL_NAMES) so streaks contains every model key with value 0, ensuring consistency with scores and viewerScores and avoiding reliance on "|| 0" fallbacks in consumers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@frontend.css`:
- Around line 236-243: In .lb-entry__top, both .streak-badge and
.lb-entry__score use margin-left: auto causing the first auto margin to consume
available space; update the CSS so only the element intended to be right-aligned
uses margin-left:auto (or switch the container to display:flex and use
gap/justify-content to control spacing). Specifically, remove or change
margin-left:auto on .streak-badge (or replace it with a fixed margin like
margin-left:8px or margin-inline-start) and ensure .lb-entry__score retains
margin-left:auto (or alternatively make .lb-entry__top a flex container and use
gap to separate .streak-badge and .lb-entry__score) so the score and badge align
and space as intended.
In `@frontend.tsx`:
- Line 635: The JSX line rendering the Standings component is long; split the
props across multiple lines or precompute fallback values to improve
readability: either break the <Standings ... /> element so each prop (scores,
viewerScores, streaks, activeRound) sits on its own line, or create local
constants like safeViewerScores = state.viewerScores ?? {} and safeStreaks =
state.streaks ?? {} then pass those into <Standings scores={state.scores}
viewerScores={safeViewerScores} streaks={safeStreaks}
activeRound={state.active}>, keeping the same defensive ?? {} behavior.
- Around line 432-434: Replace the non-null assertion in the JSX span content
with a safe optional access to match the guard: in the conditional that uses
(streaks?.[name] || 0) >= 2, change the displayed value from streaks![name] to
streaks?.[name] (or streaks?.[name] ?? 0) so you avoid the `!` assertion while
preserving the same behavior for the streak-badge span.
In `@quipslop.tsx`:
- Line 227: streaks is initialized as an empty object while scores and
viewerScores are pre-populated with all model names set to 0; change the
initialization of streaks to use the same pre-population pattern (e.g., use the
same helper or mapping used for scores/viewerScores or iterate MODEL_NAMES) so
streaks contains every model key with value 0, ensuring consistency with scores
and viewerScores and avoiding reliance on "|| 0" fallbacks in consumers.
Its placement is making the bar appear smaller.
|
2 participants
pretty explanatory from the title, i do have written a test, not sure if was good to merge tho
Summary by CodeRabbit