[freeciv-ai] (PR#10290) rfc: city production circuit

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Here is my very own city caching patch.

The changes in generic_city_refresh are simple.  set_city_bonuses and
city_support are only called for a full refresh.  However this makes
other simplifications possible.  I removed the send_unit_info parameter
(which makes this function very hard to use IMO) and replaced it with a
pointer in game.callbacks.  This makes things less bug-prone because in
the current system if there's an unrefreshed city that (for instance)
the AI calls a refresh on without send_unit_info, then we get a bug.

Autogames are identical and become marginally faster under the patch
(the autogame I ran only went through year 0 so CM runtime probably was
only a small % of total runtime).

-jason

? gmon.out
? new
? orig
Index: ai/aihand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aihand.c,v
retrieving revision 1.97
diff -u -r1.97 aihand.c
--- ai/aihand.c 15 Dec 2004 00:15:12 -0000      1.97
+++ ai/aihand.c 27 Dec 2004 19:50:00 -0000
@@ -186,7 +186,7 @@
           cm_query_result(pcity, &cmp, &cmr);
           if (cmr.found_a_valid) {
             apply_cmresult_to_city(pcity, &cmr);
-            generic_city_refresh(pcity, TRUE, NULL);
+            generic_city_refresh(pcity, TRUE);
             if (!city_happy(pcity)) {
               CITY_LOG(LOG_ERROR, pcity, "is NOT happy when it should be!");
             }
@@ -265,7 +265,7 @@
       city_list_iterate(pplayer->cities, acity) {
         acity->ai.celebrate = FALSE;
        /* This isn't strictly necessary since it's done in aaw. */
-        generic_city_refresh(acity, TRUE, NULL);
+        generic_city_refresh(acity, TRUE);
         auto_arrange_workers(acity);
       } city_list_iterate_end;
       city_list_iterate(pplayer->cities, pcity) {
@@ -308,7 +308,7 @@
     pplayer->government = current_gov;
     city_list_iterate(pplayer->cities, acity) {
       /* This isn't strictly necessary since it's done in aaw. */
-      generic_city_refresh(acity, TRUE, NULL);
+      generic_city_refresh(acity, TRUE);
       auto_arrange_workers(acity);
     } city_list_iterate_end;
     ai->govt_reeval = CLIP(5, city_list_size(&pplayer->cities), 20);
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.295
diff -u -r1.295 city.c
--- common/city.c       25 Dec 2004 20:13:05 -0000      1.295
+++ common/city.c       27 Dec 2004 19:50:00 -0000
@@ -2060,9 +2060,7 @@
   Calculate upkeep costs.  This builds the pcity->usage[] array as well
   as setting some happiness values.
 **************************************************************************/
-static inline void city_support(struct city *pcity, 
-                               void (*send_unit_info) (struct player *pplayer,
-                                                       struct unit *punit))
+static inline void city_support(struct city *pcity)
 {
   struct government *g = get_gov_pcity(pcity);
 
@@ -2168,36 +2166,49 @@
     } output_type_iterate_end;
 
     /* Send unit info if anything has changed */
-    if (send_unit_info && changed) {
-      send_unit_info(unit_owner(this_unit), this_unit);
+    if (game.callbacks.send_unit_info && changed) {
+      game.callbacks.send_unit_info(unit_owner(this_unit), this_unit);
     }
   } unit_list_iterate_end;
 }
 
 /**************************************************************************
-...
+  Does a refresh of the city.  This fills out most of the cached values
+  in the city structure from the underlying data.
+
+  pcity gives the city to be refreshed.
+
+  full_refresh tells whether a full refresh is needed.  There are two
+  levels of cached data.  The lower level takes longer to calculate and is
+  only recalculated on a full refresh; the upper level is always
+  recalculated.
+
+    The lower level includes city effect bonus and support values.  When in
+    doubt do a full refresh.  This doesn't need to be recalculated when
+    moving workers around or when a trade route changes.
+
+    The upper level includes city output and happiness values.
 **************************************************************************/
-void generic_city_refresh(struct city *pcity,
-                         bool refresh_trade_route_cities,
-                         void (*send_unit_info) (struct player * pplayer,
-                                                 struct unit * punit))
+void generic_city_refresh(struct city *pcity, bool full_refresh)
 {
   int prev_tile_trade = pcity->citizen_base[O_TRADE];
 
-  set_city_bonuses(pcity);     /* Calculate the bonus[] array values. */
+  if (full_refresh) {
+    set_city_bonuses(pcity);
+    city_support(pcity);
+  }
   get_citizen_output(pcity, pcity->citizen_base); /* Calculate output from 
citizens. */
   set_city_production(pcity);
   citizen_happy_size(pcity);
   pcity->pollution = city_pollution(pcity, pcity->prod[O_SHIELD]);
   citizen_happy_luxury(pcity); /* with our new found luxuries */
   citizen_content_buildings(pcity);    /* temple cathedral colosseum */
-  city_support(pcity, send_unit_info); /* manage settlers, and units */
   citizen_happy_units(pcity); /* Martial law & unrest from units */
   citizen_happy_wonders(pcity);        /* happy wonders & fundamentalism */
   unhappy_city_check(pcity);
   set_surpluses(pcity);
 
-  if (refresh_trade_route_cities
+  if (full_refresh
       && pcity->citizen_base[O_TRADE] != prev_tile_trade) {
     int i;
 
@@ -2205,7 +2216,7 @@
       struct city *pcity2 = find_city_by_id(pcity->trade[i]);
 
       if (pcity2) {
-       generic_city_refresh(pcity2, FALSE, send_unit_info);
+       generic_city_refresh(pcity2, FALSE);
       }
     }
   }
Index: common/city.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
retrieving revision 1.188
diff -u -r1.188 city.h
--- common/city.h       25 Dec 2004 20:13:05 -0000      1.188
+++ common/city.h       27 Dec 2004 19:50:00 -0000
@@ -500,10 +500,7 @@
 void city_remove_improvement(struct city *pcity,Impr_Type_id impr);
 
 /* city update functions */
-void generic_city_refresh(struct city *pcity,
-                         bool refresh_trade_route_cities,
-                         void (*send_unit_info) (struct player * pplayer,
-                                                 struct unit * punit));
+void generic_city_refresh(struct city *pcity, bool full_refresh);
 void adjust_city_free_cost(int *num_free, int *this_cost);
 int city_waste(const struct city *pcity, Output_type_id otype, int total);
 int city_specialists(const struct city *pcity);                 /* 
elv+tax+scie */
Index: common/game.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/game.h,v
retrieving revision 1.168
diff -u -r1.168 game.h
--- common/game.h       24 Dec 2004 16:03:19 -0000      1.168
+++ common/game.h       27 Dec 2004 19:50:01 -0000
@@ -245,6 +245,10 @@
   struct {
     /* Function to be called in game_remove_unit when a unit is deleted. */
     void (*unit_deallocate)(int unit_id);
+
+    /* Function to be called in generic_city_refresh when a unit is
+     * changed. */
+    void (*send_unit_info)(struct player *pplayer, struct unit *punit);
   } callbacks;
 };
 
Index: common/aicore/cm.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/aicore/cm.c,v
retrieving revision 1.55
diff -u -r1.55 cm.c
--- common/aicore/cm.c  15 Dec 2004 00:15:12 -0000      1.55
+++ common/aicore/cm.c  27 Dec 2004 19:50:01 -0000
@@ -671,7 +671,7 @@
   }
 
   /* Finally we must refresh the city to reset all the precomputed fields. */
-  generic_city_refresh(pcity, FALSE, 0);
+  generic_city_refresh(pcity, FALSE);
   assert(sumworkers == pcity->size);
 }
 
