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 08 Apr 2013 06:34:15 PM UTC  
 
Category: rulesetsPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Marko Lindqvist <cazfi>Open/Closed: Closed
Planned Release: 2.5.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Sun 21 Apr 2013 08:11:16 PM UTC, 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 08 Apr 2013 11:53:35 PM UTC, 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 08 Apr 2013 10:55:07 PM UTC, 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 08 Apr 2013 09:04:06 PM UTC, 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 08 Apr 2013 06:34:15 PM UTC, 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):
   
   
Comment:
   

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

 

Depends on the following items: None found

Digest:
   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.

     

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

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 21 Apr 2013 08:11:27 PM UTCcazfiStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
    Fri 19 Apr 2013 08:09:12 AM UTCcazfiStatusNone=>Ready For Test
      Assigned toNone=>cazfi
      Planned Release=>2.5.0
    Mon 08 Apr 2013 11:53:35 PM UTCpersiaAttached File-=>Added validate-survives-negated-req-values+concise.patch, #17722
    Mon 08 Apr 2013 08:45:44 PM UTCpepetoDependencies-=>patch #1449 is dependent
    Mon 08 Apr 2013 06:34:15 PM UTCpersiaAttached File-=>Added validate-survives-negated-req-values.patch, #17718
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup