[18Uruguay] Also add FCE shares to secondary corps

[patrikolesen]

Fixes #11111
Fixes #11074

Fixes #[PUT_ISSUE_NUMBER_HERE]

Before clicking "Create"

• Branch is derived from the latest master
• Add the pins or archive_alpha_games label if this change will break existing games
  This can break existing games
• Code passes linter with docker compose exec rack rubocop -a
• Tests pass cleanly with docker compose exec rack rake

Implementation Notes

Explanation of Change

Screenshots

Any Assumptions / Hacks

[ollybh]

This is looking much better. I've suggested a couple more tweaks to the code, nothing major this time.

[ollybh]

The if/unless modifiers are testing the same thing, so this would be clearer structured using an if/else statement.

Suggested change

	bundle = Engine::ShareBundle.new(@fce.shares.take(9)) if num_shares == 10
	bundle = Engine::ShareBundle.new(@fce.shares.reject(&:president).take(num_shares)) unless num_shares == 10
	bundle =
	if num_shares == 10
	Engine::ShareBundle.new(@fce.shares.take(9))
	else
	Engine::ShareBundle.new(@fce.shares.reject(&:president).take(num_shares))
	end

[ollybh]

You're trying to find how many of the secondary corporations the player is president of, right? This might be simpler:

Suggested change

	from_secondary = @merge_data[:secondary_corps].reduce(0) do |sum, secondary_corps|
	sum + (secondary_corps.president?(holder) ? 1 : 0)
	end
	from_secondary = @merge_data[:secondary_corps].count { |corp| corp.president?(holder) }

[ollybh]

Suggested change

	num_shares = (total_percent / 20).to_i
	odd_share = num_shares * 20 != total_percent
	num_shares, odd_shares = (total_percent / 10).divmod(2)

[ollybh]

Do you need to use reduce here? Would sum be simpler?

Suggested change

	@merge_data[:corp_share_sum] = @merge_data[:holders].reduce(0) do |sum, holder|
	sum + ((affected_shares(holder, @merge_data[:corps]).sum(&:percent) / 20).to_i * 10)
	end
	@merge_data[:corp_share_sum] = @merge_data[:holders].sum do |holder|
	(affected_shares(holder, @merge_data[:corps]).sum(&:percent) / 20).to_i * 10
	end

[ollybh]

@patrikolesen This pull request has been open for a while now. Has there been any progress? Just checking that you haven't forgotten about this one…

[patrikolesen]

Thanks for the ping, I will look at this the comming days