[18CO] Specific Mine Implementation #2022

ryandriskel · 2020-10-25T18:37:44Z

This change only impacts 18CO files.

Background

From #1836

Placing a tile on a location with a mine places the mine token onto the corporation's charter.
Mines award $10 per mine directly to the treasury after the dividend step as long as a train was run
Idarado Mining Company: Money gained from mines is doubled for the owning Corporation. Closes phase 5 if owned by a player. Closes phase 6 if owned by a corporation.
Mines are removed from the board in phase 6

Results

Mines show up as a corporation ability

Log is written to when a corporation picks up a mine as well as when mine money is awarded

Implementation Notes

Rather than define new Ability classes I am working with the Base class. I don't know any other games that use tokens like this. One downside is the need to build a New object when adding a mine as the attributes are read only.

Piggybacking on change_share_price isn't the most ideal place to do this, but the timing is right and the data needed to determine if mines should pay is available.

tobymao · 2020-10-25T22:39:37Z

i think normally this was done with the assign ability. could that be used instead for consistency?

lib/engine/game/g_18_co.rb

ryandriskel · 2020-10-26T16:30:51Z

i think normally this was done with the assign ability. could that be used instead for consistency?

I've looked through all the games using assign and I'm having a difficult time visualizing how to use the Assign ability here. I guess I should first try to clarify whether you meant the assign_hexes Ability or whether you meant Assignable ?

Where I see assign_hexes Ability being used is when one corporation places tokens on hexes to gain a bonus. This is the exact opposite - all corporations are removing tokens from the map to gain their respective bonuses. Additionally, once the mine is collected, the hex it came from is no longer relevant.

If Mine was its own object, using Assignable on said class would make sense, but I haven't seen a pattern of games defining their own object types.

tobymao · 2020-10-26T22:12:24Z

it's not an ability, it's an action. an entity can assign a target

michaeljb · 2020-10-28T01:56:17Z

lib/engine/step/g_18_co/track.rb

-          # TODO: Implement Mine Collection
-          @log << 'TODO: implement mine token collection'
+          # Mine Token Collection
+          unless action.hex.tile.icons.select { |icon| icon.name == 'mine' }.empty?


Suggested change

unless action.hex.tile.icons.select { |icon| icon.name == 'mine' }.empty?

if action.hex.tile.icons.map(&:name).include?('mine')

michaeljb · 2020-10-28T02:51:31Z

@tobymao @ryandriskel I took a crack at doing this via assignment; it does use the assignment hash a little differently than we have elsewhere in that the value really matters and can be something other than true. Ditching the abilities also has the benefit of not using count_per_or: 2 for something that is really just used once in the OR. In my commit I also check for positive revenue in an override of process_dividend, it seemed more natural to do that and take the revenue from the history.

