Skip to content

fix: 优先用确定来源暗牌做身份交换并修正牌顶候选#34

Merged
llsccm merged 3 commits into
devfrom
fix/top/xingxue
Jul 17, 2026
Merged

fix: 优先用确定来源暗牌做身份交换并修正牌顶候选#34
llsccm merged 3 commits into
devfrom
fix/top/xingxue

Conversation

@llsccm

@llsccm llsccm commented Jul 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • 玩家来源明牌与暗占位交换身份时,优先使用已经确定属于来源位置的暗实体;只有不存在确定来源实体时,才解析跨座位暗候选,避免按实体数组顺序提前收敛随机手牌转移约束。
  • 未公开正 ID 暗实体若碰巧命中跨座位候选,会由确定来源暗实体接管原约束后再公开该 ID,不再把内部身份占位误当成位置确认事实。
  • 手牌未知数观察会把跨座位歧义暗覆盖计入覆盖量,避免重复补建占位或错误回收。
  • 兼容 position: 'top' | 'bottom' 字符串,修正暗手牌置于牌堆顶时的公共候选文案。

Why

随机获得后使用自身暗牌时,内部暗实体 ID 可能碰巧命中转移候选;旧逻辑会按实体顺序过早确认来源,导致候选明牌被错误排除,约束组座位名额也提前坍缩。

Impact

  • 随机手牌转移后的座位/手牌候选更稳定,减少误收敛。
  • 牌堆顶/底公共候选展示更准确。
  • 开发者文档补充了相关约束优先级说明。

Validation

  • pnpm test:tracker(189 passed)
  • pnpm lint
  • pnpm typecheck:tracker

Summary by CodeRabbit

  • 功能改进
    • 优化匿名手牌数量与缺口/盈余推断,减少随机转移后不必要的暗牌占位与回收偏差。
    • 改进玩家来源暗牌与已知牌的身份匹配与交换逻辑,增强对跨座位共享/模糊候选的处理准确性。
    • 完善牌堆顶/底位置候选的归一化与展示,提升牌堆顶文案识别一致性。
  • 文档
    • 更新房间规则说明,细化“来源明牌与暗占位交换身份”的对齐优先级。
  • 测试
    • 新增并扩展多场景用例,覆盖手牌数量观测、模糊覆盖、公共候选展示与随机转移边界行为。

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5bc4add5-52b8-4459-a07f-f7e7495cf197

📥 Commits

Reviewing files that changed from the base of the PR and between cec707b and a513f03.

📒 Files selected for processing (3)
  • src/tracker/roomConstraints.ts
  • src/tracker/roomMovement/sources.ts
  • tests/tracker/handCountObservation.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/tracker/roomConstraints.ts
  • src/tracker/roomMovement/sources.ts

📝 Walkthrough

Walkthrough

本次更新扩展了模糊暗手覆盖计算、玩家来源占位匹配与身份置换逻辑,并补充公共位置归一化及相关测试,覆盖随机手牌转移、约束组同步和牌堆顶候选描述。

Changes

匿名手牌与模糊覆盖

Layer / File(s) Summary
模糊暗手覆盖与匿名对账
src/tracker/roomConstraints.ts, src/tracker/Room.ts, tests/tracker/handCountObservation.test.ts
新增按席位合并暗实体覆盖的约束计算,并将结果纳入匿名手牌缺口与盈余处理;测试覆盖共享暗实体和随机转移场景。

玩家来源占位与身份置换

Layer / File(s) Summary
来源占位匹配与身份置换
src/tracker/roomMovement/sources.ts, src/tracker/roomMovement.ts, src/tracker/roomConstraints.ts, tests/tracker/trackerController.test.ts, tests/tracker/hiddenMarkCandidates.test.ts, tests/tracker/handCountObservation.test.ts, docs/agents/card_tracker.md
统一精确优先的未知来源卡牌筛选,扩展明牌与暗占位交换条件,并在歧义身份场景中同步约束组卡牌身份。
公共位置归一化与候选断言
src/tracker/candidate/publicCandidate.ts, tests/tracker/publicCandidates.test.ts, tests/tracker/ambiguousKnownIndexIncremental.test.ts
支持字面量 topbottom 的位置归一化,并更新牌堆顶候选描述及测试断言。

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • llsccm/tracker#12:同样涉及 swapKnownCardWithPlayerSourcePlaceholder 的候选迁移与身份处理。
  • llsccm/tracker#21:同样涉及匿名手牌对账与随机转移后的收敛流程。
  • llsccm/tracker#20:该 PR 引入的 movement sources 结构与本次来源候选逻辑直接衔接。
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了本次修复的两项核心变化:优先使用确定来源暗牌做身份交换,并修正牌顶候选处理。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ 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 fix/top/xingxue

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.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request refactors the card tracker logic to prevent premature convergence of random hand transfer constraints and avoid duplicate anonymous card creation. It introduces tracking for ambiguous hidden hand coverage and refactors source card retrieval to prioritize exact unknown cards. Feedback on these changes suggests optimizing collectAmbiguousHiddenHandCoverage to reduce garbage collection pressure in a hot path, ensuring strict type safety for combinationID assignments in replaceCardInConstraintGroups, and avoiding redundant Set instantiations in getUnknownPlayerSourceCards.

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.

Comment thread src/tracker/roomConstraints.ts
Comment thread src/tracker/roomConstraints.ts
Comment thread src/tracker/roomMovement/sources.ts

@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.

Actionable comments posted: 2

