patchFreeciv - Patches: patch #3842, Ensure use of boolean TRUE/FALSE...

Show feedback again

patch #3842: Ensure use of boolean TRUE/FALSE in reqs specifications

Submitted by:  Emmet Hikory <persia>
Submitted on:  Mon Apr 8 18:34:15 2013  
Category: rulesetsPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Planned Release: 2.5.0Contains 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.


Sun Apr 21 20:11:16 2013, SVN revision 22740:

Give error if requirement fields "survives" and "negated" are not valid
boolean values in ruleset.

Patch by Emmet Hikory

See patch #3842

(Browse SVN revision 22740)

Marko Lindqvist <cazfi>
Project AdministratorIn charge of this item.
Mon Apr 8 23:53:35 2013, comment #3:

An updated patch implementing pepeto's more concise expression (Thanks!). This has passed the same set of tests applied to the original patch, with the same results.

(file #17722)

Emmet Hikory <persia>
Project Member
Mon Apr 8 22:55:07 2013, comment #2:

Thanks for the GNA instruction and adjustment.

Your logic is both more concise and absolutely correct: I missed the implications of the SECFILE_RETURN_VAL_IF_FAIL check.

Emmet Hikory <persia>
Project Member
Mon Apr 8 21:04:06 2013, comment #1:

> This should be a dependency of patch #1449 , but I can't figure
> out how to make that true from the GNA interface: if someone
> would be willing to so mark it, I would appreciate that.

You have to go to the other gna page, and search for this item (e.g. using the patch number). And then post. I have done it now.

Your patch looks good except, the first default case is not indented correctly. I don't know if this is a good idea to use a default case here anyway.

I would suggest something like:

pepeto <pepeto>
Project Member
Mon Apr 8 18:34:15 2013, original submission:

For the boolean values "survives" and "negated", the ruleset format used to accept 0 and 1. This has been changed so that TRUE and FALSE are expected. This patch ensures that values detected as integers will raise a ruleset error, rather than silently defaulting to FALSE (possibly changing gameplay behaviour for insufficiently transitioned rulesets).

Note that this uses considerably lower-level secfile access, and has some duplicated logic. In the event that there was more of this that needed doing, it may be worth considering implementing it by passing an address to secfile_lookup_bool_default(), and using the return value as an indicator of whether the returned value was default because the entry was missing or whether it was default because the content of the entry wasn't boolean. That said, the fact that the file format can support integers masquerading as booleans might make that less usefully general with such specialisation: I leave further speculation to anyone who wants to refactor the check to use a shared function.

This should be a dependency of patch #1449 , but I can't figure out how to make that true from the GNA interface: if someone would be willing to so mark it, I would appreciate that.

Emmet Hikory <persia>
Project Member


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

Attach File(s):

Attached Files
file #17718:  validate-survives-negated-req-values.patch added by persia (2kB - application/octet-stream)


Depends on the following items: None found

   patch dependencies.


Carbon-Copy List
  • -unavailable- added by cazfi (Updated the item)
  • -unavailable- added by pepeto (Posted a comment)
  • -unavailable- added by persia (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 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun Apr 21 20:11:27 2013cazfiStatusReady For Test=>Done
    Fri Apr 19 08:09:12 2013cazfiStatusNone=>Ready For Test
      Assigned toNone=>cazfi
      Planned Release=>2.5.0
    Mon Apr 8 23:53:35 2013persiaAttached File-=>Added validate-survives-negated-req-values+concise.patch, #17722
    Mon Apr 8 20:45:44 2013pepetoDependencies-=>patch #1449 is dependent
    Mon Apr 8 18:34:15 2013persiaAttached File-=>Added validate-survives-negated-req-values.patch, #17718
    Show feedback again

    Back to the top

    Powered by Savane 3.1-cleanup