patchFreeciv - Patches: patch #3881, Use an effect to control...

 
 
Show feedback again

patch #3881: Use an effect to control fortification capability

Submitted by:  Emmet Hikory <persia>
Submitted on:  Wed 24 Apr 2013 06:35:57 PM UTC  
 
Category: generalPriority: 5 - Normal
Status: PostponedPrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

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

 

Sat 27 Apr 2013 11:38:33 PM UTC, comment #2:

And this isn't suitable for review anymore. Firstly, the reuse of the API changes from patch #3871 won't work with the changes planned for that patch, and secondly, the helptext generation is wrong (incorrectly associates the ability to fortify with unit class, potentially provides different help strings for players of different nations or currently with different governments, leading to user confusion).

Emmet Hikory <persia>
Project Member
Wed 24 Apr 2013 09:37:40 PM UTC, comment #1:

And of course, my first revision was incomplete: this contains also the appropriate updates for README.effects

(file #17828)

Emmet Hikory <persia>
Project Member
Wed 24 Apr 2013 06:35:57 PM UTC, original submission:

The current code has a hardcoded is_ocean() call in the gate conditional for ACTIVITY_FORTIFYING, meaning that UCF_CAN_FORTIFY units can fortify on any land terrain and never fortify on any oceanic terrain. The attached patch changes this conditional to use an effect, supporting both more expressiveness in ruleset definition and wider scopes for complex nativity. Some examples of things the patch enables:

Submarines may fortify in Deep Ocean if the player has researched the Rebreathing technology.

Amphibious Assault Vehicles may fortify on land other than mountains or hills, but wallow in water and may not fortify even in the presence of a river.

Big Land units cannot fortify on Farmland.

This patch depends on the exposure of is_effect_active() from patch #3871, but it is trivial to restack the exposure if anyone thinks this should be applied with greater priority than that.

Note also that the construction in helpdata.c here and the construction in city.c there are sufficiently parallel that if anything else wants to do this class of RPT_POSSIBLE test, it may be better to define a simple function for that specific purpose, rather than exposing is_effect_active()

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 #17828:  fortification-possible-effect+docs.patch added by persia (20kB - application/octet-stream)
file #17827:  fortification-possible-effect.patch added by persia (20kB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -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 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sun 13 Apr 2014 09:52:04 AM UTCpersiaStatusNone=>Postponed
    Wed 24 Apr 2013 09:37:40 PM UTCpersiaAttached File-=>Added fortification-possible-effect+docs.patch, #17828
    Wed 24 Apr 2013 06:35:57 PM UTCpersiaAttached File-=>Added fortification-possible-effect.patch, #17827
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup