FS#3012 - Disable to change NewGRFs in running game by default

Attached to Project: OpenTTD
Opened by FooBar (foobar) - Saturday, 04 July 2009, 14:20 GMT
Last edited by Ingo von Borstel (planetmaker) - Sunday, 07 November 2010, 21:53 GMT
Type Feature Request
Category NewGRF
Status Closed
Assigned To No-one
Operating System All
Severity Medium
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


More and more people are complaining about things not working as expected after changing or removing newgrfs in a running game. It appears that the do-not-readme warning gets less and less sufficient.

In order to limit those 'bug' reports I came up with the idea to disable the possibility to open the NewGRF settings window from a running game by default. Enabling this feature (as it's still useful sometimes if you know what you're doing) should only be possibly by editing openttd.cfg, as people who edit that mostly know what they're doing or at least have read the manual and are aware of what can go wrong...
This task depends upon

Closed by  Ingo von Borstel (planetmaker)
Sunday, 07 November 2010, 21:53 GMT
Reason for closing:  Implemented
Additional comments about closing:  In various versions from r20918 to r21116
Comment by Ingo von Borstel (planetmaker) - Monday, 06 July 2009, 17:59 GMT
Editing the cfg won't do any good as it doesn't impact a running game. But an option in the cheat menu "allow changing newgrf" might be a solution :)
Comment by FooBar (foobar) - Monday, 06 July 2009, 18:35 GMT
Why not? My suggestion is only about hiding the newgrf dialog in a running game, nothing more, nothing less. That needn't be stored in a savegame, a setting under [misc] in openttd.cfg is more than sufficient.
Comment by Remko Bijker (Rubidium) - Saturday, 15 August 2009, 12:06 GMT
You're missing one major cause of changed NewGRFs: loading other people's savegames. Adding a setting to hide the NewGRF settings doesn't fix that at all.
Comment by FooBar (foobar) - Saturday, 15 August 2009, 12:20 GMT
I'm not completely aware of how it's currently handled (I don't load other people's savegames if they require certain newgrfs I know I don't have), but anyways:

If not all newgrfs are available and my suggested feature is disabled (the default) then deny loading the game.
If my feature is enabled, load the game and display the usual warning.
Comment by Ingo von Borstel (planetmaker) - Tuesday, 05 October 2010, 09:30 GMT
The attached patch introduces a new config-file-only setting "gui.scenario_developer". Unless this setting is enabled, one cannot load games anymore where one is missing the exact newgrfs. Also one cannot modify the newgrf config in an existing game. It is supposed to work analoguously to ai_developer, developer and newgrf_developer.

EDIT: the question arises whether loading of savegames and scenarios with compatible NewGRF shall be forbidden by this setting as well (as now) or whether this should be allowed or guarded by yet another config setting - this time maybe by a visible one in the adv. settings?
Comment by Ingo von Borstel (planetmaker) - Tuesday, 05 October 2010, 11:14 GMT
The attached patch applies on top of the previous patch and adds an option in the adv. settings which allows to enable and disable loading of savegames with compatible NewGRFs.

Thinking about it, the visible option to load games with compatible NewGRF seems desirable: if not present, many people might enable the scenario_developer, even if only loading a game where they have not the exact same version... and then they will be able to change newgrfs.
Comment by Ingo von Borstel (planetmaker) - Thursday, 07 October 2010, 16:26 GMT
After IRC discussion, a pre-condition for proper support is action14 support for minimum compatible version. Attached patches provide action14 support for "MINV". The savegames save the current newgrf version and a savegame has missing newgrf, if only newgrf with a higher MINV are found.

They probably can easily be made one commit, though, but provided for better reading as three separate patches.
Comment by Ingo von Borstel (planetmaker) - Thursday, 07 October 2010, 17:17 GMT
Updated version of 'minversion.diff'. The min_loadable_version was not copied when a compatible newgrf was loaded. Thanks for spotting, frosch.
Comment by Ingo von Borstel (planetmaker) - Thursday, 07 October 2010, 17:21 GMT
test newgrfs: contains version 39, no min_loadable_version set (=0)
snowlinemod.grf: version 45, min_loadable_version=37
Comment by Ingo von Borstel (planetmaker) - Thursday, 07 October 2010, 20:14 GMT
Yet another update of the two main patches. Thanks for the quick and extensive feedback.
Comment by Ingo von Borstel (planetmaker) - Thursday, 07 October 2010, 20:22 GMT
Hm... as usual. The obvious typo or omission after posting
Comment by Ingo von Borstel (planetmaker) - Friday, 08 October 2010, 12:03 GMT
Update. Now only shows the newest version in the NewGRF selection from the main menu.

The introduced enum FindGRFConfigMode might (currently) be replaced by a boolean named 'get_newest', but the enum seems more extensible and possibly some subsequent refactoring, unifying the call of FindGRFConfig and putting some more logic in that routine; maybe adding a return parameter which tells whether an exact match, a compatible match, an incompatible match with the same grfID or no match was found.
Comment by frosch (frosch) - Sunday, 17 October 2010, 12:38 GMT
Grfs can now specify compatibility.

Alternative GUIs might still be nice :)
Comment by Ingo von Borstel (planetmaker) - Sunday, 17 October 2010, 15:08 GMT