patchFreeciv - Patches: patch #3937, Remove const.TRUE and const.FALSE...

 
 
Show feedback again

patch #3937: Remove const.TRUE and const.FALSE from Lua API before the stable release.

Submitted by:  Engla <englabenny>
Submitted on:  Sun 09 Jun 2013 01:40:00 AM UTC  
 
Category: generalPriority: 7 - High
Status: DonePrivacy: Public
Assigned to: Jacob Nevins <jtn>Open/Closed: Closed
Planned Release: 2.4.0-RC1,2.5.0,2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

(Jump to the original submission Jump to the original submission)

Sat 10 Aug 2013 10:22:13 PM UTC, SVN revision 23190:

Remove the Lua constants const.TRUE and const.FALSE.
They weren't used, and const.FALSE evaluated to true in Lua logical
expressions.

Request/patch from Ulrik Sverdrup (englabenny@gna).

See gna patch #3937.

(Browse SVN revision 23190)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sat 10 Aug 2013 10:15:25 PM UTC, SVN revision 23187:

Remove the Lua constants const.TRUE and const.FALSE.
They weren't used, and const.FALSE evaluated to true in Lua logical
expressions.

Request/patch from Ulrik Sverdrup (englabenny@gna).

See gna patch #3937.

(Browse SVN revision 23187)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sat 10 Aug 2013 10:10:37 PM UTC, SVN revision 23184:

Remove the Lua constants const.TRUE and const.FALSE.
They weren't used, and const.FALSE evaluated to true in Lua logical
expressions.
(This is a late Lua API change to 2.4.x.)

Request/patch from Ulrik Sverdrup (englabenny@gna).

See gna patch #3937.

(Browse SVN revision 23184)

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Fri 09 Aug 2013 08:07:15 PM UTC, comment #5:

After some more poking around I don't see any reason not to remove these (boolean interface to the game seems to be Lua true/false), and having a const.FALSE that doesn't evaluate as false seems dangerous.

So I intend to commit this before 2.4.0-RC1 (since it's had time for review).

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Tue 06 Aug 2013 12:27:36 AM UTC, comment #4:

This was added in patch #2872; no further rationale for the TRUE/FALSE constants given there or in the metaticket it's linked to.

Jacob Nevins <jtn>
Project AdministratorIn charge of this item.
Sun 04 Aug 2013 07:24:22 PM UTC, comment #3:

Well, this one have to be handled about NOW for it to make it to 2.4.0-RC1. I'm ok with freeze exception (for 2.4 data file format freeze, which dictates also scripting API) in this case, but only if the change has been made before RC1

Marko Lindqvist <cazfi>
Project Administrator
Sat 15 Jun 2013 12:26:42 PM UTC, comment #2:

Are const.TRUE and const.FALSE needed anywhere? I think our bindings should handle the translation from lua's `true` and `false`.

Further confusion comes from 0 and thus const.FALSE being having boolean value `true` in lua context, the only values with `false` boolean value are `false` and `nil`.

Engla <englabenny>
Project Member
Sat 15 Jun 2013 05:25:00 AM UTC, comment #1:

> Let's remove this before 2.4 goes stable


Strictly speaking 2.4 API has been frozen for a long time already.

I think TRUE and FALSE here are meant to mirror the fact that our C-code uses such number types.

Marko Lindqvist <cazfi>
Project Administrator
Sun 09 Jun 2013 01:40:00 AM UTC, original submission:

I think

const.TRUE = 1
and
const.FALSE = 0

were added to demonstrate the const module. It would be a mistake anyway, since lua already has the keywords `true` and `false` (of boolean, not number type).

Let's remove this before 2.4 goes stable so it doesn't have to stay in the API forever.

Patch removes the two variable definitions but leaves the empty const table intact.

Patch applies for master and 2.4.

Engla <englabenny>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by jtn (Posted a comment)
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by englabenny (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 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 10 Aug 2013 10:23:58 PM UTCjtnStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
      Planned Release2.4.0-RC1=>2.4.0-RC1,2.5.0,2.6.0
    Fri 09 Aug 2013 08:07:15 PM UTCjtnAssigned toNone=>jtn
    Tue 06 Aug 2013 12:27:36 AM UTCjtnPriority5 - Normal=>7 - High
      StatusNone=>Ready For Test
    Sun 04 Aug 2013 07:24:22 PM UTCcazfiCategoryNone=>general
      Planned Release2.4=>2.4.0-RC1
    Sun 09 Jun 2013 01:40:00 AM UTCenglabennyAttached File-=>Added 0001-Remove-const.TRUE-const.FALSE-from-lua-API.patch, #18061
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup