patchFreeciv - Patches: patch #3726, Booleanize all save functions for...

Show feedback again

patch #3726: Booleanize all save functions for savegame v2

Submitted by:  Davide Baldini <davide_at_debian>
Submitted on:  Thu 21 Feb 2013 04:16:25 AM UTC  
Category: generalPriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):

You are not logged in

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


Mon 01 Apr 2013 08:55:51 PM UTC, comment #5:

Could you produce new version of this patch to go in before S2_5 branching in a month?

Marko Lindqvist <cazfi>
Project Administrator
Tue 19 Mar 2013 08:18:48 AM UTC, comment #4:

save_command() should cmd_reply(C_FAIL) when appropriate and return FALSE in such a case.

Marko Lindqvist <cazfi>
Project Administrator
Tue 19 Mar 2013 04:58:19 AM UTC, comment #3:

[ Ending piece - "browser authonomously submitted the post before I completed it" ]


I could complete the return value's climbing chain of function up to handle_stdin_input_real(), making things logically cleaner.

Davide Baldini <davide_at_debian>
Tue 19 Mar 2013 04:51:18 AM UTC, comment #2:

In its current state, the proposed patch doesn't touch how the save functions behave, and it just reports upwards in the functions chain whether a failure has been detected.
Such report emerges up to save_command() in form of return value, and there it can be checked. The value is not returned back to handle_stdin_input_real() too, because it's not actually needed, and because the patch was taken off Greatturn, which fork()s save_command() to speed up the saving time and hence it can pass no value back to handle_stdin_input_real().

Davide Baldini <davide_at_debian>
Tue 19 Mar 2013 12:05:04 AM UTC, comment #1:

Reading the patch alone (not checking existing context from sources) it seems that even after noticed failure you still continue to try to do everything inside that particular function as if everything succeeded. At least executing secfile_save() after secfile creation has already failed seems like wrong thing to do.

Marko Lindqvist <cazfi>
Project Administrator
Thu 21 Feb 2013 04:16:25 AM UTC, original submission:

Booleanize the structure of functions below save_command() for savegame version 2.

Currently, failures to save the game are only textually reported to users but are not treated by the server, which is bad for automated, self-managed servers running 24/7, such as Longturn, Greatturn and similar.

This patch intercepts all save failures along the functions chain beneath save_command(), and makes a boolean available to let the machine check the result.

Since when such save failures occur (various assert fails) they usually compromise any other future saves too, Greatturn uses this to automatically revert the game to the last functioning savefile, 10 minutes back.

Other case usage for this could be a Lua handle to send a warning email to the server operator, etc.

Davide Baldini <davide_at_debian>


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

Attach File(s):

Attached Files


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -unavailable- added by cazfi (Updated the item)
  • -unavailable- added by davide_at_debian (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.


    Error: not logged in



    Follow 6 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 29 Oct 2013 06:51:55 PM UTCcazfiPlanned Release2.5.0=>
    Fri 19 Apr 2013 09:30:13 AM UTCcazfiAssigned tocazfi=>None
    Thu 21 Feb 2013 04:34:07 AM UTCcazfiPriority3 - Low=>5 - Normal
      Assigned toNone=>cazfi
      Planned Release=>2.5.0
    Thu 21 Feb 2013 04:16:25 AM UTCdavide_at_debianAttached File-=>Added save_booleanized.patch, #17296
    Show feedback again

    Back to the top

    Powered by Savane 3.1-cleanup