@@ -1522,7 +1522,7 @@
   /* make sure the city's numbers make sense (sometimes they don't,
    * somehow) */
   memcpy(&backup, pcity, sizeof(*pcity));
-  generic_city_refresh(pcity, FALSE, NULL);
+  generic_city_refresh(pcity, FALSE);
 
   memset(state->min_production, 0, sizeof(state->min_production));
 
@@ -1802,7 +1802,7 @@
   /* Refresh the city.  Otherwise the CM can give wrong results or just be
    * slower than necessary.  Note that cities are often passed in in an
    * unrefreshed state (which should probably be fixed). */
-  generic_city_refresh(pcity, TRUE, NULL);
+  generic_city_refresh(pcity, TRUE);
 
   cm_find_best_solution(state, param, result);
   cm_free_state(state);
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.290
diff -u -r1.290 cityturn.c
--- server/cityturn.c   21 Dec 2004 22:57:36 -0000      1.290
+++ server/cityturn.c   27 Dec 2004 19:50:03 -0000
@@ -84,7 +84,7 @@
 **************************************************************************/
 void city_refresh(struct city *pcity)
 {
-   generic_city_refresh(pcity, TRUE, send_unit_info);
+   generic_city_refresh(pcity, TRUE);
    /* AI would calculate this 1000 times otherwise; better to do it
       once -- Syela */
    pcity->ai.trade_want
Index: server/sanitycheck.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/sanitycheck.c,v
retrieving revision 1.57
diff -u -r1.57 sanitycheck.c
--- server/sanitycheck.c        16 Dec 2004 19:45:18 -0000      1.57
+++ server/sanitycheck.c        27 Dec 2004 19:50:03 -0000
@@ -281,7 +281,7 @@
       }
     }
 
-    generic_city_refresh(pcity, TRUE, NULL);
+    generic_city_refresh(pcity, TRUE);
   }
 }
 
Index: server/savegame.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/savegame.c,v
retrieving revision 1.215
diff -u -r1.215 savegame.c
--- server/savegame.c   19 Dec 2004 16:47:10 -0000      1.215
+++ server/savegame.c   27 Dec 2004 19:50:04 -0000
@@ -3461,14 +3461,9 @@
 
     /* Update all city information.  This must come after all cities are
      * loaded (in player_load) but before player (dumb) cities are loaded
-     * (in player_map_load).  Cities are refreshed twice to account for
-     * trade routes: the first refresh initializes all cities tile_trade
-     * values; the second correctly updates all trade routes. */
+     * (in player_map_load). */
     cities_iterate(pcity) {
-      generic_city_refresh(pcity, FALSE, NULL);
-    } cities_iterate_end;
-    cities_iterate(pcity) {
-      generic_city_refresh(pcity, FALSE, NULL);
+      generic_city_refresh(pcity, TRUE);
     } cities_iterate_end;
 
     /* Since the cities must be placed on the map to put them on the
Index: server/srv_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/srv_main.c,v
retrieving revision 1.219
diff -u -r1.219 srv_main.c
--- server/srv_main.c   19 Dec 2004 16:47:10 -0000      1.219
+++ server/srv_main.c   27 Dec 2004 19:50:04 -0000
@@ -206,6 +206,7 @@
 
   /* Initialize callbacks. */
   game.callbacks.unit_deallocate = dealloc_id;
+  game.callbacks.send_unit_info = send_unit_info;
 
   /* done */
   return;