[Core] Check for existence of can_finance? method
#11513
Conversation
This file contains 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
The `can_finance?` method was added for 1837 in commit d08cc84. It's used to check whether the bank will step in to pay the cost of a train. This has broken 1877 Venezuela. There is a default implementation of `can_finance?` added to the Step::BuyTrain class, but 1877 Venezuela allows trains to be bought in the BuySellParShares step, which did not have this method. This commit tests for the existence of this method before calling it. Fixes tobymao#11508.
ollybh
added
1877
Core
crericha
reviewed
Feb 5, 2025
| @@ -300,7 +300,7 @@ def render | |||
| children << h(:div, issue_str) | |||
| end | |||
|
|
|||
| if @step.can_finance?(@corporation) | |||
| if @step.respond_to?(:can_finance?) && @step.can_finance?(@corporation) | |||
There was a problem hiding this comment.
I purposely avoided doing needing to do this by adding the method to the buy_train step. Looking at 1877's buy_sell_par_shares step, it copies over all the methods from the buy_train step. I think it should copy over this new method too.
There was a problem hiding this comment.
That would work too. I'd done the respond_to? check as we've already got a zillion tests like this in the UI code.
I've had a quick grep and can't see any other games that have a BuyTrain action in a step that's not inheriting from Step::BuyTrain.
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
Explanation of Change
The
can_finance?method was added for 1837 in commit d08cc84. It's used to check whether the bank will step in to pay the cost of a train.This has broken 1877 Venezuela. There is a default implementation of
can_finance?added to the Step::BuyTrain class, but 1877 Venezuela allows trains to be bought in the BuySellParShares step, which did not have this method.This commit tests for the existence of this method before calling it.
Fixes #11508.
Before clicking "Create"
masterAdd thepinsorarchive_alpha_gameslabel if this change will break existing gamesdocker compose exec rack rubocop -adocker compose exec rack rake