I did a bit of local testing and this works for at least a couple mines and with the Idarado Mining Company (I didn't keep going up till phase 6 to test closing of the mines).

michaeljb@cab6938

ryandriskel · 2020-10-28T03:46:33Z

Thanks @michaeljb - That does look a lot cleaner. I really appreciate you giving it a shot. I'll load it up on my computer tomorrow and run through everything.

Moving this to Draft for now.

ryandriskel · 2020-10-28T16:15:21Z

I've got the basic code working following Michael's lead. It is cleaner, but I feel this trades misusing Ability count_per_or for misusing Assignment active.

The one thing lost by not using the Abilities section is the number of mines owned by the corp.

Also, the corporation never places a mine on the board, so it might be misleading having the mine show up in the token list.

One possibility I see is to extend the assignment data to allow an object rather than just an active flag. I'm going to play around with that and see if there's a clean solution.

ryandriskel · 2020-10-28T22:31:52Z

lib/engine/game/g_18_co.rb

        setup_corporations
      end

-      def setup_companies


This became unnecessary with @michaeljb 's imc.owner check in mine_multiplier

ryandriskel · 2020-10-28T22:32:47Z

lib/engine/game/g_18_co.rb

+
+        case action
+        when Action::BuyCompany
+          mine_update_text(action.entity) if action.company == imc && action.entity.corporation?


When the IMC is bought in, the text on the Ability needs to be updated to reflect the change in mine value and total.

ryandriskel · 2020-10-28T22:33:27Z

lib/engine/step/g_18_co/dividend.rb

-          # Only pay mines if a train produce revenue regardless of withhold or pay
-          if tfm.positive? && (payout[:corporation].positive? || payout[:per_share].positive?)
-            @game.bank.spend(tfm, entity)
+        def process_dividend(action)


Pulled this from @michaeljb 's suggestions as it is a much more appropriate place to execute the action.

ryandriskel · 2020-10-28T22:35:02Z

lib/engine/step/g_18_co/track.rb

-            @log << "#{action.entity.name} collects a mine token from
-              #{action.hex.name} for a total of
-              #{@game.format_currency(@game.mines_total(action.entity))} from mines"
+            @log << "#{action.entity.name} collects a mine token from #{action.hex.name}"


I shortened the log message to avoid redundancy since the payout is shown on the corporation now

tobymao · 2020-10-29T05:03:10Z

conflict

ryandriskel · 2020-10-29T14:10:14Z

conflict

Rebased

ryandriskel changed the title ~~18CO Specific Mine Implementation~~ [18CO] Specific Mine Implementation Oct 25, 2020

michaeljb reviewed Oct 26, 2020

View reviewed changes

lib/engine/game/g_18_co.rb Outdated Show resolved Hide resolved

michaeljb reviewed Oct 26, 2020

View reviewed changes

lib/engine/game/g_18_co.rb Outdated Show resolved Hide resolved

michaeljb reviewed Oct 28, 2020

View reviewed changes

ryandriskel marked this pull request as draft October 28, 2020 03:46

ryandriskel force-pushed the feature/18CO_Mine_Tokens branch from d54bb86 to 180fa31 Compare October 28, 2020 14:25

ryandriskel force-pushed the feature/18CO_Mine_Tokens branch from 84b58cc to 5393a53 Compare October 28, 2020 22:21

ryandriskel marked this pull request as ready for review October 28, 2020 22:23

ryandriskel force-pushed the feature/18CO_Mine_Tokens branch 2 times, most recently from 9a7ab23 to 66422b9 Compare October 28, 2020 22:30

ryandriskel commented Oct 28, 2020

View reviewed changes

ryandriskel force-pushed the feature/18CO_Mine_Tokens branch 2 times, most recently from 38dd09f to bb2f2c7 Compare October 28, 2020 23:18

tobymao approved these changes Oct 29, 2020

View reviewed changes

Robert Ryan Driskel and others added 2 commits October 29, 2020 08:04

18CO style working mines

9cbf2ff

Combine the best updates from the last day

6f82a12

ryandriskel force-pushed the feature/18CO_Mine_Tokens branch from bb2f2c7 to 6f82a12 Compare October 29, 2020 14:05

tobymao merged commit 45f8936 into tobymao:master Oct 29, 2020

ryandriskel deleted the feature/18CO_Mine_Tokens branch October 31, 2020 21:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[18CO] Specific Mine Implementation #2022

[18CO] Specific Mine Implementation #2022

ryandriskel commented Oct 25, 2020

tobymao commented Oct 25, 2020

ryandriskel commented Oct 26, 2020

tobymao commented Oct 26, 2020

michaeljb Oct 28, 2020

michaeljb commented Oct 28, 2020

ryandriskel commented Oct 28, 2020

ryandriskel commented Oct 28, 2020

ryandriskel Oct 28, 2020

ryandriskel Oct 28, 2020

ryandriskel Oct 28, 2020 •

edited

Loading

ryandriskel Oct 28, 2020

tobymao commented Oct 29, 2020

ryandriskel commented Oct 29, 2020

	unless action.hex.tile.icons.select { \|icon\| icon.name == 'mine' }.empty?
	if action.hex.tile.icons.map(&:name).include?('mine')

[18CO] Specific Mine Implementation #2022

[18CO] Specific Mine Implementation #2022

Conversation

ryandriskel commented Oct 25, 2020

Background

Results

Implementation Notes

tobymao commented Oct 25, 2020

ryandriskel commented Oct 26, 2020

tobymao commented Oct 26, 2020

michaeljb Oct 28, 2020

Choose a reason for hiding this comment

michaeljb commented Oct 28, 2020

ryandriskel commented Oct 28, 2020

ryandriskel commented Oct 28, 2020

ryandriskel Oct 28, 2020

Choose a reason for hiding this comment

ryandriskel Oct 28, 2020

Choose a reason for hiding this comment

ryandriskel Oct 28, 2020 • edited Loading

Choose a reason for hiding this comment

ryandriskel Oct 28, 2020

Choose a reason for hiding this comment

tobymao commented Oct 29, 2020

ryandriskel commented Oct 29, 2020

ryandriskel Oct 28, 2020 •

edited

Loading