Skip to content

Fix seat candidate handling and adjust seat UI layout and tests#47

Merged
llsccm merged 2 commits into
devfrom
codex/fix-secondary-random-hand-transfer
Jul 21, 2026
Merged

Fix seat candidate handling and adjust seat UI layout and tests#47
llsccm merged 2 commits into
devfrom
codex/fix-secondary-random-hand-transfer

Conversation

@llsccm

@llsccm llsccm commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • 新功能

    • 优化座位相关手牌候选的传播逻辑,支持在已有位置候选时追加新的座位候选。
    • 改善座位旁手牌区域的布局、定位与尺寸表现。
  • 问题修复

    • 修复座位信息缺失时可能导致的判断问题。
    • 修正手牌区域宽度在缩放场景下的计算,提升显示准确性。
    • 调整红色卡牌文字颜色,增强可读性。
  • 测试

    • 增加座位候选追加及二次随机手牌转移场景的回归测试。

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

本次更新修正完整位置候选下的座位候选追加逻辑,补充随机手牌转移回归测试,并调整座位 UI 的布局、尺寸计算和绘制条件。

Changes

候选座位传播

Layer / File(s) Summary
座位候选扩展
src/tracker/Card.ts
addSeat 在已有 locationCandidates 时基于现有玩家候选生成模板,并追加新座位对应的玩家位置候选。
随机转移候选验证
src/tracker/roomMovement/candidates.ts, tests/tracker/locationCandidates.test.ts, tests/tracker/secondaryRandomHandTransfer.test.ts
补充候选追加语义说明,并验证单张及全部手牌转移时的座位候选传播和观测数量变化。

座位界面布局

Layer / File(s) Summary
座位界面布局与尺寸
html/iframe.html, src/draw.js, src/tracker/state.ts
调整座位容器定位与 Flexbox 样式、红色卡牌颜色,以及座位旁手牌镜像容器的默认宽度计算。
座位 UI 绘制兼容
src/dom.js
保留尺寸更新防抖流程,并使用可选链判断 seatUIs 是否存在后触发绘制。

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • llsccm/tracker#12:涉及 locationCandidates 作为候选写入来源及 addSeat 的迁移语义。
  • llsccm/tracker#22:涉及随机手牌转移中的位置候选约束收敛,与本次候选传播逻辑相连。
  • llsccm/tracker#34:涉及二次暗转移中的候选传播和座位落点逻辑。
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title Check ✅ Passed Title check skipped as CodeRabbit has written the PR title.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-secondary-random-hand-transfer

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot changed the title @pr Fix seat candidate handling and adjust seat UI layout and tests Jul 20, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
html/iframe.html (1)

1166-1168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

不要让 .sorderContainer 脱离 seatUI 坐标系

这里改为 position: fixed 后,容器的 top/left 会相对 viewport 定位;但 src/draw.jsapplySeatContainerLayout() 直接使用座位相对坐标,而 src/dom.js 又对父级 seatUI 设置了居中的 left/top。这会导致座位手牌镜像整体偏移。除非同时增加坐标换算,否则应保留 absolute 定位。

建议修复
-  position: fixed;
+  position: absolute;
🤖 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 `@html/iframe.html` around lines 1166 - 1168, 将 .sorderContainer 的定位方式恢复为
absolute,保持其 top/left 在 seatUI 坐标系内解释。不要使用 fixed,除非同时修改
applySeatContainerLayout() 及 seatUI 的坐标换算以适配 viewport 定位。
🧹 Nitpick comments (1)
tests/tracker/secondaryRandomHandTransfer.test.ts (1)

98-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

移除冗余的断言以简化代码。

expectSeats 辅助函数内部已经遍历并校验了传入的所有卡牌 ID(其中包括 knownIDs 里的 112)。因此,第 99 行专门针对 card112.seats 的重复断言可以安全移除。

💡 建议的修改
-      expectSeats([...knownIDs, ...hiddenIDs], room, expectedSeats)
-      expect(Array.from(card112.seats).map(Number).sort((a, b) => a - b)).toEqual(expectedSeats)
+      expectSeats([...knownIDs, ...hiddenIDs], room, expectedSeats)
🤖 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 `@tests/tracker/secondaryRandomHandTransfer.test.ts` around lines 98 - 99,
Remove the redundant card112.seats assertion after the expectSeats call;
expectSeats already validates all IDs in knownIDs, including 112, so leave the
consolidated expectSeats check unchanged.
🤖 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 `@html/iframe.html`:
- Around line 1166-1168: 将 .sorderContainer 的定位方式恢复为 absolute,保持其 top/left 在
seatUI 坐标系内解释。不要使用 fixed,除非同时修改 applySeatContainerLayout() 及 seatUI 的坐标换算以适配
viewport 定位。

---

Nitpick comments:
In `@tests/tracker/secondaryRandomHandTransfer.test.ts`:
- Around line 98-99: Remove the redundant card112.seats assertion after the
expectSeats call; expectSeats already validates all IDs in knownIDs, including
112, so leave the consolidated expectSeats check unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3c505e63-856b-495a-9830-d49c157bc04f

📥 Commits

Reviewing files that changed from the base of the PR and between 50829d3 and 81b9ace.

📒 Files selected for processing (8)
  • html/iframe.html
  • src/dom.js
  • src/draw.js
  • src/tracker/Card.ts
  • src/tracker/roomMovement/candidates.ts
  • src/tracker/state.ts
  • tests/tracker/locationCandidates.test.ts
  • tests/tracker/secondaryRandomHandTransfer.test.ts

@llsccm
llsccm merged commit 724e0c0 into dev Jul 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant