[freeciv-ai] Re: [Freeciv-Dev] Re: (PR#11995) Stupid AI Creates Tall Stacks

["Per I. Mathisen" <per@xxxxxxxxxxx>]

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

On Sun, 20 Mar 2005, Benedict Adamson wrote:
> Here is the third version of my patch, which uses PF for AI movement and
> tries to avoid creating tall stacks of units. Once this is committed I
> will work on using the movemap to more accurately estimate the danger of
> tall stacks.

A few comments.

+++ freeciv.PR11995/ai/aihunt.c 2005-03-20 21:55:35.000000000 +0000
@@ -473,8 +473,15 @@
   }

   /* Go towards it. */
-  if (!ai_unit_goto(punit, target->tile)) {
-    return TRUE;
+  if (unit_type(punit)->move_type == LAND_MOVING) {
+    /* If necessary, use a ferry */
+    if (!ai_gothere(pplayer, punit, target->tile)) {
+      return TRUE;
+    }
+  } else {
+    if (!ai_unit_goto(punit, target->tile)) {
+      return TRUE;
+    }
   }

This is unnecessary. Hunters are not meant to hunt across continents. I
have a patch which will make hunters move with pf, too. I will post it
once my ai unit management patch is in cvs, since it overlaps badly.

+++ freeciv.PR11995/ai/aiferry.c        2005-03-20 21:55:35.000000000 +0000
@@ -546,10 +546,6 @@
                ferryboat->id, ferryboat->ai.passenger);
       return FALSE;
     }
-
-    UNIT_LOG(LOGLEVEL_GOBYBOAT, punit, "Our boat has arrived "
-            "[%d](moves left: %d)", ferryboat->id, ferryboat->moves_left);
-    handle_unit_activity_request(punit, ACTIVITY_IDLE);
   }

Unfortunately, this clashes with my unit management cleanup patch. I will
commit this patch very soon, so you will have to find some way to
incorporate the change in how "extra" passengers are handled: In the patch
they are "managed" once the ferry arrives at its destination. I think this
is the correct way to do it, since I hated the way the current code would
run a full unit management routine on ferry passengers, which was a total
waste of perfectly good CPU time.

Looks like the simplest solution would be to move my new code to
ai_amphibious_goto_constrained().

+++ freeciv.PR11995/ai/aitools.c        2005-03-20 21:55:35.000000000 +0000
...
+static double p_killed_at(const struct tile *ptile,
+                         struct ai_risk_cost *risk_cost,
+                         struct pf_parameter *param)

Rename function to something intelligible, please.

+{
+  double db;
+  /* Compute the basic probability */
+  /* WAG */
+  double p = is_ocean(ptile->terrain)? 0.05: 0.15;

What is the reason for this difference in probability? Explain in a
comment. I am also unsure if this is correct. Three times higher chance of
getting hit on an ocean square? I would think, instead, that since it is
easier to avoid stacking on ocean squares, this should be penalized more,
not less.

+static int stack_risk(const struct tile *ptile,

Looking at the building cost of units when moving around may not be a good
idea - once you have assembled an attack force, their building cost is
largely irrelevant. A low cost unit may be as necessary as a high cost one
in the larger picture. There also seems to be no accounting for stacking
in order to _reduce_ risk, by adding a better best defender to the stack.
(Although I am not sure if adding this is a good idea.)

+/*********************************************************************
+  PF extra cost call back to avoid creating tall stacks.
+  By setting this as an extra-cost call-back, paths will avoid tall stacks.
+  Avoiding tall stacks *all* along a path is useful because a unit following a
+  path might have to stop early because of ZoC or random movement.
+*********************************************************************/
+static int prefer_short_stacks(const struct tile *ptile,

Random movement will be gone shortly.

+  } else if (is_military_unit(punit)) {
+    switch (punit->ai.ai_role) {
+    case AIUNIT_AUTO_SETTLER:
+    case AIUNIT_BUILD_CITY:
+      UNIT_LOG(LOG_ERROR, punit, "military settler or worker");

This is actually a possible and valid combination.

@@ -512,7 +925,6 @@
   bool alive;

   CHECK_UNIT(punit);
-  assert(unit_owner(punit)->ai.control);
   assert(is_tiles_adjacent(punit->tile, ptile));

   handle_unit_activity_request(punit, ACTIVITY_IDLE);
@@ -543,8 +955,6 @@
   struct player *pplayer = unit_owner(punit);

   CHECK_UNIT(punit);
-  assert(unit_owner(punit)->ai.control);
-  assert(is_tiles_adjacent(punit->tile, ptile));

What is the reason for removing these asserts?

+++ freeciv.PR11995/ai/aiunit.c 2005-03-20 21:55:35.000000000 +0000
@@ -1849,22 +1836,46 @@
...
-        UNIT_LOG(LOG_DEBUG, punit, "Barbarian sailing to %s", pc->name);
-        (void) ai_gothere(pplayer, punit, ftile);
+       struct unit *ferry = NULL;
+       unit_list_iterate(punit->tile->units, aunit) {
+         if (is_boat_free(aunit, punit, 2)) {
+           ferry = aunit;
+           break;
+         }
+       } unit_list_iterate_end;
+       if (ferry) {
+         struct pft_amphibious parameter;
+         UNIT_LOG(LOG_DEBUG, punit, "Barbarian sailing to conquer %s",
+                  pc->name);
....

Instead of adding a lot of code here, is it not possible to remove this
special casing for barbarians?

Cosmetic nitpick: Please leave a line of space betweeen variable
declarations and the rest of a bracket of code.

I like this patch a lot. Once you address the comments above, I will
commit it.

  - Per