18USA: PR cleanup

assets/app/view/game/discard_trains.rb

@@ -7,10 +7,6 @@ module Game
     class DiscardTrains < Snabberb::Component
       include Actionable
 
-      def current_entity_actions
-        @current_entity_actions ||= @game.round.actions_for(@game.round.active_step&.current_entity) || []
-      end
-
       def render
         block_props = {
           style: {
@@ -39,7 +35,8 @@ def render
             h(:div, trains),
           ])
         end
-        overflow << h(ScrapTrains, corporation: @current_entity) if current_entity_actions.include?('scrap_train')
+        overflow << h(ScrapTrains, corporation: @current_entity) if @game.round.actions_for(step.current_entity)
+            &.include?('scrap_train')
         overflow << h(Map, game: @game) if @game.round.is_a?(Engine::Round::Operating)
 
         h(:div, [

assets/app/view/game/scrap_trains.rb

@@ -10,11 +10,32 @@ class ScrapTrains < Snabberb::Component
 
       def render
         @corporation ||= @game.round.active_step.current_entity
-        step = @game.round.active_step
+        step = @game.round.step_for(@corporation, 'scrap_train')
 
         scrappable_trains = step.scrappable_trains(@corporation)
         return nil if scrappable_trains.empty?
 
+        if step.respond_to?(:scrap_trains_button_only?) && step.scrap_trains_button_only?
+          render_button(scrappable_trains)
+        else
+          render_section(scrappable_trains)
+        end
+      end
+
+      def render_button(scrappable_trains)
+        step = @game.round.step_for(@corporation, 'scrap_train')
+        h(:div, scrappable_trains.flat_map do |train|
+          scrap = lambda do
+            process_action(Engine::Action::ScrapTrain.new(
+              @corporation,
+              train: train,
+            ))
+          end
+          h(:button, { on: { click: scrap } }, step.scrap_info(train))
+        end)
+      end
+
+      def render_section(scrappable_trains)
         div_props = {
           style: {
             display: 'grid',
@@ -23,32 +44,24 @@ def render
             alignItems: 'center',
           },
         }
-        if @game.use_compact_scrap_trains_view
-          h(:div, scrap_trains(scrappable_trains))
-        else
-          h(:div,
-            [h(:h3, 'Trains to Scrap'),
-             h(:div, div_props, scrap_trains(scrappable_trains))])
-        end
+        h(:div,
+          [h(:h3, 'Trains to Scrap'),
+           h(:div, div_props, scrap_trains(scrappable_trains))])
       end
 
       def scrap_trains(scrappable_trains)
-        step = @game.round.active_step
+        step = @game.round.step_for(@corporation, 'scrap_train')
         scrappable_trains.flat_map do |train|
           scrap = lambda do
             process_action(Engine::Action::ScrapTrain.new(
               @corporation,
               train: train,
             ))
           end
-          if @game.use_compact_scrap_trains_view
-            h(:button, { on: { click: scrap } }, step.scrap_info(train))
-          else
-            [h(:div, train.name),
-             h('div.nowrap', train.owner.name),
-             h('div.right', step.scrap_info(train)),
-             h('button.no_margin', { on: { click: scrap } }, step.scrap_button_text(train))]
-          end
+          [h(:div, train.name),
+           h('div.nowrap', train.owner.name),
+           h('div.right', step.scrap_info(train)),
+           h('button.no_margin', { on: { click: scrap } }, step.scrap_button_text(train))]
         end
       end
     end

assets/app/view/game_page.rb

@@ -419,7 +419,7 @@ def render_action
         h(Game::Round::Auction, game: @game, user: @user)
       when Engine::Round::Merger
         if !(%w[buy_train scrap_train reassign_trains] & current_entity_actions).empty? &&
-              @game.use_1840_style_merger_round_scrap_trains?
+              @game.train_actions_always_use_operating_round_view?
           h(Game::Round::Operating, game: @game)
         else
           h(Game::Round::Merger, game: @game)

lib/engine/game/base.rb

@@ -2778,13 +2778,13 @@ def show_map_legend?
 
       # This is a bit of a hack to get around how 1840 and 18USA have different and incompatible opinions on what the
       #  option to use the scrap_trains action in a merger round means
-      def use_1840_style_merger_round_scrap_trains?
-        true
-      end
-
-      # If true, instead of doing a relatively big view with a header and corporation context for scrapping trains, only a
-      # button [{scrap}] will be displayed, where {scrap} is taken from the current step's scrap_info(train) method
-      def use_compact_scrap_trains_view
+      # If a game overrides this to true, then if the possible actions for the current entity include any of:
+      # * buy_train
+      # * scrap_train
+      # * reassign_train
+      # then the Operating view will be used instead of the Merger round view in a merger round.
+      # See https://github.com/tobymao/18xx/issues/7169
+      def train_actions_always_use_operating_round_view?
         false
       end
     end

lib/engine/game/g_18_usa/game.rb

@@ -652,7 +652,7 @@ def operating_round(round_num)
             G1817::Step::CashCrisis,
             G18USA::Step::ObsoleteTrain,
             G18USA::Step::Loan,
-            G18USA::Step::ScrapTrain,
+            G18USA::Step::DiscardTrain,
             G18USA::Step::SpecialTrack,
             G18USA::Step::SpecialToken,
             G18USA::Step::SpecialBuyTrain,
@@ -663,65 +663,12 @@ def operating_round(round_num)
             G18USA::Step::BuyPullman,
             G18USA::Step::Route,
             G18USA::Step::Dividend,
-            G18USA::Step::DiscardTrain,
             G18USA::Step::BuyTrain,
           ], round_num: round_num)
         end
 
