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

["Per Inge Mathisen" <pim@xxxxxxxxxxxx>]

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

On Wed, 8 Dec 2004, Jason Short wrote:
> > However, CMA will not work with this patch. I have no idea why.
>
> Looks like the error is in set_tax_income:
>
> +pcity->luxury_total += output[O_LUXURY];
> +pcity->science_total += output[O_SCIENCE];
> +pcity->tax_total += output[O_GOLD];
>
> should be = not +=.

Ah. No, they should be +=, but we need to clear these fields in one place
that I forgot. New patch attached.

  - Per

Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.180
diff -u -r1.180 aicity.c
--- ai/aicity.c 8 Dec 2004 07:48:24 -0000       1.180
+++ ai/aicity.c 8 Dec 2004 21:56:23 -0000
@@ -106,35 +106,38 @@
 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, prod[O_COUNT];
 
-  get_food_trade_shields(acity, &food, &trade, &shields);
-  trade -= city_waste(acity, O_TRADE, trade);
-  shields -= city_waste(acity, O_SHIELD, shields);
-  get_tax_income(pplayer, trade, &sci, &lux, &tax);
+  add_citizen_output(acity, prod); /* this also clears prod[] */
+  output_type_iterate(o) {
+    prod[o] -= city_waste(acity, o, prod[o]);
+  } output_type_iterate_end;
+  add_tax_income(pplayer, prod);
 
   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_output_bonus(acity, O_SHIELD)) / 100)
+  want += prod[O_FOOD] * ai->food_priority;
+  if (prod[O_SHIELD] != 0) {
+    want += ((prod[O_SHIELD] * get_city_output_bonus(acity, O_SHIELD)) / 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_output_bonus(acity, O_LUXURY)) / 100)
+  if (prod[O_LUXURY] > 0) {
+    want += ((prod[O_LUXURY] * get_city_output_bonus(acity, O_LUXURY)) / 100)
             * ai->luxury_priority;
   }
-  if (sci > 0) {
-    want += ((sci * get_city_output_bonus(acity, O_SCIENCE)) / 100)
+  if (prod[O_SCIENCE] > 0) {
+    want += ((prod[O_SCIENCE] * get_city_output_bonus(acity, O_SCIENCE)) / 100)
             * ai->science_priority;
   }
-  if (tax > 0) {
-    tax *= get_city_output_bonus(acity, O_GOLD) / 100;
+  if (prod[O_GOLD] > 0) {
+    prod[O_GOLD] *= get_city_output_bonus(acity, O_GOLD) / 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.275
diff -u -r1.275 city.c
--- common/city.c       8 Dec 2004 07:48:24 -0000       1.275
+++ common/city.c       8 Dec 2004 21:56:24 -0000
@@ -1646,11 +1646,10 @@
 }
 
 /**************************************************************************
-  Get the incomes of a city according to the taxrates (ignore # of 
+  Add the incomes of a city according to the taxrates (ignore # of 
   specialists). trade should usually be pcity->surplus[O_TRADE].
 **************************************************************************/
-void get_tax_income(struct player *pplayer, int trade, int *sci, 
-                    int *lux, int *tax)
+void add_tax_income(const struct player *pplayer, int *output)
 {
   const int SCIENCE = 0, TAX = 1, LUXURY = 2;
   int rates[3], result[3];
@@ -1672,13 +1671,11 @@
     rates[TAX] = 0;
   }
 
-  distribute(trade, 3, rates, result);
+  distribute(output[O_TRADE], 3, rates, result);
 
-  *sci = result[SCIENCE];
-  *tax = result[TAX];
-  *lux = result[LUXURY];
-
-  assert(*sci + *tax + *lux == trade);
+  output[O_SCIENCE] += result[SCIENCE];
+  output[O_GOLD] += result[TAX];
+  output[O_LUXURY] += result[LUXURY];
 }
 
 /**************************************************************************
@@ -1690,22 +1687,74 @@
   return pcity->turn_last_built + 1 >= game.turn;
 }
 
-/**************************************************************************
-  Modify the incomes according to the taxrates and # of specialists.
-**************************************************************************/
-static inline void set_tax_income(struct city *pcity)
+/****************************************************************************
+  Calculate output (food, trade and shields) generated by the worked tiles
+  of a city.  This will completely overwrite the output[] array.
+****************************************************************************/
+static inline void add_worked_tile_output(const struct city *pcity,
+                                         int *output)
 {
-  get_tax_income(city_owner(pcity), pcity->surplus[O_TRADE], 
&pcity->science_total, 
-                 &pcity->luxury_total, &pcity->tax_total);
+  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];
 
-    pcity->luxury_total += count * bonus[O_LUXURY];
-    pcity->science_total += count * bonus[O_SCIENCE];
-    pcity->tax_total += count * bonus[O_GOLD];
+    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 add_citizen_output(const struct city *pcity, int *output)
+{
+  add_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];
+
+  memset(output, 0, O_COUNT * sizeof(*output));
+  pcity->luxury_total = 0;
+  pcity->science_total = 0;
+  pcity->tax_total = 0;
+  output[O_TRADE] = pcity->surplus[O_TRADE];
+  add_tax_income(city_owner(pcity), 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);
 }
@@ -1975,38 +2024,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);
+  add_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.177
diff -u -r1.177 city.h
--- common/city.h       8 Dec 2004 07:48:24 -0000       1.177
+++ common/city.h       8 Dec 2004 21:56:24 -0000
@@ -521,10 +521,8 @@
 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_tax_income(struct player *pplayer, int trade, int *sci,
-                    int *lux, int *tax);
+void add_citizen_output(const struct city *pcity, int *output);
+void add_tax_income(const struct player *pplayer, int *output);
 int get_city_tithes_bonus(const struct city *pcity);
 int city_pollution(struct city *pcity, int shield_total);
 
Index: common/government.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/government.h,v
retrieving revision 1.36
diff -u -r1.36 government.h
--- common/government.h 6 Dec 2004 17:54:39 -0000       1.36
+++ common/government.h 8 Dec 2004 21:56:24 -0000
@@ -149,7 +149,7 @@
    and techs before that are guaranteed to exist */
 
 struct government *get_government(int gov);
-struct government *get_gov_pplayer(struct player *pplayer);
+struct government *get_gov_pplayer(const struct player *pplayer);
 struct government *get_gov_pcity(const struct city *pcity);
 
 struct government *find_government_by_name(const char *name);
Index: common/government.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/government.c,v
retrieving revision 1.47
diff -u -r1.47 government.c
--- common/government.c 10 Nov 2004 17:01:59 -0000      1.47
+++ common/government.c 8 Dec 2004 21:56:24 -0000
@@ -139,7 +139,7 @@
 /***************************************************************
 ...
 ***************************************************************/
-struct government *get_gov_pplayer(struct player *pplayer)
+struct government *get_gov_pplayer(const struct player *pplayer)
 {
   assert(pplayer != NULL);
   return get_government(pplayer->government);