The International Simutrans Forum

 

Author Topic: Cleanup of savegame version checks  (Read 723 times)

0 Members and 1 Guest are viewing this topic.

Offline ceeac

  • *
  • Posts: 55
Cleanup of savegame version checks
« on: May 13, 2019, 09:30:46 AM »
This patch consists of three parts:
  • 01-version-check.patch: Cleans up checking of savegame versions by introducing three functions: loadsave_t::is_version_atleast, loadsave_t::is_version_less and loadsave_t::is_version_equal. The first function can be used when introducing new data to the save game:
Code: [Select]
if (file->is_version_atleast(120, 8)) { /* load/save new data */ } The second function can be used for compatibility with old saves:
Code: [Select]
if (file->is_version_less(120, 8)) { /* code to load/save old saves */ }
    This is IMO more readable and less error-prone than comparing the output of file->get_version() to a raw integer.
    [/li]
  • 02-cleanup-int-version.patch: Cleans up loadsave_t::int_version
  • 03-fix-invalid-version-check.patch: Fixes a bug that caused invalid saves to be potentially recognized as valid saves

Offline ACarlotti

  • *
  • Posts: 483
Re: Cleanup of savegame version checks
« Reply #1 on: May 13, 2019, 12:41:07 PM »
01-version-check.patch: Cleans up checking of savegame versions by introducing three functions: loadsave_t::is_version_atleast, loadsave_t::is_version_less and loadsave_t::is_version_equal.
I have been considering introducing something similar to Extended (where I was thinking of names 'is_extended_version_before' and 'is_extended_version_after' to correspond to your first two functions). In Extended the current situation is more complicated than Standard because we're having to check for two different version numbers in combination (requiring three numerical comparisions) rather than encoding this in a single integer.

Online prissi

  • Developer
  • Administrator
  • *
  • Posts: 9595
  • Languages: De,EN,JP
Re: Cleanup of savegame version checks
« Reply #2 on: May 15, 2019, 12:14:14 AM »
The old binary mode was different from the zipped mode. But such old games will fail anyway.

Why the test for "if (pak_extension_str) {"?

Also, why renaming "get_version" to "get_version_raw"? There is no gain I see, but making more conflict on patches.

Offline ceeac

  • *
  • Posts: 55
Re: Cleanup of savegame version checks
« Reply #3 on: May 21, 2019, 11:54:30 AM »
Why the test for "if (pak_extension_str) {"?
It may be NULL, see e.g. simmain.cc line 631. It is more of a precaution since currently for all calls to int_version where pak_extension_str is NULL, version_text contains a valid version string.

Also, why renaming "get_version" to "get_version_raw"?
My intention was to make it clear that the function returns a raw integer as a version number and not a (hypothetical) version_t structure. But I can change it back if that's desired.

Online prissi

  • Developer
  • Administrator
  • *
  • Posts: 9595
  • Languages: De,EN,JP
Re: Cleanup of savegame version checks
« Reply #4 on: May 23, 2019, 06:31:15 AM »
One could of course define version as an enum and generate warnings when compared to intergers.

But for your puspose, how about get_version_int? This makes much clearer that an integer is returned.

Offline ceeac

  • *
  • Posts: 55
Re: Cleanup of savegame version checks
« Reply #5 on: May 23, 2019, 09:04:39 AM »
That is fine by me. I updated the patch files accordingly.

EDIT: Last patch file was broken, should be fixed now.
« Last Edit: May 23, 2019, 10:52:59 AM by ceeac »

Online prissi

  • Developer
  • Administrator
  • *
  • Posts: 9595
  • Languages: De,EN,JP
Re: Cleanup of savegame version checks
« Reply #6 on: June 19, 2019, 12:15:33 AM »
added in r8770 thanks