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 Feb 28 16:08:51 2014  
Category: clientPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Sveinung Kvilhaugsvik <sveinung>Open/Closed: Closed
Planned Release: 2.6.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.


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

Sun Apr 27 17:29:08 2014, 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 Apr 24 18:07:00 2014, 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 Apr 24 15:34:55 2014, 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 Apr 24 07:44:49 2014, 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 Apr 20 23:54:01 2014, 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 Mar 2 01:47:14 2014, 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 Feb 28 16:08:51 2014, 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):


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.


    Error: not logged in



    Follow 9 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun Apr 27 17:30:47 2014sveinungStatusReady For Test=>Done
    Thu Apr 24 18:07:00 2014sveinungAttached File-=>Added 0001-Make-the-function-to-check-if-a-unit-fulfills-a-requ.patch, #20572
    Thu Apr 24 07:44:49 2014sveinungAttached File-=>Added 0001-Make-the-function-to-check-if-a-unit-fulfills-a-requ.patch, #20570
    Sun Mar 2 23:44:21 2014sveinungDependenciesRemoved dependancy from patch #4571=>-
    Sun Mar 2 17:42:57 2014sveinungDependencies-=>patch #4571 is dependent
    Sun Mar 2 01:47:14 2014sveinungAttached 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