-        def pullman_scrap_value
-          50
-        end
-
-        def scrap_info
-          "Scrap Pullman for #{format_currency(pullman_scrap_value)}"
-        end
-
-        def scrap_button_text
-          'Scrap Pullman'
-        end
-
-        def use_compact_scrap_trains_view
-          true
-        end
-
-        def use_1840_style_merger_round_scrap_trains?
-          false
-        end
-
         def crowded_corps
-          @crowded_corps ||= corporations.select do |c|
-            trains = self.class::OBSOLETE_TRAINS_COUNT_FOR_LIMIT ? c.trains.size : c.trains.count { |t| !t.obsolete }
-            trains > train_limit(c) || c.trains.count { |t| pullman_train?(t) } > 1
-          end
-        end
-
-        # owner is the alleged corporation scrapping a pullman
-        def scrap_train_by_corporation(action, _owner)
-          entity = action.entity
-          raise GameError, "#{entity.name} cannot scrap a train now" unless entity == current_entity
-
-          train = action.train
-          raise GameError, "#{entity.name} cannot scrap a #{train.name} train" unless pullman_train?(train)
-
-          scrap_train(train)
-        end
-
-        # owner is the alleged owner of the company scrapping a pullman
-        def scrap_train_by_owner(action, _owner)
-          entity = action.entity
-          raise GameError, "#{entity.name} cannot scrap a train now" unless entity&.owner == current_entity
-
-          train = action.train
-          raise GameError, "#{entity.name} cannot scrap a #{train.name} train" unless pullman_train?(train)
-
-          scrap_train(train)
-        end
-
-        # Do error checking before calling this.
-        def scrap_train(train)
-          @bank.spend(pullman_scrap_value, train.owner)
-          @log << "#{train.owner.name} scraps a pullman for #{format_currency(pullman_scrap_value)}"
-          @depot.reclaim_train(train)
+          @crowded_corps ||= super | corporations.select { |c| c.trains.count { |t| pullman_train?(t) } > 1 }
         end
 
         def next_round!
@@ -734,11 +681,6 @@ def next_round!
               reorder_players
               new_operating_round
             when Engine::Round::Operating
-              # The normal export logic which plays nicely with privates exists in first_turn_housekeeping but it does not
-              #  work in the edgecase where there are 0 entities acting in an operating round - this exists to cover this
-              #  edge case. Since there are no corporatiosn, there is nobody who could use a private to save a train from
-              #  rusting prematurely so this is fine.
-              export_train if @round.entities.empty?
               # Store the share price of each corp to determine if they can be acted upon in the AR
               @stock_prices_start_merger = @corporations.to_h { |corp| [corp, corp.share_price] }
               @log << "-- #{round_description('Merger and Conversion', @round.round_num)} --"

lib/engine/game/g_18_usa/round/operating.rb

@@ -22,6 +22,14 @@ def finished?
 
             true
           end
+
+          def pay_interest!(entity)
+            # 1817's pay_interest! does a 'return unless step_passed?(Engine::Step::BuyTrain)' which unintentionally
+            #   passes for the 18USA BuyPullmanStep - here we check that 18USA BuyTrain is passed before continuing
+            return unless step_passed?(G18USA::Step::BuyTrain)
+
+            super
+          end
         end
       end
     end

lib/engine/game/g_18_usa/step/acquire.rb

@@ -10,47 +10,26 @@ class Acquire < G1817::Step::Acquire
           include ScrapTrainModule
           def actions(entity)
             actions = super
-            if entity.corporation? && entity == @buyer && entity.trains.any? { |t| @game.pullman_train?(t) }
+            if entity == @buyer && can_scrap_train?(entity)
               actions = %w[pass] if actions.empty?
               actions << 'scrap_train'
             end
             actions
           end
 
           def pass_description
