Skip to content

[18 India] Fix TR home token blocked when Nepal is full (#11379)#12705

Open
ddaybell wants to merge 1 commit into
tobymao:masterfrom
ddaybell:18india-fix-tr-nepal-home-token
Open

[18 India] Fix TR home token blocked when Nepal is full (#11379)#12705
ddaybell wants to merge 1 commit into
tobymao:masterfrom
ddaybell:18india-fix-tr-nepal-home-token

Conversation

@ddaybell

@ddaybell ddaybell commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • When NCR places in one Nepal (M10) city and EIR uses Danish EIC (P5) in the other, TR could not float - both city slots were consumed and TR's home token had nowhere to go
  • Root cause: Danish EIC's cheater flag bypassed TR's tile-level reservation check in tokenable?, taking the last open slot
  • Fix: add G18India::Step::SpecialToken that overrides process_place_token; when a cheater token ability would bypass a tile reservation block, it sets @extra_slot = true on the ability before calling super, routing the token to @extra_tokens instead of consuming the reserved slot
  • TR's tile-level reservation is preserved, so TR's home token placement (via the pending-tokens player-choice mechanism) continues to work normally

Closes #11379

Test plan

  • All 133 existing 18 India fixture tests pass
  • Manually verify a game where Nepal gets filled before TR floats - TR should be able to place its home token in the remaining open city

?? Generated with Claude Code

When NCR places in one Nepal city and EIR uses Danish EIC (P5) in the
other, TR could not float because both city slots were consumed. Danish
EIC's cheater flag bypassed TR's tile-level reservation check in
tokenable?, taking the last open slot.

Add G18India::Step::SpecialToken overriding process_place_token: when
a cheater ability would bypass a tile reservation block, set extra_slot
on the ability before calling super. This routes the token to
@extra_tokens instead of consuming the reserved regular slot, leaving
TR's slot available for normal home-token placement.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
perwestling
perwestling previously approved these changes Jun 14, 2026

@perwestling perwestling left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I looked at this and it looked good to me. But I also made a test and let an LLM review it.

Here is what the LLM (Mistral) thought about the PR:

Code Review: PR #12705

Overall: LGTM ✅

Analysis:

  • Clean, minimal fix (2 files, 28 lines)
  • Correctly identifies the root cause: Danish EIC's cheater flag bypassing TR's reservation
  • Elegant solution using existing @extra_slot mechanism
  • Well-commented code explaining the logic
  • Preserves TR's tile-level reservation

One suggestion:
Consider adding a specific fixture test that reproduces the bug scenario:

  1. NCR places in Nepal city 1
  2. EIR uses Danish EIC (P5) in Nepal city 2
  3. TR floats and places home token successfully

Testing:

  • All 133 existing 18 India fixture tests pass ✅
  • Manual verification recommended for the specific scenario

Approval: Ready to merge 🚀

@perwestling perwestling dismissed their stale review June 15, 2026 19:34

Remove AI assisted approval.

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.

[18 India] TR can't place it's home token, when Nepal is full

2 participants