patchFreeciv - Patches: patch #4558, Help generator: generalize...

 
 
Show feedback again

patch #4558: Help generator: generalize unit_type_fulfills_requirement()

Submitted by:  Sveinung Kvilhaugsvik <sveinung>
Submitted on:  Fri 28 Feb 2014 04:08:51 PM UTC  
 
Category: clientPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Sveinung Kvilhaugsvik <sveinung>Open/Closed: Closed
Planned Release: 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)

Sun 27 Apr 2014 05:29:08 PM UTC, SVN revision 24814:

Make the function to check if a unit fulfills a requirement
vector more reusable

- Move it to common requirement code
- Make it easy to support other universal kinds

Reviewed by Emmet Hikory

See patch #4558

(Browse SVN revision 24814)

Sveinung Kvilhaugsvik <sveinung>
Project MemberIn charge of this item.
Thu 24 Apr 2014 06:07:00 PM UTC, comment #5:

> The only other thing I see is IFT_NO vs. ITF_YES

Typo. Good catch. Fixed version attached.

> I suspect a typo, but the number of repetitions makes me uncertain

Integrated development environment have many nice typo related features. One is that they will auto complete you typos for you so you won't have to repeat them. Another is that if a symbol is missing a typo you can easily include one in all its instances using the handy rename feature. Should you ever require more typos I warmly recommend using one.

(file #20572)

Sveinung Kvilhaugsvik <sveinung>
Project MemberIn charge of this item.
Thu 24 Apr 2014 03:34:55 PM UTC, comment #4:

The only other thing I see is IFT_NO vs. ITF_YES with no comment explaining why they differ (I suspect a typo, but the number of repetitions makes me uncertain).

Emmet Hikory <persia>
Project Member
Thu 24 Apr 2014 07:44:49 AM UTC, comment #3:

Thank you for your feed back. New version:
- moved to common requirements code
- got rid of "arg" as a variable name

(file #20570)

Sveinung Kvilhaugsvik <sveinung>
Project MemberIn charge of this item.
Sun 20 Apr 2014 11:54:01 PM UTC, comment #2:

This code is readable to me, and seems both sensible and useful in all sorts of ways. I'd rather not see the universals_u in unit_type_fulfills_requirement() named "arg", because I have to think about it in the function call. Perhaps use "univ" or similar (as in universal_fulfills_requirement()).

Personally, I'd rather see most of the enablement code moved to requirements.c, as part of the interface, so that other cases where we want to run boolean analysis inside a requirements iteration can be similarly encapsulated (the AI wants to check stuff like this fairly often as well).

Emmet Hikory <persia>
Project Member
Sun 02 Mar 2014 01:47:14 AM UTC, comment #1:

Patch that use the described approach. A reference to a helper function is passed to a generic function that test for all universals. Applies on top of patch #4557.

(file #20227)

Sveinung Kvilhaugsvik <sveinung>
Project MemberIn charge of this item.
Fri 28 Feb 2014 04:08:51 PM UTC, original submission:

I would like to generalize generalize unit_type_fulfills_requirement() by splitting out the utype specific checks so I can reuse it for other universal kinds. My current design is to define two helper functions that is used by unit_type_fulfills_requirement(). The first helper function is a parameter to the second. Is this the preferred way to do this in C or is something else (like macros) preferred?

Reasons why I ask:

  • I didn't find a way to send the utype to the utype specific code (like partial application or a closure) in unit_type_fulfills_requirement(). This makes me suspect C isn't meant to be used this way. (I don't mind passing the parameter to the first helper function in the second helper function. I just wonder if this is a hint)
  • I don't know the details of the performance characteristics for gcc compiled C code. I therefore don't know if this approach will make things slow or not.
  • I don't know if other C developers reading the code easily will recognize this pattern. I see call back functions used in the gui code of the clients but for all I know it could be specific to GUI programming.
Sveinung Kvilhaugsvik <sveinung>
Project MemberIn charge of this item.

 

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

Attach File(s):
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by persia (Posted a comment)
  • -unavailable- added by sveinung (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
    Sun 27 Apr 2014 05:30:47 PM UTCsveinungStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
    Thu 24 Apr 2014 06:07:00 PM UTCsveinungAttached File-=>Added 0001-Make-the-function-to-check-if-a-unit-fulfills-a-requ.patch, #20572
    Thu 24 Apr 2014 07:44:49 AM UTCsveinungAttached File-=>Added 0001-Make-the-function-to-check-if-a-unit-fulfills-a-requ.patch, #20570
    Sun 02 Mar 2014 11:44:21 PM UTCsveinungDependenciesRemoved dependancy from patch #4571=>-
    Sun 02 Mar 2014 05:42:57 PM UTCsveinungDependencies-=>patch #4571 is dependent
    Sun 02 Mar 2014 01:47:14 AM UTCsveinungAttached File-=>Added 0003-Make-it-easy-to-write-functions-like-unit_type_fulfi.patch, #20227
      StatusNeed Info=>Ready For Test
      Summary[RFC] Help generator: generalize unit_type_fulfills_requirement()=>Help generator: generalize unit_type_fulfills_requirement()
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup