[Freeciv-Dev] Re: (PR#4758) client crash on goto

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Gregory Berkolaiko wrote:
> On Tue, 5 Aug 2003, Jason Short wrote:
> 
>>With the "return to nearest city" operation:
>>
>>[1]+  Segmentation fault      (core dumped) ./civ
> 
> The problem here is that find_nearest_allied_city finds a city to which 
> more rigourous client-goto cannot find a path.
> 
> The code itself is ridiculous, find_nearest_allied_city should return the 
> path and not ask update_last_path to recompute everything from scratch.
> 
> It should, of course, use standard client-goto callbacks rather then 
> simple_unit_path_iterate, which is called simple for a reason.

How about this?

jason

? rc
Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.107
diff -u -r1.107 control.c
--- client/control.c    2003/07/23 13:46:01     1.107
+++ client/control.c    2003/08/06 15:26:37
@@ -664,15 +664,12 @@
 void request_unit_return(struct unit *punit)
 {
   struct city *pcity;
+  struct pf_path *path;
 
-  enter_goto_state(punit);
-
-  if ((pcity = find_nearest_allied_city(punit))) {
-    draw_line(pcity->x, pcity->y);
-    send_goto_route(punit);
+  if ((pcity = find_nearest_allied_city(punit, &path))) {
+    send_goto_path(punit, path);
+    pf_destroy_path(path);
   }
-
-  exit_goto_state();
 }
 
 /**************************************************************************
Index: client/goto.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.c,v
retrieving revision 1.54
diff -u -r1.54 goto.c
--- client/goto.c       2003/07/21 19:01:01     1.54
+++ client/goto.c       2003/08/06 15:26:37
@@ -439,6 +439,34 @@
 }
 
 /********************************************************************** 
+  Send the given path for the unit to the server.
+***********************************************************************/
+void send_goto_path(struct unit *punit, struct pf_path *path)
+{
+  struct packet_goto_route p;
+  int i;
+
+  p.unit_id = punit->id;
+
+  /* we skip the start position */
+  /* FIXME: but for unknown reason the server discards the last position */
+  p.length = path->length - 1 + 1;
+  p.first_index = 0;
+  p.last_index = p.length - 1;
+  p.pos = fc_malloc(p.length * sizeof(*p.pos));
+  for (i = 0; i < path->length - 1; i++) {
+    p.pos[i].x = path->positions[i + 1].x;
+    p.pos[i].y = path->positions[i + 1].y;
+    freelog(PACKET_LOG_LEVEL, "  packet[%d] = (%d,%d)",
+           i, p.pos[i].x, p.pos[i].y);
+  }
+
+  send_packet_goto_route(&aconnection, &p, ROUTE_GOTO);
+
+  free(p.pos);
+}
+
+/********************************************************************** 
   FIXME: the packet interface need to be changed to support danger
   paths.
 ***********************************************************************/
@@ -490,9 +518,8 @@
 ***********************************************************************/
 void send_goto_route(struct unit *punit)
 {
-  struct packet_goto_route p;
-  int i;
   struct pf_path *path = NULL;
+  int i;
 
   assert(is_active);
   assert(punit->id == goto_map.unit_id);
@@ -500,24 +527,8 @@
   for (i = 0; i < goto_map.num_parts; i++) {
     path = pft_concat(path, goto_map.parts[i].path);
   }
-
-  p.unit_id = punit->id;
 
-  /* we skip the start position */
-  /* FIXME: but for unknown reason the server discards the last position */
-  p.length = path->length - 1 + 1;
-  p.first_index = 0;
-  p.last_index = p.length - 1;
-  p.pos = fc_malloc(p.length * sizeof(struct map_position));
-  for (i = 0; i < path->length - 1; i++) {
-    p.pos[i].x = path->positions[i + 1].x;
-    p.pos[i].y = path->positions[i + 1].y;
-    freelog(PACKET_LOG_LEVEL, "  packet[%d] = (%d,%d)", i, p.pos[i].x,
-            p.pos[i].y);
-  }
-  send_packet_goto_route(&aconnection, &p, ROUTE_GOTO);
-  free(p.pos);
-  p.pos = NULL;
+  send_goto_path(punit, path);
   pf_destroy_path(path);
 }
 
@@ -604,28 +615,53 @@
 
 /**************************************************************************
   Find the nearest (fastest to reach) allied city for the unit, or NULL if
-  none is reachable.
+  none is reachable.  If there is a reachable city and 'path' is NULL, it
+  will be set to a newly-allocated PF path indicating how to get there.
 ***************************************************************************/
-struct city *find_nearest_allied_city(struct unit *punit)
+struct city *find_nearest_allied_city(struct unit *punit,
+                                     struct pf_path **path)
 {
   struct city *pcity = NULL;
+  struct pf_parameter parameter;
+  struct pf_map *map;
 
   if ((pcity = is_allied_city_tile(map_get_tile(punit->x, punit->y),
                                   game.player_ptr))) {
-    /* We're already on a city - PF doesn't check for this (!). */
-    return pcity;
+    /* We're already on a city - don't go anywhere. */
+    return NULL;
+  }
+
+  pft_fill_default_parameter(&parameter);
+  pft_fill_unit_parameter(&parameter, punit);
+  parameter.get_EC = get_EC;
+  if (unit_type(punit)->attack_strength > 0) {
+    parameter.get_TB = get_TB_aggr;
+  } else {
+    parameter.get_TB = get_TB_peace;
   }
+  parameter.turn_mode = TM_WORST_TIME;
+  parameter.start_x = punit->x;
+  parameter.start_y = punit->y;
+  parameter.moves_left_initially = punit->moves_left;
 
-  simple_unit_path_iterator(punit, pos) {
+  map = pf_create_map(&parameter);
+
+  while (pf_next(map)) {
+    struct pf_position pos;
+
+    pf_next_get_position(map, &pos);
+
     if ((pcity = is_allied_city_tile(map_get_tile(pos.x, pos.y),
                                     game.player_ptr))) {
-      /* We use break so that the PF map can be destroyed. */
       break;
     }
-  } simple_unit_path_iterator_end;
+  }
+
+  if (pcity && path) {
+    *path = pf_next_get_path(map);
+  }
 
-  /* FIXME: For some reason this sometimes seems to find arbitrary but
-   * reproducable far-away cities instead of the obvious closest city. */
+  pf_destroy_map(map);
 
   return pcity;
 }
Index: client/goto.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/goto.h,v
retrieving revision 1.9
diff -u -r1.9 goto.h
--- client/goto.h       2003/07/07 19:14:36     1.9
+++ client/goto.h       2003/08/06 15:26:37
@@ -15,6 +15,8 @@
 
 #include "map.h"
 
+#include "path_finding.h"
+
 void init_client_goto(void);
 void free_client_goto(void);
 void enter_goto_state(struct unit *punit);
@@ -27,9 +29,11 @@
 void draw_line(int dest_x, int dest_y);
 int get_drawn(int x, int y, int dir);
 
+void send_goto_path(struct unit *punit, struct pf_path *path);
 void send_patrol_route(struct unit *punit);
 void send_goto_route(struct unit *punit);
 
-struct city *find_nearest_allied_city(struct unit *punit);
+struct city *find_nearest_allied_city(struct unit *punit,
+                                     struct pf_path **path);
 
 #endif /* FC__GOTO_H */