[freeciv-ai] (PR#11995) Stupid AI Creates Tall Stacks

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=11995 >

A coupled of more comments on the patch.

* I'm pretty sure ai_air_goto is wrong.  For air units it's not that no
tiles are dangerous; rather EVERY tile (except those with airbases,
empty carriers, or cities) is dangerous.  I think your goto function
will result in planes swiftly charging off to run out of fuel.  You can
look at how this is done in client/goto.c.  Remember that PF has no
handling of fuel yet.  But maybe I don't understand this code and
immediate_destination takes care of this.  In the long run it would be
better to have fuel and waypoints be handled directly by PF.  BTW what
happens if no immediate destination is found?  Does the (air) unit just
keep on trying or does it stop and look for a new role?

* I don't like how you replace calls of ai_unit_goto with a specific
typed goto call (ai_air_goto, ai_ferry_goto, etc.).  IMO it would be
better to just have ai_unit_goto and have this function check the type
of the unit and call one of the other functions.  If you don't want to
do this, you at least need to add an assertion at the top of each goto
function (e.g. ai_air_goto should assert(is_air_unit() || is_heli_unit()) ).

* I'm not sure of some of the code you add to pftools.c.  pftools is
used not just by the AI but also by human players.  You should avoid
adding ai-specific there *if possible*.

-jason