-            if @offer
-              'Pass (Offer for Sale)'
-            elsif @auctioning
-              'Pass (Bid)'
-            elsif @buyer && can_take_loan?(@buyer)
-              'Pass (Take Loan)'
-            elsif @buyer && can_payoff?(@buyer)
-              'Pass (On payoff Loan)'
-            elsif @buyer
-              'Pass (Scrap Train)'
-            end
+            return 'Pass (Scrap Train)' if @buyer && !can_take_loan?(@buyer) && !can_payoff?(@buyer)
+
+            super
           end
 
           def process_pass(action)
-            if @offer
-              @game.log << "#{@offer.owner.name} declines to put #{@offer.name} up for sale"
-              @round.offering.delete(@offer)
-              @offer = nil
-              setup_auction
-            elsif @buyer && can_take_loan?(@buyer)
-              @passed_take_loans = true
-              @game.log << "#{@buyer.name} passes taking additional loans"
-              acquire_post_loan
-            elsif @buyer && can_payoff?(@buyer)
-              @passed_payoff_loans = true
-              @game.log << "#{@buyer.name} passes paying off additional loans"
-              acquire_post_loan
-            elsif @buyer
+            if @buyer && !can_take_loan?(@buyer) && !can_payoff?(@buyer)
               @passed_scrap_trains = true
               @game.log << "#{@buyer.name} passes scrapping trains"
               acquire_post_loan
             else
-              pass_auction(action.entity)
+              super
             end
           end
 
@@ -60,14 +39,10 @@ def acquire_post_loan
             super
           end
 
+          # This version is needed to reference @passed_scrap_trains
           def can_scrap_train?(entity)
             return true if entity.corporation? && !@passed_scrap_trains && entity.trains.find { |t| @game.pullman_train?(t) }
           end
-
-          def process_scrap_train(action)
-            @corporate_action = action
-            @game.scrap_train_by_corporation(action, current_entity)
-          end
         end
       end
     end

lib/engine/game/g_18_usa/step/assign.rb

@@ -1,13 +1,12 @@
 # frozen_string_literal: true
 
 require_relative '../../../step/assign'
-require_relative 'scrap_train_module'
+
 module Engine
   module Game
     module G18USA
       module Step
         class Assign < Engine::Step::Assign
-          include ScrapTrainModule
           def process_assign(action)
             company = action.entity
             target = action.target

lib/engine/game/g_18_usa/step/buy_pullman.rb

@@ -1,54 +1,37 @@
 # frozen_string_literal: true
 
-require_relative '../../../step/base'
-require_relative '../../../step/train'
-require_relative 'scrap_train_module'
+require_relative '../../../step/buy_train'
 module Engine
   module Game
     module G18USA
       module Step
-        class BuyPullman < Engine::Step::Base
-          include Engine::Step::Train
-          include ScrapTrainModule # Why would you scrap a pullman so you can buy a pullman!?
-          def actions(entity)
-            return [] if entity != current_entity
-
-            if @game.depot.upcoming.any? { |t| t.name == 'P' } && can_buy_train?(entity) && @game.pullmans_available? && \
-                entity.runnable_trains.none? { |t| @game.pullman_train?(t) }
-              return %w[buy_train pass]
-            end
-
-            []
-          end
-
+        class BuyPullman < Engine::Step::BuyTrain
           def description
-            'Buy Pullman (Early)'
+            'Buy Pullman'
           end
 
           def pass_description
             'Skip (Pullman)'
           end
 
-          def process_buy_train(action)
-            check_spend(action)
-            buy_train_action(action)
-            pass! if !can_buy_train?(action.entity) && pass_if_cannot_buy_train?(action.entity)
+          def must_buy_train?(_)
+            false
           end
 
-          def check_spend(action)
-            return if action.train.price <= buying_power(action.entity)
+          def president_may_contribute?(_)
+            false
+          end
 
-            raise GameError, "#{action.entity.name} may not spend "\
-                             "#{@game.format_currency(action.price)} on "\
-                             "#{action.train.owner.name}'s #{action.train.name} "\
-                             "train; may only spend #{@game.format_currency(buying_power(action.entity))}."
+          def can_buy_train?(entity, _shell = nil)
+            @game.pullmans_available? && entity.runnable_trains.none? { |t| @game.pullman_train?(t) }
           end
 
           def buyable_trains(entity)
             # Can't buy a second pullman and can't buy a pullman if it's not legal to well, buy pullmans.
-            [] if entity.runnable_trains.any? { |t| @game.pullman_train?(t) } || !@game.pullmans_available?
+            return [] unless can_buy_train?(entity)
+
             # Cannot buy a pullman if you have a pullman
-            Array(@game.depot.upcoming.find { |t| @game.pullman_train?(t) })
+            (@depot.depot_trains & super).select { |t| @game.pullman_train?(t) }
           end
         end
       end