Skip to content

[18India] Fix connection bonus fires on visited but unpaid cities#12712

Open
ddaybell wants to merge 1 commit into
tobymao:masterfrom
ddaybell:18india-fix-connection-bonus
Open

[18India] Fix connection bonus fires on visited but unpaid cities#12712
ddaybell wants to merge 1 commit into
tobymao:masterfrom
ddaybell:18india-fix-connection-bonus

Conversation

@ddaybell

@ddaybell ddaybell commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • connection_bonus was using route.visited_stops instead of the stops (paid stops) argument
  • This caused the R+ route bonus to fire when both endpoint cities of a bonus pair were merely visited rather than counted among the train's paid cities
  • A 4E train visiting Karachi and Chennai but paying for 4 other cities would incorrectly collect the R+80 bonus; now both cities must be among the paid stops

Fixes #11512

This will very likely require pinning games, as it changes the amounts awarded for many E train runs.

Test plan

  • 18India fixtures: 43/43 pass
  • Manual: run a 4E train that visits but does not pay for both cities of a bonus pair; confirm R+ does not appear
  • Manual: run a 4E that pays for both cities of a bonus pair; confirm R+ still appears

… trains)

connection_bonus was using route.visited_stops instead of the paid stops
argument, causing the R+ route bonus to fire when both endpoint cities were
merely visited rather than counted among the train's paid cities. A 4E train
visiting Karachi and Chennai but only paying for 4 other cities would
incorrectly receive the R+80 bonus.

Fixes tobymao#11512

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.

PR Review: #12712 - [18India] Fix connection bonus fires on visited but unpaid cities

Reviewed with AI assistance by Mistral Vibe


✅ Strengths

  • Fixes documented bug (#11512)
  • Minimal, surgical change (2 lines)
  • Correct root cause: was using route.visited_stops instead of paid stops
  • All 18India fixtures pass (43/43)

💡 Strong Recommendation

Since 18India has existing fixtures, please add a test case for this fix. State-based fixture preferred, but a unit test for the connection_bonus method is also acceptable.


✅ Verdict: Approve

The fix is correct. Adding a test is strongly recommended but not blocking.

def connection_bonus(route, _stops)
visited_location_names = route.visited_stops.map { |stop| stop.tile.location_name }.compact
def connection_bonus(_route, stops)
visited_location_names = stops.map { |stop| stop.tile.location_name }.compact

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.

Strongly recommend: Add state-based fixture test

Since 18India has existing fixtures, please add a test case that:

  1. Runs a 4E train that visits but does not pay for both cities of a bonus pair
  2. Confirms R+ does not appear
  3. Runs a 4E that pays for both cities
  4. Confirms R+ appears

Alternative: If setting up the game state is complex, a unit test for connection_bonus method verifying it uses the stops parameter (not route.visited_stops) would also work.

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

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] Route Bonus given even if Cities are not counted with 4E train

2 participants