[Patch] One Airport per Player - FS#5968

[jogi]

Hi Everybody

I wrote a patch which should be an answer for this feature request: https://bugs.openttd.org/task/5968

The current status is that it works in singleplayer, also with multiple AIs and I have checked even in a local Multiplayer game with multiple instances of openttd running. However, there is one issue left, which is why I haven't uploaded it to flyspray yet.

To understand the issue, I will first explain what I had in mind:

The Plan is to replace the Allow town controlled noise level for airports setting by a new setting, which lets you chose between the original behaviour(2 Airports per town), noise controlled behaviour and the new 1 Airport per player per town setting. Everything works fine, until I remove the old setting. Then, old savegames may crash in the pathfinding routine for trains (I didn't encounter this in savegames created by this version, it only affects old saves). Note that the old setting is never accessed anymore, all checks for the noise control setting have been updated to the new setting.

So, the solution I have implemented does avoid these crashes by leaving the setting in the game and just ignoring it. However, that bugs me. I don't like to introduce dead code.

In the crashes.zip file I have the patch version, which has the setting removed and a few savegames where it happens, including the crash-logs. They work fine, if they are used with the first patch. Also, the crash can be avoided by stopping all trains or switching the Pathfinder-Setting to NPF. So basically, there is a crash of which I'm not sure how i have caused it and which has seemingly nothing to do with my changes. Does anybody have an idea how I could fix it? (Savegames which where created with the patched version did not crash on me...)

About the savefiles:
Puntborough Transport, Nov 3rd, 1956.sav crashes when the train in Lunfingburg Mines is Full and tries to leave (around 11th November).
Puntborough Transport, Aug 14th, 1954.sav crashes when train 5 tries to leave Lunfingburg Train Depot
Fort Redingwell Ridge Transport, 31st Jan 2020.sav crashes immediatly because there are much more trains

All these saves work fine in the patch version where the setting is still present, and crash at the same line.

Edit: Forgot to mention that the patches were tested with r27137

[planetmaker]

Hi,

I wrote a patch which should be an answer for this feature request: https://bugs.openttd.org/task/5968

The current status is that it works in singleplayer, also with multiple AIs and I have checked even in a local Multiplayer game with multiple instances of openttd running. However, there is one issue left, which is why I haven't uploaded it to flyspray yet.

The Plan is to replace the Allow town controlled noise level for airports setting by a new setting, which lets you chose between the original behaviour(2 Airports per town), noise controlled behaviour and the new 1 Airport per player per town setting. Everything works fine, until I remove the old setting. Then, old savegames may crash in the pathfinding routine for trains (I didn't encounter this in savegames created by this version, it only affects old saves). Note that the old setting is never accessed anymore, all checks for the noise control setting have been updated to the new setting.

So, the solution I have implemented does avoid these crashes by leaving the setting in the game and just ignoring it. However, that bugs me. I don't like to introduce dead code.

you are missing two things:
* The conversion code of savegames between version 96 and 195, converting the existing setting to your new one in afterload.cpp
* In settings.ini the old setting needs also indication that it is being discontinued. Thus yes, the space needs to remain reserved in the variables struct, marked as void (see settings.ini:836 for instance, the old mammoth trains setting)

I also might miss some other things - it's a bit ago that I added or retired a setting; but you surely find such commits in the history which will teach you the requirements

Fixing these things might also be reason for the crash you see - though I didn't look at that (yet).

[jogi]

Thank you, with your input I was able to fix the Crashes.

However, about the conversion of the savegame:

In versions prior to 96, it is easy, because the Noise-Setting didn't exist then. In order be consistent with the old save, I only have to set mey new setting to "Original". In the versions betwen 96 and 194 there are two possibilities and the setting would be gone. So I don't really see a method to set it to the right value (either Original or Noise).

Also, as far as I understand the comment in afterload.cpp:700, it seems that this is not possible. However, I'm not sure about this, because don't get the complete meaning of this comment (Not a native english speaker)

Code: Select all

	/*
	 * Force the old behaviour for compatibility reasons with old savegames. As new
	 * settings can only be loaded from new savegames loading old savegames with new
	 * versions of OpenTTD will normally initialize settings newer than the savegame
	 * version with "new game" defaults which the player can define to their liking.
	 * For some settings we override that to keep the behaviour the same as when the
	 * game was saved.
	 *
	 * Note that there is no non-stop in here. This is because the setting could have
	 * either value in TTDPatch. To convert it properly the user has to make sure the
	 * right value has been chosen in the settings. Otherwise we will be converting
	 * it incorrectly in half of the times without a means to correct that.
	 */

[Eddi]

Also, as far as I understand the comment in afterload.cpp:700, it seems that this is not possible. However, I'm not sure about this, because don't get the complete meaning of this comment (Not a native english speaker)

you are probably misinterpreting that. the comment means that OpenTTD cannot guess the status of the non-stop handing variable when converting from TTD(Patch) savegames, which did not store this information.

but you are dealing here with a setting that was stored, but you want to remove it. the way to do that is that you store the value from the old setting into a variable (this can be the same variable as you would keep your new setting), and convert the values in afterloadgame (if necessary)

basically you need to keep the existing entry in settings.ini intact, only set a max version.

[jogi]

Okay, In this case my first try wasn't actually too far off.

So, there is the latest version of the patch, the savegames are converted properly and I haven't encountered a crash with this version.

Thank you for the guidance

jogi