Skip to content

[System18] Changes requested by designer #11631

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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

philcampeau
Copy link
Collaborator

@philcampeau philcampeau commented Mar 20, 2025

Fixes #10456
Fixes #10459

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

Implementation Notes

Explanation of Change

Requested changes from designer:

  • For both president sales and EMR issuing shares, the price moves down one space per block of shares sold/issued.
  • Also, share issuing is done automatically in one block. It should not be allowed to sell shares in multiple blocks.

I made these changes. Additionally, I noted that there were two separate variables tracking if shared had been emergency issued: @emergency_issued and @emr_issue. I consolidated these together as @emergency_issued, as they didn't appear to be tracking different information.

I also removed this code below, since @emergency_issued is already being reset in the def setup method of that same step.

def process_buy_train(action)
super
@emr_issue = false
end

Screenshots

Any Assumptions / Hacks

@philcampeau philcampeau added the archive_alpha_games Needs alpha games archiving label Mar 20, 2025
@ollybh ollybh added the System18 label Apr 3, 2025
@ollybh ollybh requested a review from roseundy April 3, 2025 21:30
@@ -15,7 +15,7 @@ module Meta
GAME_LOCATION = 'Various'
GAME_PUBLISHER = :all_aboard_games
GAME_INFO_URL = 'https://github.com/tobymao/18xx/wiki/System18'
GAME_RULES_URL = 'https://github.com/tobymao/18xx/wiki/System18'
GAME_RULES_URL = 'https://www.dropbox.com/scl/fi/or38t89eczc5eki8e21y7/System18_Rules.pdf'
Copy link
Collaborator

Choose a reason for hiding this comment

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

The link needs to include the rkley parameter (from #10459). That lets you access the file without being signed into Dropbox.

Suggested change
GAME_RULES_URL = 'https://www.dropbox.com/scl/fi/or38t89eczc5eki8e21y7/System18_Rules.pdf'
GAME_RULES_URL = 'https://www.dropbox.com/scl/fi/or38t89eczc5eki8e21y7/System18_Rules.pdf?rlkey=n9kblkl7kvnz96oobs6hsatk2&dl=0'

@@ -14,7 +14,7 @@ def actions(entity)
end

def setup
@emr_issue = false
@emergency_issued = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're referencing both @emergency_issued and @round.emergency_issued in this file. These will be two different variables.

Comment on lines +539 to +549
# Sort bundles by price in ascending order
issuable_bundles.sort_by!(&:price)

# Find the smallest bundle that raises enough cash to buy the train
issuable_bundles.each do |bundle|
total_cash = bundle.price + entity.cash
return [bundle] if total_cash >= min_train_price
end

# If no single bundle is enough, return the largest bundle
[issuable_bundles.last]
Copy link
Collaborator

Choose a reason for hiding this comment

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

issuable_bundles could contain share bundles for more than one corporation. I guess this is for an 1841-style game where corporations can own shares of other corporations, and these could be issued in emergency money raising.

This is going to return a single bundle for one corporation. It should return a single bundle for each corporation. So this bit needs to be moved into the @corporations.flat_map block.

I don't know if any currently implemented rules for System18 allow corporations to own other corporations' shares, but this has been written to allow this, so should be preserved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
archive_alpha_games Needs alpha games archiving System18
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System18 Rules System18 Incremental Cap Rules - Price moves down one space
2 participants