[Freeciv-Dev] Re: (PR#4604) fix bug with 'virtual' unit in settler code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Jason Short wrote:
> In contemplate_terrain_improvements we make use of a 'virtual' unit. 
> But since this data isn't created via create_unit_virtual the goto dest 
> is never set.  This means cities near (0,0) may behave oddly, and under 
> gen-topologies generates a crash...
> 
> The attached patch is the simplest fix.  But I think the comment should 
> be followed and a "correct" fix made.  There may be other important 
> values that aren't initialized...

This patch just uses create_unit_virtual to build the unit.  The only 
drawback is this means a malloc/free is needed (potentially slower).

I also changed another location in settlers.c to do the same thing.

jason

Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.167
diff -u -r1.167 settlers.c
--- server/settlers.c   2003/07/17 18:56:51     1.167
+++ server/settlers.c   2003/07/18 07:04:55
@@ -1557,27 +1557,24 @@
 void contemplate_new_city(struct city *pcity)
 {
   struct player *pplayer = city_owner(pcity);
-  struct unit virtualunit;
+  struct unit *virtualunit;
   int want;
   int gx = 0, gy = 0;
   struct unit *ferryboat = NULL; /* dummy */
+  Unit_Type_id unit_type = best_role_unit(pcity, F_CITIES); 
 
-  memset(&virtualunit, 0, sizeof(struct unit));
-  virtualunit.id = 0;
-  /* note virtual unit is not added to unit lists (eg pplayer->units),
-     so equivalently don't need to call idex_register_unit()  --dwp
-  */
-  virtualunit.owner = pplayer->player_no;
-  virtualunit.x = pcity->x;
-  virtualunit.y = pcity->y;
-  virtualunit.type = best_role_unit(pcity, F_CITIES);
-  if (virtualunit.type == U_LAST) {
+  if (unit_type == U_LAST) {
     freelog(LOG_DEBUG, "No F_CITIES role unit available");
     return;
   }
-  virtualunit.moves_left = unit_type(&virtualunit)->move_rate;
-  virtualunit.hp = unit_type(&virtualunit)->hp;
-  want = evaluate_city_building(&virtualunit, &gx, &gy, &ferryboat);
+
+  /* Create a localized "virtual" unit to do operations with. */
+  virtualunit = create_unit_virtual(pplayer, pcity, unit_type, FALSE);
+  virtualunit->x = pcity->x;
+  virtualunit->y = pcity->y;
+  want = evaluate_city_building(virtualunit, &gx, &gy, &ferryboat);
+  free(virtualunit);
+
   unit_list_iterate(pplayer->units, qpass) {
     /* We want a ferryboat with want 199 */
     if (qpass->ai.ferryboat == pcity->id)
@@ -1598,26 +1595,26 @@
 void contemplate_terrain_improvements(struct city *pcity)
 {
   struct player *pplayer = city_owner(pcity);
-  struct unit virtualunit;
+  struct unit *virtualunit;
   int want;
   int gx, gy; /* dummies */
   enum unit_activity best_act;
   struct tile *ptile = map_get_tile(pcity->x, pcity->y);
   struct ai_data *ai = ai_data_get(pplayer);
+  Unit_Type_id unit_type = best_role_unit(pcity, F_SETTLERS);
 
-  memset(&virtualunit, 0, sizeof(struct unit));
-  virtualunit.id = 0;
-  virtualunit.owner = pplayer->player_no;
-  virtualunit.x = pcity->x;
-  virtualunit.y = pcity->y;
-  virtualunit.type = best_role_unit(pcity, F_SETTLERS);
-  if (virtualunit.type == U_LAST) {
+  if (unit_type == U_LAST) {
     freelog(LOG_DEBUG, "No F_SETTLERS role unit available");
     return;
   }
-  virtualunit.moves_left = unit_type(&virtualunit)->move_rate;
-  virtualunit.hp = unit_type(&virtualunit)->hp;  
-  want = evaluate_improvements(&virtualunit, &best_act, &gx, &gy);
+
+  /* Create a localized "virtual" unit to do operations with. */
+  virtualunit = create_unit_virtual(pplayer, pcity, unit_type, FALSE);
+  virtualunit->x = pcity->x;
+  virtualunit->y = pcity->y;
+  want = evaluate_improvements(virtualunit, &best_act, &gx, &gy);
+  free(virtualunit);
+
 /* FIXME: AI does not ship F_SETTLERS around, only F_CITIES - Per */
 #ifdef AI_SMART
   unit_list_iterate(pplayer->units, qpass) {