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.

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

[1822Africa] Fix share movement on issue #9468

Merged
merged 1 commit into from
Aug 26, 2023

Conversation

vandamm
Copy link
Collaborator

@vandamm vandamm commented Aug 25, 2023

Before clicking "Create"

  • Branch is derived from the latest master
  • Add the pins 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

When issuing shares stock price only moved left one step regardless of the number of shares issues.
Correct behavior is to move one step per share issues.

This change may break existing games if they had share issuing

@scottredracecar
Copy link
Collaborator

@michaeljb this is how it works in 1822CA too in case you have not modified this from base 22. It's correctly done in 1822MX already.
#9376

Rule from MX is (should be same as CA):
A major company may either issue or redeem its own stock, but not both in one turn. Issued stock is transferred from the company
treasury to the bank pool. Issuing stock causes the company’s price to move one step left per 10% share issued. The company
receives from the bank the final resting price, after dropping the price for each 10% share sold. Issuing stock must not cause more
than 50% of the stock to be in the bank pool. If the token moves to a space on the stock market which is already occupied, it is placed
at the bottom of the pile; if the token was already on the left-most space of the stock market, it keeps its relative position in a pile.

michaeljb added a commit to michaeljb/18xx.games that referenced this pull request Aug 26, 2023
@michaeljb
Copy link
Collaborator

Thanks for the tip @scottredracecar! Fixed for CA in #9472.

@michaeljb
Copy link
Collaborator

Now 1822CA, 1822MX, 1822PNW, and 1822Africa can all have this same class duplicated...this is certainly one area making a strong case for me to pursue my idea of creating a new abstract game class, G1822Base, which all the 1822 games could inherit from so that the derivative games could stop overriding things that are specific to 1822 Great Britain.

Actually that abstract game class pattern would be good to apply to 1846 as well, would likely help smooth some things over when it comes time to do 1833NE.

@scottredracecar
Copy link
Collaborator

scottredracecar commented Aug 26, 2023 via email

@michaeljb michaeljb merged commit ded46ef into tobymao:master Aug 26, 2023
@vandamm vandamm deleted the 1822-africa-fix-issue-shares branch August 26, 2023 20:59
ollybh pushed a commit to ollybh/18xx that referenced this pull request Sep 4, 2023
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.

3 participants