bugFreeciv - Bugs: bug #18673, Savegame setting loading can...

 
 
Show feedback again

bug #18673: Savegame setting loading can reject valid setting combinations in contrived circumstances

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sat 17 Sep 2011 03:15:25 PM UTC  
 
Category: NoneSeverity: 2 - Minor
Priority: 3 - LowStatus: None
Assigned to: NoneOpen/Closed: Open
Release: Operating System: None
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Tue 08 Nov 2011 09:03:23 PM UTC, SVN revision 20506:

fix map definition / generation

the map is defined with several settings (mapsize, size, tilesperplayer,
xsize, ysize, topology). Now, some combinations are invalid like in this
bug

mapsize == XYSIZE && ysize % 2 != 0 && MAP_IS_ISOMETRIC

I did add the required checks but a framework to check the settings
would be helpful. Check also bug #18673 and patch #2542.

see gna bug #18875

(Browse SVN revision 20506)

Matthias Pfafferodt <syntron>
Project Member
Tue 08 Nov 2011 09:02:50 PM UTC, SVN revision 20505:

fix map definition / generation

the map is defined with several settings (mapsize, size, tilesperplayer,
xsize, ysize, topology). Now, some combinations are invalid like in this
bug

mapsize == XYSIZE && ysize % 2 != 0 && MAP_IS_ISOMETRIC

I did add the required checks but a framework to check the settings
would be helpful. Check also bug #18673 and patch #2542.

see gna bug #18875
--ine, and those below, will be ignored--

M server/settings.c

(Browse SVN revision 20505)

Matthias Pfafferodt <syntron>
Project Member
Tue 08 Nov 2011 08:57:08 PM UTC, SVN revision 20503:

fix map definition / generation

the map is defined with several settings (mapsize, size, tilesperplayer,
xsize, ysize, topology). Now, some combinations are invalid like in this
bug

mapsize == XYSIZE && ysize % 2 != 0 && MAP_IS_ISOMETRIC

I did add the required checks but a framework to check the settings
would be helpful. Check also bug #18673 and patch #2542.

see gna bug #18875

(Browse SVN revision 20503)

Matthias Pfafferodt <syntron>
Project Member
Sun 18 Sep 2011 08:27:54 PM UTC, comment #1:

An idea/possible solution:

define different levels to set/validate the values:

- basic checks (limits which should never broken)
- real checks (with settings for all variables)
- real+ checks (with settings for all variables; if a value is limited due to other values it is set to the next valid value)

The default behaviour (real check) would be used if the user changes a setting. If a savegame is loaded, first basic checks are done and, after all values are loaded, the real+ checks are run. Thus, one would get a valid game with the smallest number of changes. If the settings are a valid set no changes would be needed.

Matthias Pfafferodt <syntron>
Project Member
Sat 17 Sep 2011 03:15:25 PM UTC, original submission:

In settings_game_load(), care is taken to only run setting action functions once all settings are loaded to avoid dependency issues, but setting validation functions are run one at a time during loading. Since some of these functions do cross-checks (xsize/ysize, timeout/unitwaittime), depending on the order of settings in the save file, it is possible for a setting to be rejected even if it would have been valid had other settings been loaded first.

Contrived (unplayable) example attached, with the following settings section:

which generates an error when loaded with trunk (r20250):

but not if ysize is moved before xsize.

This is unlikely to bite anyone in practice at the moment, but if validation functions get more cross-checking, it could become a problem.

This will be fiddly to fix, as the full set of new (or replaced) settings will have to be held somewhere temporarily while the new set is validated, so that settings that fail the validation at the end can be rolled back.

(Also, the non-setting-specific validation is not called, e.g., setting_int_validate() which does the range check.)

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #14095:  hack.sav added by jtn (22kB - application/octet-stream - hacked unplayable version of tutorial scenario to illustrate this)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by syntron (Posted a comment)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follows 1 latest change.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 17 Sep 2011 03:16:33 PM UTCjtnAttached File-=>Added hack.sav, #14095
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup