Refactor Tracker Lifecycle and Game State Reset Flow#32
Conversation
📝 WalkthroughWalkthrough本次变更重构 Tracker 生命周期钩子与状态重置流程,调整开局和法术处理入口,停用部分交互与自动触发逻辑,扩展技能处理器,并同步更新测试、项目文档及仓库规则。 ChangesTracker 生命周期与入口
法术与工具行为
验证与仓库说明
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProtocolHandler
participant GameState
participant BrowserGameState
participant Room
ProtocolHandler->>GameState: 调用 init/start/enter/record
GameState->>BrowserGameState: 分发生命周期钩子
ProtocolHandler->>Room: 注册玩家
Room->>BrowserGameState: 同步房间座位
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tracker/gameState.ts (1)
213-219: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win修复
start()钩子因isGameStart被拦截的问题及相关测试。由于
init()已将isGameStart置为true,导致后续的正常流程中调用start()时,必定触发if (this.isGameStart) return提前返回。这将导致onStart()钩子(以及子类中的laya.init()等核心副作用)在真实对局流中永远无法被执行。测试用例由于在start()前意外插入了end()而掩盖了此缺陷。
src/tracker/gameState.ts#L213-L219: 引入独立的防重入标志(如this.hasTriggeredStart,并在resetSessionState中将其初始化为false)来替代此处的this.isGameStart拦截条件。tests/tracker/room.import.test.ts#L106-L108: 移除为绕过上述拦截而多余调用的gameState.end()。tests/tracker/room.import.test.ts#L120-L120: 同步更新事件断言,移除数组中多余的首个'end'字符串,使测试匹配真实的init -> start -> ...执行流。🤖 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/tracker/gameState.ts` around lines 213 - 219, Update gameState.ts lines 213-219 in start() to guard against repeated start hooks with a dedicated hasTriggeredStart flag, initialized to false in resetSessionState, while preserving isGameStart for game-state tracking. In tests/tracker/room.import.test.ts lines 106-108, remove the unnecessary gameState.end() call; at lines 120-120, remove the leading 'end' event assertion so the expected sequence matches init -> start -> ... .
🧹 Nitpick comments (1)
src/handler/skills/JiZhan.js (1)
23-36: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using template literals for better readability.
可以使用模板字符串(Template Literals)来替代多行字符串拼接,使 HTML 结构的构造更加直观清晰。
💡 可选的重构建议
- updateResult( - '<span class="textRes">【吉占】猜' + - (g > l ? '大' : '小') + - '</span>' + - '<br><span class="textRes">跟' + - cardNum + - '比,' + - g + - '张大\t\t' + - l + - '张小\t\t' + - e + - '平</span>' - ) + updateResult( + `<span class="textRes">【吉占】猜${g > l ? '大' : '小'}</span><br><span class="textRes">跟${cardNum}比,${g}张大\t\t${l}张小\t\t${e}平</span>` + )🤖 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/handler/skills/JiZhan.js` around lines 23 - 36, Refactor the HTML message construction passed to updateResult to use a template literal instead of chained string concatenation, preserving the existing text, interpolation of g, l, cardNum, and e, and rendered line-break and span structure.
🤖 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/draw.js`:
- Around line 472-484: Remove or revise the automatic-allocation hint associated
with the disabled handlers in the surrounding draw logic, including the message
near the button setup. Ensure the UI no longer claims that clicking the numbers
performs automatic allocation, while preserving the current click behavior that
bubbles to the button and copies text.
In `@src/utils/index.js`:
- Around line 261-264: Update setSuitRecord to verify that document is available
before calling document.getElementById, returning early in non-DOM environments
while preserving the existing missing-element guard and rendering behavior.
---
Outside diff comments:
In `@src/tracker/gameState.ts`:
- Around line 213-219: Update gameState.ts lines 213-219 in start() to guard
against repeated start hooks with a dedicated hasTriggeredStart flag,
initialized to false in resetSessionState, while preserving isGameStart for
game-state tracking. In tests/tracker/room.import.test.ts lines 106-108, remove
the unnecessary gameState.end() call; at lines 120-120, remove the leading 'end'
event assertion so the expected sequence matches init -> start -> ... .
---
Nitpick comments:
In `@src/handler/skills/JiZhan.js`:
- Around line 23-36: Refactor the HTML message construction passed to
updateResult to use a template literal instead of chained string concatenation,
preserving the existing text, interpolation of g, l, cardNum, and e, and
rendered line-break and span structure.
🪄 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 Plus
Run ID: da76fb09-9d9f-479b-a248-44abbe16f1b0
📒 Files selected for processing (23)
.gitignoreAGENTS.mddocs/agents/card_tracker.mddocs/agents/commands.mddocs/agents/lifecycle.mddocs/agents/overview.mddocs/agents/serena.mdsrc/draw.jssrc/handler/StartGame.jssrc/handler/skills/JiZhan.jssrc/handler/skills/YanXi.jssrc/handler/spellEffects.jssrc/logic.jssrc/tracker/Game.tssrc/tracker/Room.tssrc/tracker/gameState.tssrc/utils/index.jstests/tracker/gameFlowState.test.tstests/tracker/hiddenMarkCandidates.test.tstests/tracker/pileDisplayOrder.test.tstests/tracker/room.import.test.tstests/tracker/spellEffects.test.tstests/tracker/viewDirtyRender.test.ts
There was a problem hiding this comment.
Code Review
This pull request refactors the game state management and skill handlers, and updates the project documentation. Specifically, it splits GameState into a pure base class and a browser-specific subclass using protected lifecycle hooks (onInit, onEnd, etc.), extracts the JiZhan and YanXi skill handlers into separate files, and cleans up unused utility functions. Additionally, the test suite has been refactored to use parameterized tests, and the documentation has been updated to remove fragile line-number anchors. The review feedback correctly points out that the newly extracted JiZhan and YanXi handlers are commented out and missing imports in spellEffects.js, and provides suggestions to register them and update the corresponding test assertions.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
src/tracker/gameState.ts (3)
213-219: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value确认
onStart的触发时机是否符合预期。目前
this.onStart()被置于this.isGameStart保护判断之前调用,这意味着如果start()发生重复调用,onStart生命周期钩子也会被多次触发。如果原本意图是仅仅在游戏初次开始时触发一次该钩子,建议将其移动到状态判断逻辑的下方。♻️ 建议的修改(如果仅需触发一次)
start(): void { - this.onStart() if (this.isGameStart) return + this.onStart() this.isGameStart = true this.isPassed = false }🤖 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/tracker/gameState.ts` around lines 213 - 219, 调整 GameState.start() 中 onStart() 的调用时机:先执行 isGameStart 的重复启动判断,仅在首次启动并完成状态更新后触发 onStart(),确保重复调用 start() 不会再次触发生命周期钩子。
259-260: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value移除注释掉的代码。
建议直接移除注释掉的无用代码,以保持代码库的整洁。
♻️ 建议的修改
- // 第一轮开始时 检测开始状态 - // if (turn === 1) this.start()🤖 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/tracker/gameState.ts` around lines 259 - 260, Remove the commented-out this.start() call beneath the first-round start-state comment in the turn-handling logic, while leaving the surrounding active code and meaningful comment unchanged.
101-110: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value删除
setTurn()里的注释代码
src/tracker/gameState.ts:259-260这段// if (turn === 1) this.start()已经不会执行,保留只会增加噪音。🤖 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/tracker/gameState.ts` around lines 101 - 110, Remove the obsolete commented-out `if (turn === 1) this.start()` code from the `setTurn()` method in `gameState.ts`, leaving the method’s active turn-handling logic unchanged.Sources: Coding guidelines, Learnings
src/handler/skills/JiZhan.js (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议替换 HTML 字符串中的
\t\t以确保正确的视觉间距。在标准的 HTML 渲染中,连续的空白字符(包括制表符
\t)通常会被折叠成一个普通的空格。如果使用\t\t是为了在 UI 上提供视觉层面的分隔,它在浏览器中实际上可能无法生效。建议使用 HTML 实体(如 )或通过外层 CSS 样式控制来实现间距。🎨 建议的修复方案(使用 HTML 实体)
- `<span class="textRes">【吉占】猜${g > l ? '大' : '小'}</span><br><span class="textRes">跟${cardNum}比,${g}张大\t\t${l}张小\t\t${e}平</span>` + `<span class="textRes">【吉占】猜${g > l ? '大' : '小'}</span><br><span class="textRes">跟${cardNum}比,${g}张大 ${l}张小 ${e}平</span>`🤖 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/handler/skills/JiZhan.js` at line 24, Update the HTML string in the JiZhan result rendering to replace the literal tab separators between the 大、小、平 counts with reliable HTML spacing, preferably using the appropriate non-breaking-space entity. Preserve the existing text content and interpolation logic.
🤖 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 `@src/handler/skills/JiZhan.js`:
- Line 24: Update the HTML string in the JiZhan result rendering to replace the
literal tab separators between the 大、小、平 counts with reliable HTML spacing,
preferably using the appropriate non-breaking-space entity. Preserve the
existing text content and interpolation logic.
In `@src/tracker/gameState.ts`:
- Around line 213-219: 调整 GameState.start() 中 onStart() 的调用时机:先执行 isGameStart
的重复启动判断,仅在首次启动并完成状态更新后触发 onStart(),确保重复调用 start() 不会再次触发生命周期钩子。
- Around line 259-260: Remove the commented-out this.start() call beneath the
first-round start-state comment in the turn-handling logic, while leaving the
surrounding active code and meaningful comment unchanged.
- Around line 101-110: Remove the obsolete commented-out `if (turn === 1)
this.start()` code from the `setTurn()` method in `gameState.ts`, leaving the
method’s active turn-handling logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f957ad3c-30ab-4cf9-9e35-23e2bd354a74
📒 Files selected for processing (5)
src/draw.jssrc/handler/skills/JiZhan.jssrc/tracker/Game.tssrc/tracker/gameState.tssrc/utils/index.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/draw.js
- src/utils/index.js
- src/tracker/Game.ts
Summary by CodeRabbit