patchFreeciv - Patches: patch #4997, [Metaticket] Get rid of...

Show feedback again

patch #4997: [Metaticket] Get rid of action_enabler_append_hard()

Submitted by:  Marko Lindqvist <cazfi>
Submitted on:  Tue 29 Jul 2014 04:48:12 PM UTC  
Category: generalPriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
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.


Mon 25 Aug 2014 09:22:23 PM UTC, comment #3:

patch #5102 is a step towards removing the hard coding of the Diplomat unit type flag requirement.

Sveinung Kvilhaugsvik <sveinung>
Project Member
Thu 31 Jul 2014 10:33:03 AM UTC, comment #2:

My apologies for any confusion: the current AI is omniscient, so there's no increase of want for embassy creation based on other conditions (like relations with the other player, wish to understand the techs / money / map / units of the other player, etc.). The current AI also has hardcoded knowledge that embassies are player-scoped, so doesn't bother considering an embassy if one exists.

A different AI could certainly not have this knowledge, but there are performance benefits to understanding that embassies are player-scoped, whereas most other actions are city- or unit-scoped, so I imagine that other AIs would also embed this knowledge, either in code, or in datasets used for initial knowledge definition.

For humans, it's a UI question: establishing an embassy in a city owned by a player with whom one already has an embassy should be indicated to be useless in the UI (and possibly disabled): I'm unsure how having this in the ruleset vs. the code makes a difference.

Now, if embassies were moved to city-scope (so that if one has an embassy, and the city is conquered, one loses that embassy and gains an embassy with the conquering player), then the above no longer applies, but unless that is a real plan today, it would be sensible to deal with a hardcoded restriction during that effort (as there are so many other hardcoded assumptions one needs to adjust for such a change).

Emmet Hikory <persia>
Project Member
Thu 31 Jul 2014 10:08:49 AM UTC, comment #1:

Added patch #4671 (move the source tile limits to the ruleset) as a dependency.

In patch #4995 Emmet Hikory <persia> wrote: I'm unsure whether having that in a requirements vector is better: it makes it easier to write help or set the action dialog content in the UI for humans, but the AI will still benefit from hardcoding here (because it's not worth checking for a unit that can create an embassy or where it might go unless there is a lack of embassy with the target), but the current AI is omniscient, so doesn't really care about embassies, making this matter less.
Depends on the AI's knowledge source. An AI that has hard coded knowledge that creating an embassy when one already exists is pointless won't need it. An AI that get its (initial) domain knowledge from the rules will benefit from it.

Also, the fix affects also freeciv-manual, which might not be what we want
It isn't. I could have introduced a new argument "read only" (perhaps change save_script?) to make it work as wanted in freeciv-manual. Based on how unpopular action_enabler_append_hard() appears to be the least invasive solution (only loading it when playing) looked better.

Sveinung Kvilhaugsvik <sveinung>
Project Member
Tue 29 Jul 2014 04:48:12 PM UTC, original submission:

From patch #4995:
"if you're using action_enabler_append_hard() as a list of outstanding tasks, then I don't see the point of doing it a different way (which exposes the issues to users). My fear was that this was a facility that would be extended over time, rather than one that would be reduced over time."

action_enabler_append_hard() is not sustainable, but it must be only temporary solution. While bug #22401 fixes immediate problem with freeciv-ruledit, it doesn't help any future constructs inside freeciv-server (or client) using ruleset data. Also, the fix affects also freeciv-manual, which might not be what we want (patch #4995: "exposing them to the user is the correct thing to do. They are rules. The user should not have to care what source a rule came from."). Not to mention that having different rules inside freeciv-server and freeciv-ruledit risks bugs in validity checking etc.

While this temporary solution can be used at the moment (during 2.6 development), it should be cleaned out before it might cause problems with future development. Thus removal of action_enabler_append_hard() should be one of the criteria to decide if action enablers are ready for 2.6. It also means that no such actions should be moved there, for which the estimated "clean solution" is further away than 1Q2015.

Marko Lindqvist <cazfi>
Project Administrator


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

Attach File(s):

No files currently attached


   patch dependencies.

   task dependencies.


Carbon-Copy List
  • -unavailable- added by persia (Posted a comment)
  • -unavailable- added by sveinung (Posted a comment)
  • -unavailable- added by cazfi (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
    Thu 18 Dec 2014 09:24:24 PM UTCsveinungDependencies-=>Depends on patch #5583
    Thu 18 Dec 2014 09:03:03 PM UTCsveinungDependencies-=>Depends on patch #5582
    Mon 15 Dec 2014 11:06:20 PM UTCsveinungDependencies-=>Depends on patch #5569
    Wed 10 Dec 2014 04:48:04 PM UTCsveinungDependencies-=>Depends on patch #5532
    Fri 29 Aug 2014 03:04:13 PM UTCsveinungDependencies-=>Depends on patch #5148
    Wed 27 Aug 2014 02:11:51 PM UTCsveinungDependencies-=>Depends on patch #5131
    Mon 25 Aug 2014 09:22:23 PM UTCsveinungDependencies-=>Depends on patch #5102
    Thu 31 Jul 2014 10:08:49 AM UTCsveinungDependencies-=>Depends on patch #4671
    Show feedback again

    Back to the top

    Powered by Savane 3.1-cleanup