Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Configure minimum share trading years #7780

Merged
merged 3 commits into from Oct 19, 2019

Conversation

luludotdev
Copy link
Contributor

Currently a company must be at least 6 years old to be able to trade shares. This is a hardcoded value.
This pull request changes that hardcoded value to a configurable uint8. It is set as an expert setting and defaults to 6.

@glx22
Copy link
Contributor

glx22 commented Oct 18, 2019

Many issues with the checks.
First the commit message must follow the coding style.
Then the regression fails. It's because you add a new setting without bumping the savegame version. Basically all savegames created before your change won't load, and I'm surprised the intro game loaded fine.

Edit: the intro game is a very old version from before the PATS chunk (the one containing settings), that explains why it loads fine.

Copy link
Contributor

@glx22 glx22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok for me.

@James103
Copy link
Contributor

Setting min share trading years = 0 and buying out a company as it starts should work, but when the date cheat is used to set the game date before the introduction date of a company, the company may still be considered protected. Could you make it so setting "min_years_for_shares" = 0 also allow shares to be traded for companies whose inauguration date is in the in-game future (as a result of the date cheat)?

@LordAro
Copy link
Member

LordAro commented Oct 19, 2019

I don't see that as being an issue - if you cheat, there are plenty of "problems" that might arise. The inability to buy out a company is not something I'm concerned about

@LordAro LordAro merged commit f159d91 into OpenTTD:master Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants