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, 1822CA] match up lanes of different width, simplify France hex #9519

Merged
merged 2 commits into from
Sep 2, 2023

Conversation

michaeljb
Copy link
Collaborator

  • fixes Windsor-Detroit and Hamilton-Buffalo in 1822CA
  • change France in 1822 to always have 2 lanes, now that 1 lane can run into the two; remove code around "upgrading" France

#9376

@michaeljb michaeljb requested a review from roseundy August 31, 2023 23:22
@michaeljb michaeljb force-pushed the lane-width branch 2 times, most recently from b8a339b to b0279d5 Compare August 31, 2023 23:32
@michaeljb michaeljb merged commit 94f930b into tobymao:master Sep 2, 2023
@michaeljb michaeljb deleted the lane-width branch September 2, 2023 00:55
michaeljb added a commit to michaeljb/18xx.games that referenced this pull request Jan 30, 2024
…n tile upgrade

M13 in 1822CA with Toronto works like 1822's M14 and London (it effectively adds
a permanent extra station slot), but London always has 6 cities that normally
have 1 slot each; the Toronto tiles start with 2 separate cities that join in
brown and vary in size (1-4 slots).

The `extra_tokens` and `extra_slot` functionality on `Engine::Part::City` is not
used because the slot needs to persist even if the minor's home token is picked
up and not replaced by a token from the major when it is acquired.

The cheater token fix changes how tokens are moved from the old tile to the new
tile when a tile is upgraded. With the old approach, it was done by iterating
over each city on the old tile, moving tokens city-by-city. The new approach
first gathers all of the tokens, mapped to their new city, sorts them so that
cheater tokens are at the end of the list, and then places the tokens onto the
new city. This ensures that old cheater tokens will be the cheater tokens on the
new city, if the new city doesn't have enough normal slots for all of the
tokens.

M13 is the cheater token in yellow and green, but it was being placed onto the
brown tile before GT's home token, so with tile #T4 (only 2 normal slots), M13's
and M12's tokens were first filled up the normal slots, and then there was no
room for GT's home token. The change ensures that M12 and GT are placed on the
new city before M13 is.

Additionally:

* clean up 1822 dead code, missed in tobymao#9519
* update 1822CA fixtures descriptions (missed doing this when fixture 3 was
  added)
* update constants in other 1822-family games for `MINOR_14_ID` references

Fixes tobymao#10226
michaeljb added a commit to michaeljb/18xx.games that referenced this pull request Jan 31, 2024
…n tile upgrade

[core] `hex.rb` - Refactor moving tokens to new tile to fix bug when multiple
cities on the old tile are being combined into one city on the new tile, and
there was a cheater token in one of the old cities.

[1822/1822CA] Changes the check to add the extra slot for "minor 14"'s home
token to work for more than one slot.

[1822CA] Fixes tobymao#10226, where M13's home in Toronto (1822CA) is a cheater token
but should not take up added token slots when combined with the other city when
a brown T tile is laid.

M14 in 1822 (and therefore M13 in 1822CA) is implemented as a cheater token,
which normally use up a new token slot when one is added to their city. However,
these tokens do not do that, instead adding a permanent extra slot that could be
vacated if the major chooses so when acquiring the minor. They are not
implemented as `extra_tokens` as those created slots cannot persist. On the
physical board, these extra slots are represented on rondels drawn offboard, but
near the London/Toronto hexes. Perhaps further refactoring could be done to
more robustly implement the rondel functionality and distinguish it from cheater
tokens.

Additionally:

* [1822] clean up some dead code missed in tobymao#9519
* [1822CA] update fixtures descriptions (missed doing this when fixture 3 was
  added)
* [1822MX, 1822PNW] update constants in other 1822-family games for
  `MINOR_14_ID` references
* [core] implement `Engine::Token#inspect` for useful debugger output, whether
  the token is on the map or not
* [core] refactor how test fixture files are opened in game_state_spec.rb
    * traverse only the subdirectory relevant for the given title (extracted
      from the string used in the `describe` block)
    * allow fixtures for different titles to have the same game ID (18ZOO and
      1822CA both have a fixture `4.json`)
michaeljb added a commit to michaeljb/18xx.games that referenced this pull request Feb 7, 2024
…n tile upgrade

[core] `hex.rb` - Refactor moving tokens to new tile to fix bug when multiple
cities on the old tile are being combined into one city on the new tile, and
there was a cheater token in one of the old cities.

[1822/1822CA] Changes the check to add the extra slot for "minor 14"'s home
token to work for more than one slot.

[1822CA] Fixes tobymao#10226, where M13's home in Toronto (1822CA) is a cheater token
but should not take up added token slots when combined with the other city when
a brown T tile is laid.

M14 in 1822 (and therefore M13 in 1822CA) is implemented as a cheater token,
which normally use up a new token slot when one is added to their city. However,
these tokens do not do that, instead adding a permanent extra slot that could be
vacated if the major chooses so when acquiring the minor. They are not
implemented as `extra_tokens` as those created slots cannot persist.

On the physical board, these extra slots are represented on rondels drawn
offboard, but near the London/Toronto hexes. Perhaps further refactoring could
be done to more robustly implement the rondel functionality and distinguish it
from cheater tokens.

Additionally:

* [1822] clean up some dead code missed in tobymao#9519
* [1822CA] update fixtures descriptions (missed doing this when fixture 3 was
  added)
* [1822MX, 1822PNW] update constants in other 1822-family games for
  `MINOR_14_ID` references
* [core] implement `Engine::Token#inspect` for useful debugger output, whether
  the token is on the map or not
* [core] refactor how test fixture files are opened in game_state_spec.rb
    * traverse only the subdirectory relevant for the given title (extracted
      from the string used in the `describe` block)
    * allow fixtures for different titles to have the same game ID (18ZOO and
      1822CA both have a fixture `4.json`)
* implement useful `inspect` methods for rounds
michaeljb added a commit to michaeljb/18xx.games that referenced this pull request Feb 7, 2024
…in 1822CA

In 1822, M14 in London is always in a city that only has one normal slot. On the
Toronto hex and tiles in 1822CA, M13's home city can have 1-4 slots. This is
progress on tobymao#10226.

A couple other small miscellaneous fixes:

* clean up some dead 1822 code missed in tobymao#9519
* add note to fixtures README about 1822CA game 3
* make `MINOR_14_ID` references explicit for PNW and MX; remove from CA WRS as
  M13 is not present there (probably worth a refactor to give this a more
  descriptive name at some point, it's weird for the constant named with "14" to
  refer to the minor company "13" in CA)
michaeljb added a commit to michaeljb/18xx.games that referenced this pull request Feb 7, 2024
…in 1822CA

In 1822, M14 in London is always in a city that only has one normal slot. On the
Toronto hex and tiles in 1822CA, M13's home city can have 1-4 slots. This is
progress on tobymao#10226.

A couple other small miscellaneous fixes:

* clean up some dead 1822 code missed in tobymao#9519
* add note to fixtures README about 1822CA game 3
* make `MINOR_14_ID` references explicit for PNW and MX; remove from CA WRS as
  M13 is not present there (probably worth a refactor to give this a more
  descriptive name at some point, it's weird for the constant named with "14" to
  refer to the minor company "13" in CA)
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.

2 participants