tobymao / 18xx

A platform for playing 18xx games online!
https://18xx.games
Other
276 stars 181 forks source link

OR Step rewrite shows various problems in games #1090

Closed jenf closed 4 years ago

jenf commented 4 years ago

This is an overview of the rewrites we'll need to do when we migrate to the step code

Buy trains and Discard trains

Discard trains waits until the completion of the buy trains it should be immediate https://www.18xx.games/game/314?action=336

Solution: If unexpected action is buy trains, move the discard trains earlier Migration possible before deployment: Yes

1836jr30 Train pass

https://www.18xx.games/game/2851?action=156

1836jr30 wanted a pass where none was needed.

Solution: convert pass into a dummy message

Migration possible before deployment: No

1836jr30 Move Token -> Home Token

Old code did a move token, replaced with place token

Migration possible before deployment: Yes, Ready in JSON

Bankrupcy can violate shares in market limit

https://www.18xx.games/game/314

This violated the 50% in the market rule

While this shows that the UI was used to discard past 50%, the current master allows discarding more.

Solution: pin them (or modify for fixture 314)

johnhawkhaines sells 2 shares KO and receives ¥80 KO's share price changes from ¥40 to ¥30 johnhawkhaines sells 1 share AR and receives ¥60 johnhawkhaines sells 1 share SR and receives ¥55 SR's share price changes from ¥55 to ¥50 johnhawkhaines goes bankrupt and sells remaining shares Game over: scottredracecar (¥1473), Rebus (¥1134), johnhawkhaines (¥320)

Pass sometimes needed when there was no token to lay

https://www.18xx.games/game/2809?action=137

There's no valid token spot.

If next action is run_routes, delete the pass

Pass needed on token action for 18Chesapeake when action is run_routes

"1116": { "id": 1116, "title": "18Chesapeake", "status": "finished", "finished": false, "note":"add pass", "exception": "Step Place a Token cannot process {\"type\"=>\"run_routes\", \"entity\"=>\"LV\", \"entity_type\"=>\"corporation\", \"id\"=>301, \"routes\"=>[{\"train\"=>\"3-3\", \"connections\"=>[[\"J2\", \"J4\"], [\"J4\", \"J6\"]]}, {\"train\"=>\"4-2\", \"connections\"=>[[\"K1\", \"J2\"], [\"K1\", \"K3\"], [\"L2\", \"K3\"]]}]}" }, "1118": { "id": 1118, "title": "18Chesapeake", "status": "finished", "finished": false, "note":"add pass", "exception": "Step Place a Token cannot process {\"type\"=>\"run_routes\", \"entity\"=>\"PRR\", \"entity_type\"=>\"corporation\", \"id\"=>271, \"routes\"=>[{\"train\"=>\"3-1\", \"connections\"=>[[\"I5\", \"I3\", \"H2\", \"G1\", \"F2\"], [\"I5\", \"H6\"]]}, {\"train\"=>\"4-2\", \"connections\"=>[[\"F8\", \"G7\", \"H6\"], [\"E3\", \"F4\", \"G5\", \"H6\"], [\"D2\", \"E3\"]]}]}" },

Pass when no pass was needed for buy companies https://www.18xx.games/game/1941?action=260

Pass was sometimes needed for buy companies when it shouldn't have been

https://www.18xx.games/game/1941?action=260 https://www.18xx.games/game/2482?action=172 https://www.18xx.games/game/2556?action=394

michaeljb commented 4 years ago

Note that 1846 bankruptcy permits breaking the 50% limit. In #1079 I used Round::Operating#sell_shares and that did the trick. Just want to note that and make sure whatever is done to fix that 1889 example isn't so low-level that it's difficult to get around :slightly_smiling_face:

jenf commented 4 years ago

The migration script only fixes when it detects an problem :)

jenf commented 4 years ago

1079 hasn't come along with us, we'll need to reintegrate that.

jenf commented 4 years ago

now migrated