Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

System18: Remove discarded trains from the game #11275

Merged
merged 2 commits into from
Oct 24, 2024
Merged

System18: Remove discarded trains from the game #11275

merged 2 commits into from
Oct 24, 2024

Conversation

vandamm
Copy link
Collaborator

@vandamm vandamm commented Oct 12, 2024 •
edited
Loading

Fixes #11264

Before clicking "Create"

  • Branch is derived from the latest master
  • Add the pins or archive_alpha_games label if this change will break existing games
  • Code passes linter with docker compose exec rack rubocop -a
  • Tests pass cleanly with docker compose exec rack rake

May require pins if in any game discarded train was bought

@vandamm vandamm requested a review from roseundy October 12, 2024 21:03
@vandamm vandamm added System18 pins PR that will require some games to be pinned labels Oct 12, 2024
@roseundy
Copy link
Collaborator

Sorry, this is not the correct way to fix this.

Take a look at the variable DISCARDED_TRAINS

roseundy
roseundy requested changes Oct 13, 2024
View reviewed changes
Copy link
Collaborator

@roseundy roseundy left a comment

Choose a reason for hiding this comment

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

Please look at the variable defined in the Game base class: DISCARDED_TRAINS
This just needs to be changed in the map method map_*_constants

@vandamm
Copy link
Collaborator Author

vandamm commented Oct 13, 2024

@roseundy sorry did not find this and found 1822 implementation instead.
Fixed to using DISCARDED_TRAINS = :remove in the base System18 because it's a base rule. Checked all implemented maps and none of them need to override it per the rules

@vandamm vandamm requested a review from roseundy October 13, 2024 11:42
@roseundy roseundy added archive_alpha_games Needs alpha games archiving and removed pins PR that will require some games to be pinned labels Oct 19, 2024
@roseundy
Copy link
Collaborator

We are still archiving System18 games, not pinning them.

@ollybh ollybh merged commit f2c3633 into tobymao:master Oct 24, 2024
1 check passed
@ollybh ollybh mentioned this pull request Oct 24, 2024
Merged
4 tasks
ollybh added a commit to ollybh/18xx that referenced this pull request Oct 24, 2024
@ollybh
[System18] Remove broken fixture file
e1fbe09 
The game 182070 fixture file for System18 includes buying a previously
discarded train at action 258. PR tobymao#11275 has changed it so that
discarded trains are removed from the game, so there is no longer a
train to buy, breaking this fixture.
This was referenced Oct 24, 2024
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roseundy roseundy roseundy approved these changes

Assignees
No one assigned
Labels
archive_alpha_games Needs alpha games archiving System18
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[System18: Poland] Discarded train available for purchase
3 participants
@vandamm @roseundy @ollybh