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 Feb 21 04:16:25 2013  
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 Apr 1 20:55:51 2013, 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 Mar 19 08:18:48 2013, comment #4:

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

Marko Lindqvist <cazfi>
Project Administrator
Tue Mar 19 04:58:19 2013, 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 Mar 19 04:51:18 2013, 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 Mar 19 00:05:04 2013, 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 Feb 21 04:16:25 2013, 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 Oct 29 18:51:55 2013cazfiPlanned Release2.5.0=>
    Fri Apr 19 09:30:13 2013cazfiAssigned tocazfi=>None
    Thu Feb 21 04:34:07 2013cazfiPriority3 - Low=>5 - Normal
      Assigned toNone=>cazfi
      Planned Release=>2.5.0
    Thu Feb 21 04:16:25 2013davide_at_debianAttached File-=>Added save_booleanized.patch, #17296
    Show feedback again

    Back to the top

    Powered by Savane 3.1-cleanup