18usa pullman scrapping #7099
18usa pullman scrapping #7099
Conversation
ventusignis
force-pushed
the
18usa-pullman-scrapping
branch
from
cfdd92c to
98f33f1
Compare
assets/app/view/game/scrap_trains.rb
Outdated
| h(:h3, 'Trains to Scrap'), | ||
| h(:div, div_props, scrap_trains(scrappable_trains)), | ||
| ]) | ||
| if @game.use_compact_scrap_trains_view |
There was a problem hiding this comment.
This choice may vary from step to step. For example, buy_train may want to show the entire scrap train display. Can you add the method to the step instead?
I also recommend calling it something like scrap_train_button_only?.
There was a problem hiding this comment.
It's now called from the step, I've added the method to the steps in the games that use it instead of the base step
| actions | ||
| end | ||
|
|
||
| def can_scrap_train?(entity) |
There was a problem hiding this comment.
Maybe use a module, so you don't have to duplicate these methods between here and the scrap_train step?
There was a problem hiding this comment.
Since can_scrap_train? is the same between buy_sell_par_shares and scrap_train I pulled it into the scrap_train_module but process_scrap_train is slightly different between the two (one calls scrap_train_by_owner, the other calls scrap_train_by_corporation)
ventusignis
force-pushed
the
18usa-pullman-scrapping
branch
5 times, most recently
from
b546bf5 to
a2ac460
Compare
| @@ -35,7 +35,9 @@ def render | |||
| h(:div, trains), | |||
| ]) | |||
| end | |||
|
|
|||
| overflow << h(ScrapTrains, corporation: @current_entity) if @game.round.actions_for( | |||
| @game.round.active_step&.current_entity | |||
There was a problem hiding this comment.
active_step is assigned to step above. Can you use that instead of triggering active_step again?
There was a problem hiding this comment.
I've updated this block
assets/app/view/game/scrap_trains.rb
Outdated
| end | ||
|
|
||
| def render_button(scrappable_trains) | ||
| step = @game.round.step_for(@corporation, 'scrap_train') |
There was a problem hiding this comment.
This section is a duplicate of line 53 and on. Can you break out into a method both call?
There was a problem hiding this comment.
I was able to do it
assets/app/view/game/scrap_trains.rb
Outdated
|
|
||
| scrappable_trains = step.scrappable_trains(@corporation) | ||
| return nil if scrappable_trains.empty? | ||
|
|
||
| if step.scrap_trains_button_only? |
There was a problem hiding this comment.
Can you use step.respond_to?(:scrap_trains_button_only?) && step.scrap_trains_button_only?, so the other games aren't required to implement the method?
Then you can remove the method from all the other games and not require future games to implement it either.
There was a problem hiding this comment.
I've changed this and the related files
| @@ -47,6 +47,10 @@ def process_scrap_train(action) | |||
| @game.scrap_train(action.train, action.entity) | |||
| end | |||
|
|
|||
| def scrap_trains_button_only? | |||
There was a problem hiding this comment.
Remove from here and other games as per the comment in the scrap_trains view file
| @game.bank.spend(pullman_scrap_value, train.owner) | ||
| @game.log << "#{train.owner.name} scraps a pullman for #{@game.format_currency(pullman_scrap_value)}" | ||
| @game.depot.reclaim_train(train) | ||
| @game.reset_crowded_corps |
There was a problem hiding this comment.
What's the purpose of this action? In the discard_train step, the reclaim_train action is all that process_discard_train does. Why is this different?
There was a problem hiding this comment.
I did some testing and digging and depot.reclaim_train() invokes @game.remove_train() which clears the @crowded_corps cache so this method is unnecessary so I'm removing it.
I added it originally so that the "have to discard one of two pullmans" case doesn't prevent the subsequent "still over train limit" scenario from triggering
| end | ||
|
|
||
| def can_scrap_train?(entity) | ||
| return false unless entity |
There was a problem hiding this comment.
a nice trick here is return false unless entity&.corporation? It will handle both cases.
ventusignis
force-pushed
the
18usa-pullman-scrapping
branch
2 times, most recently
from
cd5e07a to
74d1ce8
Compare
…o allow scrapping pullmans in 18USA stock rounds
…mple to afford tokens)
…Acquisition round
… so that a corporation can scrap a train whenver they operate in the operating round
…t annoying during operating rounds
ventusignis
force-pushed
the
18usa-pullman-scrapping
branch
from
74d1ce8 to
aad8f2c
Compare
18USA Pullmans:
1:
2:
3: Player 2 is acting, so ATSF cannot scrap the pullman
4:
5: Over train limit with 2 pullmans, may only discard pullmans
6: More train limit discard/scrapping
7: Can scrap a pullman as a corporate SR action
8: