[1824/1858/1866/18Tokaido] Unlimited trains#12722
Open
ollybh wants to merge 6 commits into
Open
Conversation
1848's overridden `init_train_handler` method is not needed if `DEPOT_CLASS` is used to set the custom Depot class.
Commit 287fcba added support for unlimited trains. Part of this involved using the `num_trains` method to calculate how many train objects needed to be initially created for an unlimited rank of train, normally one, but more if there is an event that fires on the nth train being bought. Several games (1824/1858/1866/18Tokaido) already had their own versions of `num_trains`. They are using this method to allow the number of trains to be calculated from optional rules selected, or player counts. These two different uses of `num_trains` don't work together, preventing these games from simply changing the train rank count to `unlimited` in their definitions. This splits the uses, creating a new `train_rank_count` method that is called from `init_train_handler` to determine how many Train objects should be created for each type of train. This also changes the Train initialiser to take `unlimited` as a parameter, instead of assuming that the `trains` hash will contain a `:num` key.
7E, 6M and 5D trains are unlimited in 1858 Iberia and 1858 India. 6E, 5M and 5D trains are unlimited in 1858 Switzerland.
10 trains are unlimited in both base 1824 and the Cisleithania variants. This removes the custom `init_train_handler` method. A custom `num_trains` method that looks up the number of each train type is added and the base `init_train_handler` method can be used.
The 10 and 6E trains are unlimited in the full 1866 game and all the scenarios.
18Tokaido is an 18Chesapeake variant and so diesels are unlimited.
Collaborator
|
I checked the 1824 code and it looked fine. (Although 1824::Depot probably could be refactored a little bit. But as it works I suppose it is fine. It is over 5 years since I created it.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implementation Notes
Adds support for unlimited trains to 1824 (base game and Cisleithania variants), the 1858 family (Iberia, Switzerland and India), 1866 and 18Tokaido.
Explanation of Change
Commit 287fcba added support for unlimited trains. Part of this involved using the
num_trainsmethod to calculate how many train objects needed to be initially created for an unlimited rank of train, normally one, but more if there is an event that fires on the nth train being bought.1824, 1858, 1866 and 18Tokaido already had their own versions of
num_trains. They are using this method to allow the number of trains to be calculated from optional rules selected, or player counts.These two different uses of
num_trainsdon't work together, preventing these games from simply changing the train rank count tounlimitedin their definitions.This splits the uses, creating a new
train_rank_countmethod that is called frominit_train_handlerto determine how many Train objects should be created for each type of train.This also changes the Train initialiser to take
unlimitedas a parameter, instead of assuming that thetrainshash will contain a:numkey.Finally, it changes 1848 to remove that game's custom
init_train_handlermethod. To do this theDEPOT_CLASSconstant is set for that game.Fixes #12719. Works towards fixing #12237.
This supersedes PR #12595 and parts of #12593.
Any Assumptions / Hacks
An alternate approach to adding the
unlimitedparameter to the Train class would be to callnum_trains(or something similar) from the view classes and drop theunlimitedattribute from a train object. That might make more sense (it's the rank of trains that's unlimited, not the individual train) but would have been a bigger change.Before clicking "Create"
masterAdd thepinsorarchive_alpha_gameslabel if this change will break existing gamesdocker compose exec rack rubocop -adocker compose exec rack rake