Skip to content

[18India] Fix GIPR bond conversion: manager and float#12695

Open
ddaybell wants to merge 1 commit into
tobymao:masterfrom
ddaybell:18india-fix-gipr-bond
Open

[18India] Fix GIPR bond conversion: manager and float#12695
ddaybell wants to merge 1 commit into
tobymao:masterfrom
ddaybell:18india-fix-gipr-bond

Conversation

@ddaybell

@ddaybell ddaybell commented May 27, 2026

Copy link
Copy Markdown
Contributor

Bond shares in GIPR are created outside the normal IPO system, which caused two bugs when players converted Railroad Bonds.

#11289 - Crash on bond conversion if nobody was GIPR manager yet (undefined method 'percent_of' for nil):
convert_bond_to_gipr called check_manager_change, which accessed gipr.owner.percent_of(...). If no normal GIPR share had been purchased yet, gipr.owner is nil and the game crashes. Fixed by calling make_manager on the converting player when GIPR has no manager yet, mirroring what normal share purchases already do.

#11242 - GIPR doesn't float when exchanging bonds:
Bond shares bypass the IPO, so @ipo_owner.percent_of(self) never decreases from bond conversions and the standard floated? check never fires. Fixed with a new check_gipr_float method in game.rb that sums share_holders directly - capturing both normal IPO sales and bond conversions - and explicitly sets floated and calls float_corporation when the 30% threshold is reached. The percent_to_float display on the charter was also incorrect for the same reason; fixed with an override in Corporation using the same share_holders approach.

Home token placement after a bond-triggered float is handled in process_choose in the step, following the same pattern as normal share purchases.

Closes #11289, #11242.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@perwestling

perwestling commented May 31, 2026

Copy link
Copy Markdown
Collaborator

There is a template for PRs. Should you not be using that for your PRs?

Also, decide if [18India] or [18 India] should be used in the titles.

You can also label all 18 India PRs with the 18 India label. If they also requires pin or archive, those are good labels as well to use. There are other labels that could possibly also be used.

@ollybh ollybh added the 18India label Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[18 India] can't convert bonds to shares of the GIPR

3 participants