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

[1822 family] allowed to sell fewer shares than needed, leading to taking a loan #10059

Closed
michaeljb opened this issue Dec 30, 2023 · 6 comments
Assignees
Labels
1822Africa 1822CA 1822MX 1822PNW 1822 bug Prevents game from being played correctly rules-permissive Problems where 18xx.games allows things the game rules forbid (fixing these may impact active games)

Comments

@michaeljb
Copy link
Collaborator

michaeljb commented Dec 30, 2023

In this 1822CA gist, Kurt had the option of selling 1 or 2 shares of GTP. After selling only 1 (and selling the other sellable share he held), he was forced to take a loan to pay for the train. He should have instead been forced to sell 2 GTP shares instead of just 1.

To fix this we'd need to add some check looking for the total sellable shares and prevent a loan from happening if that total is enough to buy a train without the loan, or make sure smaller bundles aren't sellable if selling them would lead to a loan.

I suspect this also affects 1822MX, 1822PNW, and 1822Africa--other 1822 games with MUST_SELL_IN_BLOCKS = true. Standard 1822 doesn't have this problem, he would have just been able to sell the 2nd share after selling the first.

update: it's not just a sell in blocks issue


SEO

"need to sell shares before a loan can be granted"
Shares must be sold before a loan is taken
Shares must be sold before loan granted

@michaeljb
Copy link
Collaborator Author

michaeljb commented Jan 24, 2024

The duplicate issues refer to 4 games (plus one archived game):

  • 141719 (1822), started November 19 / last action before the broken one on December 19
  • 142948 (1822), November 29 / December 21
  • 143212 (1822PNW), December 1 / December 14
  • 145577 (1822MRS), December 20 / December 29 (also the date the issue was discovered in then-prealpha 1822CA)

Locally I tried running from the commits that have been turned into pins, all the way back to 189006e from early October, and repeatedly got the same error in all of them. Maybe during December 14-29 it was possible to make the illegal move, but both before and after the time period it was prevented? The dates of the closest pin commits are December 6 and December 30, so possibly a release in between them is responsible.

Best guess at related PRs are #9989 and #10031, will dig into those some more.

@michaeljb
Copy link
Collaborator Author

michaeljb commented Jan 24, 2024

5eb5a09bd, the master commit right before #10031, works for 141719 and 142948, those games are now pinned

@michaeljb
Copy link
Collaborator Author

Found 87 finished broken games in the 1822 family, didn't check all of them, but archived them, assuming they had a similar issue.

@michaeljb
Copy link
Collaborator Author

michaeljb commented Jan 24, 2024

Found one more broken 1822 game, 141115, also pinned that one.

@michaeljb
Copy link
Collaborator Author

michaeljb commented Jan 24, 2024

Pinned 143212 and 145577 to c6da84fcd, the commit before #10022

@michaeljb
Copy link
Collaborator Author

Having run a validation on the prod server, I believe all affected games are fixed. If the issue comes up again, the first thing to try is to pin the affected game to c6da84fcd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1822Africa 1822CA 1822MX 1822PNW 1822 bug Prevents game from being played correctly rules-permissive Problems where 18xx.games allows things the game rules forbid (fixing these may impact active games)
Projects
None yet
Development

No branches or pull requests

1 participant