[18India] Fix yellow tile lay blocked through red city (rule 8.4.1.1)#12711
Open
ddaybell wants to merge 1 commit into
Open
[18India] Fix yellow tile lay blocked through red city (rule 8.4.1.1)#12711ddaybell wants to merge 1 commit into
ddaybell wants to merge 1 commit into
Conversation
Disable the simple-case shortcut in calculate_railhead_hexes that was returning early when the last laid tile had 1-2 direct white neighbors. The shortcut excluded red hex neighbors from empty_neighbors, causing it to miss white hexes reachable through adjacent red cities (e.g. E22/triple-dit via Mumbai D23). The full walk already handles the common 1-2 neighbor case correctly, so the shortcut's benefit was marginal. Fixes tobymao#11780 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
perwestling
reviewed
Jun 6, 2026
| # Performance shortcut: if the last tile has 1-2 direct white neighbors (and is not a | ||
| # triple-town/OO tile), return them immediately without a full walk. DISABLED because | ||
| # it misses white hexes reachable through adjacent red cities (rule 8.4.1.1 — e.g. | ||
| # E22/triple-dit via Mumbai). To restore, add a guard skipping when any exit is red: |
Collaborator
There was a problem hiding this comment.
Cannot the commented away code be removed? Line 27-30 might be enough.
Collaborator
There was a problem hiding this comment.
Suggestion: Replace commented code with SHA reference
Instead of keeping dead code in the file, reference the commit where it existed:
# OPTIMIZATION DISABLED: This performance shortcut was removed in <SHA> because it
# missed white hexes reachable through red cities (rule 8.4.1.1). The full walk
# handles all cases correctly. To restore: add guard for red-hex neighbors and see
# commit <SHA> for the original implementation.Benefits:
- ✅ No commented-out code smell
- ✅ No git grep noise
- ✅ Code still recoverable via
git show <SHA> - ✅ Clearer intent
perwestling
previously approved these changes
Jun 14, 2026
perwestling
left a comment
perwestling
left a comment
Collaborator
There was a problem hiding this comment.
PR Review: #12711 - [18India] Fix yellow tile lay blocked through red city (rule 8.4.1.1)
Reviewed with AI assistance by Mistral Vibe
✅ Strengths
- Fixes documented bug (#11780)
- Minimal, surgical change
- All tests pass (18India: 43/43, Full suite: 7429/7429)
- Clear PR description
💡 Suggestions
- Consider replacing the commented-out optimization with a SHA reference to the original implementation
- Or remove entirely and document in a separate OPTIMIZATIONS.md if you want to keep it for future reference
❓ Questions
- Have you considered removing the shortcut code entirely rather than commenting it out?
✅ Verdict: Approve
The bug fix is correct and ready. The commented-out code is the only concern, but it's not blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
calculate_railhead_hexesthat returned early when the last laid tile had 1-2 direct white neighborsempty_neighbors, causing it to miss white hexes reachable through adjacent red cities (e.g. E22/triple-dit via Mumbai D23 - rule 8.4.1.1)Fixes #11780
Test plan