[Freeciv-Dev] Re: (PR#3928) Convert client to use PF

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxx>]

On Fri, 2 May 2003, Raimar Falke wrote:

> On Sun, Apr 27, 2003 at 02:48:14PM -0700, Gregory Berkolaiko wrote:
> 
> New version attached which doesn't core dump for me and also doesn't
> produce outputs.

Attached is the path_finding.c updates torn out of Raimar's patch plus 
some extra bugfixes.  The summary:
* in TM_WORST_TIME the cost wasn't correctly capped
* upodate pf_print_path format
* remove LOG_NORMAL printouts
* when dealing with waiting, wrong cost was recorded in the path and also 
  "-1" for waiting_dir was put in the wrong place.

Please commit.

> > 2. When there is no path to a tile (say it's inland and we have a
> > ship), moving the mouse around this tile causes repeated calls to
> > reset_last_part.  Not a fatal bug, just CPU burn.
> 
> This should IMHO be done in do_unit_goto.

Raimar: You do not understand. Go, now

In your function update_last_part, instead of

  if (!new_path) {
    freelog(PATH_LOG_LEVEL, "  no path found");
    reset_last_part();
    return;
  }

it should be

  if (!new_path) {
    freelog(PATH_LOG_LEVEL, "  no path found");
    if (!same_pos(p->start_x, p->start_y, p->end_x, p->end_y)) {
      reset_last_part();
    }
    return;
  }

Alternatively, you could put this check in reset_last_part() itself.  Yes, 
I think it would be even better there.

> > 3. Maybe we could do basic sanity checks before calling PF.  We know
> > that a ship cannot go inland and such.  But this could be more
> > trouble than it's worth.
> 
> No. Don't let us duplicate the rules all over the place.

Ok.  But I might put a quick check if the target tile is not TB_DONT_ENTER
in the path_finding.c before we actually search for a path to this tile.  
I believe it's not rule duplication, it's more like a clever use of the
information supplied to us by the user.

G.

Index: common/aicore/path_finding.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/path_finding.c,v
retrieving revision 1.7
diff -u -r1.7 path_finding.c
--- common/aicore/path_finding.c        2003/05/01 21:04:46     1.7
+++ common/aicore/path_finding.c        2003/05/03 12:44:20
@@ -149,6 +149,7 @@
     cost = MIN(cost, pf_map->params->move_rate);
     break;
   case TM_WORST_TIME:
+    cost = MIN(cost, pf_map->params->move_rate);
     {
       int moves_left
          = get_moves_left(pf_map, pf_map->lattice[pf_map->index].cost);
@@ -169,7 +170,7 @@
       break;
     }
   default:
-    die("unknown TC");
+    die("unknown TM");
   }
   return cost;
 }
@@ -653,7 +654,7 @@
 
   for (i = 0; i < path->length; i++) {
     freelog(log_level,
-           "PF:   %2d/%2d: (%2d,%2d) dir=%2s cost=%2d (%2d, %d) EC=%d",
+           "PF:   %2d/%2d: (%2d,%2d) dir=%-2s cost=%2d (%2d, %d) EC=%d",
            i + 1, path->length,
            path->positions[i].x, path->positions[i].y,
            dir_get_name(path->positions[i].dir_to_next_pos),
@@ -955,12 +956,12 @@
 
   if (pf_map->status[pf_map->index] == NS_WAITING) {
     /* We've already returned this node once, skip it */
-    freelog(LOG_NORMAL, "Considering waiting at (%d, %d)", pf_map->x,
+    freelog(LOG_DEBUG, "Considering waiting at (%d, %d)", pf_map->x,
            pf_map->y);
     return danger_iterate_map(pf_map);
   } else if (pf_map->d_lattice[index].is_dangerous) {
     /* We don't return dangerous tiles */
-    freelog(LOG_NORMAL, "Reached dangerous tile (%d, %d)", pf_map->x,
+    freelog(LOG_DEBUG, "Reached dangerous tile (%d, %d)", pf_map->x,
            pf_map->y);
     return danger_iterate_map(pf_map);
   } else {
@@ -996,6 +997,7 @@
   path->length = d_node->step + 1;
 
   for (i = d_node->step; i >= 0; i--) {
+    bool old_waited;
 
     /* 1: Deal with waiting */
     if (!d_node->is_dangerous) {
@@ -1008,9 +1010,12 @@
         path->positions[i].turn = get_turn(pf_map, node->cost) + 1;
         path->positions[i].moves_left = pf_map->params->move_rate;
         path->positions[i].total_MC 
-          = path->positions[i].turn * pf_map->params->move_rate;
-        /* Staying here! */
-        path->positions[i].dir_to_next_pos = -1;
+          = ((path->positions[i].turn - 1) * pf_map->params->move_rate
+             + pf_map->params->moves_left_initially);
+        path->positions[i].dir_to_next_pos = dir_next;
+        /* Set old_waited so that we record -1 as a direction at the step 
+         * we were going to wait */
+        old_waited = TRUE;
         i--;
       }
       /* Update "waited" (d_node->waited means "waited to get here") */
@@ -1034,7 +1039,7 @@
       = get_moves_left(pf_map, path->positions[i].total_MC);
     path->positions[i].total_MC -= pf_map->params->move_rate
       - pf_map->params->moves_left_initially;
-    path->positions[i].dir_to_next_pos = dir_next;
+    path->positions[i].dir_to_next_pos = (old_waited ? -1 : dir_next);
 
     /* 3: Check if we finished */
     if (i == 0) {