[Freeciv-Dev] Re: (PR#11271) get_food_trade_shields isn't good

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Per I. Mathisen wrote:

>>In fact I think it's buggy because the caller (city_want) doesn't
>>account for specialists, and neither does get_food_trade_shields().
> 
> 
> Oh, this is bad. I am sure the code did this at one time, but it must have
> gotten lost during one of the many rewrites.

This patch should fix things.

- get_food_trade_shields is renamed as get_worked_tile_output.  It is 
made static.

- A new function in city.c is separated out: add_specialist_output. 
It's also static.

- A single function get_citizen_output() is provided to the interface. 
It returns the entire "base" output of citizen workers and specialists. 
  This function is then used by the aicity code.

-jason

Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.178
diff -u -r1.178 aicity.c
--- ai/aicity.c 30 Nov 2004 08:37:02 -0000      1.178
+++ ai/aicity.c 2 Dec 2004 19:56:20 -0000
@@ -106,35 +106,44 @@
 static inline int city_want(struct player *pplayer, struct city *acity, 
                             struct ai_data *ai)
 {
-  int want = 0, food, trade, shields, lux, sci, tax;
+  int want = 0;
+  int prod[O_COUNT], extra[O_COUNT];
 
-  get_food_trade_shields(acity, &food, &trade, &shields);
-  trade -= city_corruption(acity, trade);
-  shields -= city_waste(acity, shields);
-  get_tax_income(pplayer, trade, &sci, &lux, &tax);
+  get_citizen_output(acity, prod);
+
+  prod[O_TRADE] -= city_corruption(acity, prod[O_TRADE]);
+  prod[O_SHIELD] -= city_waste(acity, prod[O_SHIELD]);
+
+  get_tax_income(pplayer, prod[O_TRADE],
+                &extra[O_SCIENCE], &extra[O_LUXURY], &extra[O_GOLD]);
+  prod[O_SCIENCE] += extra[O_SCIENCE];
+  prod[O_LUXURY] += extra[O_LUXURY];
+  prod[O_GOLD] += extra[O_GOLD];
 
   built_impr_iterate(acity, i) {
-    tax -= improvement_upkeep(acity, i);
+    prod[O_GOLD] -= improvement_upkeep(acity, i);
   } built_impr_iterate_end;
+  /* Unit upkeep isn't handled here.  Unless we do a full city_refresh it
+   * won't be changed anyway. */
 
-  want += food * ai->food_priority;
-  if (shields != 0) {
-    want += ((shields * get_city_shield_bonus(acity)) / 100)
+  want += prod[O_FOOD] * ai->food_priority;
+  if (prod[O_SHIELD] != 0) {
+    want += ((prod[O_SHIELD] * get_city_shield_bonus(acity)) / 100)
             * ai->shield_priority;
-    want -= city_pollution(acity, shields) * ai->pollution_priority;
+    want -= city_pollution(acity, prod[O_SHIELD]) * ai->pollution_priority;
   }
-  if (lux > 0) {
-    want += ((lux * get_city_luxury_bonus(acity)) / 100)
+  if (prod[O_LUXURY] > 0) {
+    want += ((prod[O_LUXURY] * get_city_luxury_bonus(acity)) / 100)
             * ai->luxury_priority;
   }
-  if (sci > 0) {
-    want += ((sci * get_city_science_bonus(acity)) / 100)
+  if (prod[O_SCIENCE] > 0) {
+    want += ((prod[O_SCIENCE] * get_city_science_bonus(acity)) / 100)
             * ai->science_priority;
   }
-  if (tax > 0) {
-    tax *= get_city_tax_bonus(acity) / 100;
+  if (prod[O_GOLD] > 0) {
+    prod[O_GOLD] *= get_city_tax_bonus(acity) / 100;
   }
-  want += tax * ai->gold_priority;
+  want += prod[O_GOLD] * ai->gold_priority;
 
   return want;
 }
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.265
diff -u -r1.265 city.c
--- common/city.c       2 Dec 2004 10:20:30 -0000       1.265
+++ common/city.c       2 Dec 2004 19:56:21 -0000
@@ -1702,22 +1702,73 @@
   return pcity->turn_last_built + 1 >= game.turn;
 }
 
+/****************************************************************************
+  Calculate output (food, trade and shields) generated by the worked tiles
+  of a city.  This will completely overwrite the output[] array.
+****************************************************************************/
+static inline void get_worked_tile_output(const struct city *pcity,
+                                         int *output)
+{
+  bool is_celebrating = base_city_celebrating(pcity);
+
+  memset(output, 0, O_COUNT * sizeof(*output));
+  
+  city_map_iterate(x, y) {
+    if (get_worker_city(pcity, x, y) == C_TILE_WORKER) {
+      output[O_FOOD] += base_city_get_food_tile(x, y, pcity, is_celebrating);
+      output[O_SHIELD] += base_city_get_shields_tile(x, y, pcity,
+                                                    is_celebrating);
+      output[O_TRADE] += base_city_get_trade_tile(x, y, pcity,
+                                                 is_celebrating);
+    }
+  } city_map_iterate_end;
+}
+
+/****************************************************************************
+  Calculate output (gold, science, and luxury) generated by the specialists
+  of a city.  The output[] array is not cleared but is just added to.
+****************************************************************************/
+static inline void add_specialist_output(const struct city *pcity,
+                                        int *output)
+{
+  specialist_type_iterate(sp) {
+    int *bonus = game.rgame.specialists[sp].bonus;
+    int count = pcity->specialists[sp];
+
+    output[O_GOLD] += count * bonus[O_GOLD];
+    output[O_LUXURY] += count * bonus[O_LUXURY];
+    output[O_SCIENCE] += count * bonus[O_SCIENCE];
+  } specialist_type_iterate_end;
+}
+
+/****************************************************************************
+  Calculate the base output (all types) of all workers and specialists in
+  the city.  The output[] array is completely cleared in the process.  This
+  does not account for output transformations (like trade->(sci,gold,lux))
+  or city bonuses (pcity->bonus[]).
+****************************************************************************/
+void get_citizen_output(const struct city *pcity, int *output)
+{
+  get_worked_tile_output(pcity, output);
+  add_specialist_output(pcity, output);
+}
+
 /**************************************************************************
   Modify the incomes according to the taxrates and # of specialists.
 **************************************************************************/
 static inline void set_tax_income(struct city *pcity)
 {
+  int output[O_COUNT];
+
   get_tax_income(city_owner(pcity), pcity->surplus[O_TRADE], 
&pcity->science_total, 
                  &pcity->luxury_total, &pcity->tax_total);
 
-  specialist_type_iterate(sp) {
-    int *bonus = game.rgame.specialists[sp].bonus;
-    int count = pcity->specialists[sp];
 
-    pcity->luxury_total += count * bonus[O_LUXURY];
-    pcity->science_total += count * bonus[O_SCIENCE];
-    pcity->tax_total += count * bonus[O_GOLD];
-  } specialist_type_iterate_end;
+  memset(output, 0, O_COUNT * sizeof(*output));
+  add_specialist_output(pcity, output);
+  pcity->luxury_total += output[O_LUXURY];
+  pcity->science_total += output[O_SCIENCE];
+  pcity->tax_total += output[O_GOLD];
 
   pcity->tax_total += get_city_tithes_bonus(pcity);
 }
@@ -1988,38 +2039,20 @@
 }
 
 /**************************************************************************
-  Calculate food, trade and shields generated by a city, and set
-  associated variables given to us.
-**************************************************************************/
-void get_food_trade_shields(const struct city *pcity, int *food, int *trade,
-                            int *shields)
-{
-  bool is_celebrating = base_city_celebrating(pcity);
-
-  *food = 0;
-  *trade = 0;
-  *shields = 0;
-  
-  city_map_iterate(x, y) {
-    if (get_worker_city(pcity, x, y) == C_TILE_WORKER) {
-      *food += base_city_get_food_tile(x, y, pcity, is_celebrating);
-      *shields += base_city_get_shields_tile(x, y, pcity, is_celebrating);
-      *trade += base_city_get_trade_tile(x, y, pcity, is_celebrating);
-    }
-  } city_map_iterate_end;
-}
-
-/**************************************************************************
    Set food, trade and shields production in a city.
 **************************************************************************/
 static inline void set_food_trade_shields(struct city *pcity)
 {
   int i;
+  int output[O_COUNT];
+
   pcity->surplus[O_FOOD] = 0;
   pcity->surplus[O_SHIELD] = 0;
 
-  get_food_trade_shields(pcity, &pcity->food_prod, &pcity->surplus[O_TRADE],
-                         &pcity->shield_prod);
+  get_worked_tile_output(pcity, output);
+  pcity->food_prod = output[O_FOOD];
+  pcity->shield_prod = output[O_SHIELD];
+  pcity->surplus[O_TRADE] = output[O_TRADE];
   
   pcity->tile_trade = pcity->surplus[O_TRADE];
   pcity->surplus[O_FOOD] = pcity->food_prod - pcity->size * 2;
Index: common/city.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
retrieving revision 1.173
diff -u -r1.173 city.h
--- common/city.h       2 Dec 2004 10:20:30 -0000       1.173
+++ common/city.h       2 Dec 2004 19:56:21 -0000
@@ -522,8 +522,7 @@
 void city_styles_alloc(int num);
 void city_styles_free(void);
 
-void get_food_trade_shields(const struct city *pcity, int *food, int *trade,
-                            int *shields);
+void get_citizen_output(const struct city *pcity, int *output);
 void get_tax_income(struct player *pplayer, int trade, int *sci,
                     int *lux, int *tax);
 int get_city_tithes_bonus(const struct city *pcity);