[Freeciv-Dev] (PR#9355) initialize_infrastructure_cache is not called

["Jason Dorje Short" <jdorje@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=9355 >

The city infrastructure cache is not rebuilt when a savegame is loaded. 
  It's probably not rebuilt at other important times.  This gives rise 
to beautiful errors like

==22612== Conditional jump or move depends on uninitialised value(s)
==22612==    at 0x80AD411: consider_settler_action (settlers.c:841)
==22612==    by 0x80AE05F: evaluate_improvements (settlers.c:1117)
==22612==    by 0x80AF7B2: contemplate_terrain_improvements 
(settlers.c:1574)
==22612==    by 0x812FCC7: ai_manage_cities (aicity.c:589)
==22612==    by 0x8133BAE: ai_do_last_activities (aihand.c:386)
==22612==    by 0x804EF39: end_phase (srv_main.c:532)
==22612==    by 0x8050ACB: main_loop (srv_main.c:1468)
==22612==    by 0x805128C: srv_loop (srv_main.c:1758)
==22612==    by 0x8050C54: srv_main (srv_main.c:1545)
==22612==    by 0x804A5DE: main (civserver.c:161)

because pcity->ai.foo[i][j] has not been built.


It's built:

- When creating or transferring a city.
- When doing auto-settlers.
- For nearby cities when the AI builds a city.

It is used only in evaluate_improvements:

- When the AI manages its cities (ai_manage_cities()).
- When doing auto-settlers.


The second is already handled properly.  For the first we just need to 
rebuild the caches in ai_manage_cities.  Thus rather than try to keep 
this value updated all the time we should just make sure it's updated 
before we use it.  This patch does that.  I wanted to make 
initialize_infrastructure_cache static but that's a bit harder.

Note, the AI infrastructure cache consistes of 
pcity->ai.{detox,derad,mine,irrigate,transform,road,railroad}.

jason

Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.158
diff -u -r1.158 aicity.c
--- ai/aicity.c 12 Jul 2004 03:03:28 -0000      1.158
+++ ai/aicity.c 12 Jul 2004 20:19:08 -0000
@@ -578,6 +578,8 @@
 
   ai_manage_buildings(pplayer);
 
+  /* Initialize the infrastructure cache, which is used shortly. */
+  initialize_infrastructure_cache(pplayer);
   city_list_iterate(pplayer->cities, pcity) {
     /* Note that this function mungs the seamap, but we don't care */
     military_advisor_choose_build(pplayer, pcity, &pcity->ai.choice);
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.261
diff -u -r1.261 citytools.c
--- server/citytools.c  14 Jun 2004 23:43:08 -0000      1.261
+++ server/citytools.c  12 Jul 2004 20:19:10 -0000
@@ -1008,7 +1008,6 @@
     update_city_tile_status_map(pcity, x, y);
   } map_city_radius_iterate_end;
   auto_arrange_workers(pcity);
-  initialize_infrastructure_cache(pcity);
   if (raze)
     raze_city(pcity);
 
@@ -1131,7 +1130,6 @@
   map_clear_special(x, y, S_FORTRESS);
   send_tile_info(NULL, x, y);
 
-  initialize_infrastructure_cache(pcity);
   reset_move_costs(x, y);
 /* I stupidly thought that setting S_ROAD took care of this, but of course
 the city_id isn't set when S_ROAD is set, so reset_move_costs doesn't allow
Index: server/settlers.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.c,v
retrieving revision 1.183
diff -u -r1.183 settlers.c
--- server/settlers.c   9 Jul 2004 18:52:36 -0000       1.183
+++ server/settlers.c   12 Jul 2004 20:19:10 -0000
@@ -78,14 +78,6 @@
     return FALSE;
   }
 
-  /* initialize infrastructure cache for both this city and other cities
-     nearby. This is neccesary to avoid having settlers want to transform
-     a city into the ocean. */
-  map_city_radius_iterate(pcity->x, pcity->y, x_itr, y_itr) {
-    struct city *pcity2 = map_get_city(x_itr, y_itr);
-    if (pcity2 && city_owner(pcity2) == pplayer)
-      initialize_infrastructure_cache(pcity2);
-  } map_city_radius_iterate_end;
   return TRUE;
 }
 
@@ -1285,36 +1277,41 @@
 
 /**************************************************************************
   Do all tile improvement calculations and cache them for later.
+
+  These values are used in evaluate_improvements() so this function must
+  be called before doing that.  Currently this is only done when handling
+  auto-settlers or when the AI contemplates building worker units.
 **************************************************************************/
-void initialize_infrastructure_cache(struct city *pcity)
+void initialize_infrastructure_cache(struct player *pplayer)
 {
-  struct player *pplayer = city_owner(pcity);
-  int best = best_worker_tile_value(pcity);
+  city_list_iterate(pplayer->cities, pcity) {
+    int best = best_worker_tile_value(pcity);
+
+    city_map_iterate(cx, cy) {
+      pcity->ai.detox[cx][cy] = -1;
+      pcity->ai.derad[cx][cy] = -1;
+      pcity->ai.mine[cx][cy] = -1;
+      pcity->ai.irrigate[cx][cy] = -1;
+      pcity->ai.transform[cx][cy] = -1;
+      pcity->ai.road[cx][cy] = -1;
+      pcity->ai.railroad[cx][cy] = -1;
+    } city_map_iterate_end;
 
-  city_map_iterate(cx, cy) {
-    pcity->ai.detox[cx][cy] = -1;
-    pcity->ai.derad[cx][cy] = -1;
-    pcity->ai.mine[cx][cy] = -1;
-    pcity->ai.irrigate[cx][cy] = -1;
-    pcity->ai.transform[cx][cy] = -1;
-    pcity->ai.road[cx][cy] = -1;
-    pcity->ai.railroad[cx][cy] = -1;
-  } city_map_iterate_end;
-
-  city_map_checked_iterate(pcity->x, pcity->y, cx, cy, mx, my) {
-    pcity->ai.detox[cx][cy] = ai_calc_pollution(pcity, cx, cy, best, mx, my);
-    pcity->ai.derad[cx][cy] =
+    city_map_checked_iterate(pcity->x, pcity->y, cx, cy, mx, my) {
+      pcity->ai.detox[cx][cy] = ai_calc_pollution(pcity, cx, cy, best, mx, my);
+      pcity->ai.derad[cx][cy] =
        ai_calc_fallout(pcity, pplayer, cx, cy, best, mx, my);
-    pcity->ai.mine[cx][cy] = ai_calc_mine(pcity, cx, cy, mx, my);
-    pcity->ai.irrigate[cx][cy] =
+      pcity->ai.mine[cx][cy] = ai_calc_mine(pcity, cx, cy, mx, my);
+      pcity->ai.irrigate[cx][cy] =
        ai_calc_irrigate(pcity, pplayer, cx, cy, mx, my);
-    pcity->ai.transform[cx][cy] = ai_calc_transform(pcity, cx, cy, mx, my);
-    pcity->ai.road[cx][cy] = ai_calc_road(pcity, pplayer, cx, cy, mx, my);
-/* gonna handle road_bo dynamically for now since it can change
-as punits arrive at adjacent tiles and start laying road -- Syela */
-    pcity->ai.railroad[cx][cy] =
+      pcity->ai.transform[cx][cy] = ai_calc_transform(pcity, cx, cy, mx, my);
+      pcity->ai.road[cx][cy] = ai_calc_road(pcity, pplayer, cx, cy, mx, my);
+      /* gonna handle road_bo dynamically for now since it can change
+        as punits arrive at adjacent tiles and start laying road -- Syela */
+      pcity->ai.railroad[cx][cy] =
        ai_calc_railroad(pcity, pplayer, cx, cy, mx, my);
-  } city_map_checked_iterate_end;
+    } city_map_checked_iterate_end;
+  } city_list_iterate_end;
 }
 
 /************************************************************************** 
@@ -1327,9 +1324,8 @@
 
   t = renew_timer_start(t, TIMER_CPU, TIMER_DEBUG);
 
-  city_list_iterate(pplayer->cities, pcity)
-    initialize_infrastructure_cache(pcity); /* saves oodles of time -- Syela */
-  city_list_iterate_end;
+  /* Initialize the infrastructure cache, which is used shortly. */
+  initialize_infrastructure_cache(pplayer);
 
   pplayer->ai.warmth = WARMING_FACTOR * (game.heating > game.warminglevel ? 2 
: 1);
 
Index: server/settlers.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/settlers.h,v
retrieving revision 1.23
diff -u -r1.23 settlers.h
--- server/settlers.h   23 Sep 2003 15:59:05 -0000      1.23
+++ server/settlers.h   12 Jul 2004 20:19:10 -0000
@@ -30,7 +30,7 @@
 void init_settlers(void);
 void remove_city_from_minimap(int x, int y);
 void add_city_to_minimap(int x, int y);
-void initialize_infrastructure_cache(struct city *pcity);
+void initialize_infrastructure_cache(struct player *pplayer);
 
 void contemplate_terrain_improvements(struct city *pcity);
 void contemplate_new_city(struct city *pcity);