🤖 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/tracker/roomConstraints.ts`:
- Around line 147-223: 修正 collectAmbiguousHiddenHandCoverage 中按任意 hiddenCards
交集直接跳过整个 coveragePackage 的逻辑。将共享实体的覆盖包按重叠连通分量合并,并结合各包的 coverageBySeat
计算满足不同座位需求的最大可行覆盖,确保同一实体不能同时覆盖多个槽位但不丢弃不冲突的席位覆盖;保留互不相交分量可累加的行为,避免低估覆盖量或重复创建匿名实体。

In `@src/tracker/roomMovement/sources.ts`:
- Around line 73-74: Update the source spell ID handling near
getCompatibleMarkSpellIDs so every value in the spellIDs array is preserved and
evaluated for compatibility, rather than only spellIDs[0]. Keep scalar spellIDs
supported, and ensure markSpellIDs includes all compatible IDs so existing mark
entities are selected before fallback handling.
🪄 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: 9f2ad573-0d86-45e3-a4b8-95ad72e0ef86

📥 Commits

Reviewing files that changed from the base of the PR and between 6622bfc and 29129dd.

📒 Files selected for processing (10)
  • docs/agents/card_tracker.md
  • src/tracker/Room.ts
  • src/tracker/candidate/publicCandidate.ts
  • src/tracker/roomConstraints.ts
  • src/tracker/roomMovement.ts
  • src/tracker/roomMovement/sources.ts
  • tests/tracker/ambiguousKnownIndexIncremental.test.ts
  • tests/tracker/handCountObservation.test.ts
  • tests/tracker/publicCandidates.test.ts
  • tests/tracker/trackerController.test.ts

Comment thread src/tracker/roomConstraints.ts
Comment thread src/tracker/roomMovement/sources.ts Outdated
@llsccm
llsccm marked this pull request as ready for review July 17, 2026 10:29
@llsccm

llsccm commented Jul 17, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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 (2)
src/tracker/roomMovement/sources.ts (1)

369-392: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

不要撤销迁移后的约束组身份。

Line 370 的 replaceCardInConstraintGroups() 已将 placeholder.combinationID 同步为命中的组 ID,但 Line 392 随即无条件恢复 oldCombinationID,会让实体的组合身份与实际组成员关系不一致。仅在没有迁移任何约束组时恢复旧值。

建议修改
+    let migratedConstraintIdentity = false
     if (preserveAmbiguousIdentity) {
-      this.room.constraints.replaceCardInConstraintGroups(card, placeholder)
+      migratedConstraintIdentity =
+        this.room.constraints.replaceCardInConstraintGroups(card, placeholder)
     }
...
-      placeholder.combinationID = oldCombinationID
+      if (!migratedConstraintIdentity) {
+        placeholder.combinationID = oldCombinationID
+      }
🤖 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/roomMovement/sources.ts` around lines 369 - 392, Update the
placeholder restoration logic in swapKnownCardWithPlayerSourcePlaceholder so
oldCombinationID is restored only when no constraint groups were migrated.
Preserve the combination ID assigned by replaceCardInConstraintGroups when
preserveAmbiguousIdentity performs a migration, while retaining the existing
old-value restoration behavior otherwise.
src/tracker/roomConstraints.ts (1)

288-293: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

合并同一席位的候选实体,不要覆盖先前集合。

Line 288 会累加同席位多个 hand location key 的覆盖量,但 Line 292 仅保留最后一个 key 的实体。若这些 key 的 spellID 不同且候选不重叠,匹配会错误少算覆盖量。

建议修改
-        hiddenCardsBySeat.set(handCandidate.seatID, new Set(ambiguousHiddenCards))
+        const seatHiddenCards =
+          hiddenCardsBySeat.get(handCandidate.seatID) ?? new Set<Card>()
+        ambiguousHiddenCards.forEach((card) => seatHiddenCards.add(card))
+        hiddenCardsBySeat.set(handCandidate.seatID, seatHiddenCards)
         ambiguousHiddenCards.forEach((card) => hiddenCards.add(card))
🤖 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/roomConstraints.ts` around lines 288 - 293, 在处理 handCandidate
的席位聚合逻辑中,更新 hiddenCardsBySeat 时合并该席位已有集合与 ambiguousHiddenCards,而不是用新集合覆盖旧集合;保留
coverageBySeat 的累加行为,并确保合并后的集合继续用于后续匹配。
🤖 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 `@src/tracker/roomConstraints.ts`:
- Around line 288-293: 在处理 handCandidate 的席位聚合逻辑中,更新 hiddenCardsBySeat
时合并该席位已有集合与 ambiguousHiddenCards,而不是用新集合覆盖旧集合;保留 coverageBySeat
的累加行为,并确保合并后的集合继续用于后续匹配。

In `@src/tracker/roomMovement/sources.ts`:
- Around line 369-392: Update the placeholder restoration logic in
swapKnownCardWithPlayerSourcePlaceholder so oldCombinationID is restored only
when no constraint groups were migrated. Preserve the combination ID assigned by
replaceCardInConstraintGroups when preserveAmbiguousIdentity performs a
migration, while retaining the existing old-value restoration behavior
otherwise.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 222d34ba-de92-4fdd-ae06-ad9ecbe2c59d

📥 Commits

Reviewing files that changed from the base of the PR and between 29129dd and cec707b.

📒 Files selected for processing (4)
  • src/tracker/roomConstraints.ts
  • src/tracker/roomMovement/sources.ts
  • tests/tracker/handCountObservation.test.ts
  • tests/tracker/hiddenMarkCandidates.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/tracker/handCountObservation.test.ts

@llsccm
llsccm merged commit 80724e9 into dev Jul 17, 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