[Freeciv-Dev] Re: (PR#9963) bug in waste code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Per I. Mathisen wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=9963 >
> 
> In common/city.c, we multiply shield production after we call
> city_waste(). This should of course be done before we call city_waste(),
> preferably inside get_food_trade_shields().

Here's a patch.  This also fixes a related problem with trade bonus and 
trade waste and taxed trade.

Per, you should look at the aicity.c bits.

-jason

? gmon.out
Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.189
diff -u -r1.189 aicity.c
--- ai/aicity.c 7 Jan 2005 02:56:27 -0000       1.189
+++ ai/aicity.c 10 Jan 2005 05:58:44 -0000
@@ -133,13 +133,24 @@
 static inline int city_want(struct player *pplayer, struct city *acity, 
                             struct ai_data *ai)
 {
-  int want = 0, prod[O_COUNT];
+  int want = 0, prod[O_COUNT], bonus[O_COUNT], waste[O_COUNT], i;
 
+  /* The below calculation mostly duplicates set_city_production(). */
   get_citizen_output(acity, prod); /* this also clears prod[] */
+  for (i = 0; i < NUM_TRADEROUTES; i++) {
+    prod[O_TRADE] += acity->trade_value[i];
+  }
+  prod[O_GOLD] += get_city_tithes_bonus(acity);
+  output_type_iterate(o) {
+    bonus[o] = get_city_output_bonus(acity, o);
+    waste[o] = city_waste(acity, o, prod[o] * bonus[o] / 100);
+  } output_type_iterate_end;
+  add_tax_income(pplayer,
+                prod[O_TRADE] * bonus[O_TRADE] / 100 - waste[O_TRADE],
+                prod);
   output_type_iterate(o) {
-    prod[o] -= city_waste(acity, o, prod[o]);
+    prod[o] = prod[o] * bonus[o] / 100 - waste[o];
   } output_type_iterate_end;
-  add_tax_income(pplayer, prod);
 
   built_impr_iterate(acity, i) {
     prod[O_GOLD] -= improvement_upkeep(acity, i);
@@ -149,21 +160,11 @@
 
   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 += prod[O_SHIELD] * ai->shield_priority;
     want -= city_pollution(acity, prod[O_SHIELD]) * ai->pollution_priority;
   }
-  if (prod[O_LUXURY] > 0) {
-    want += ((prod[O_LUXURY] * get_city_output_bonus(acity, O_LUXURY)) / 100)
-            * ai->luxury_priority;
-  }
-  if (prod[O_SCIENCE] > 0) {
-    want += ((prod[O_SCIENCE] * get_city_output_bonus(acity, O_SCIENCE)) / 100)
-            * ai->science_priority;
-  }
-  if (prod[O_GOLD] > 0) {
-    prod[O_GOLD] *= get_city_output_bonus(acity, O_GOLD) / 100;
-  }
+  want += prod[O_LUXURY] * ai->luxury_priority;
+  want += prod[O_SCIENCE] * ai->science_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.303
diff -u -r1.303 city.c
--- common/city.c       8 Jan 2005 09:44:39 -0000       1.303
+++ common/city.c       10 Jan 2005 05:58:45 -0000
@@ -1604,7 +1604,7 @@
   Add the incomes of a city according to the taxrates (ignore # of 
   specialists). trade should be in output[O_TRADE].
 **************************************************************************/
-void add_tax_income(const struct player *pplayer, int *output)
+void add_tax_income(const struct player *pplayer, int trade, int *output)
 {
   const int SCIENCE = 0, TAX = 1, LUXURY = 2;
   int rates[3], result[3];
@@ -1626,7 +1626,7 @@
     rates[TAX] = 0;
   }
 
-  distribute(output[O_TRADE], 3, rates, result);
+  distribute(trade, 3, rates, result);
 
   output[O_SCIENCE] += result[SCIENCE];
   output[O_GOLD] += result[TAX];
@@ -1987,6 +1987,19 @@
 {
   int i;
 
+  /* Calculate city production!
+   *
+   * This is a rather complicated process if we allow rules to become
+   * more generalized.  We can assume that there are no recursive dependency
+   * loops, but there are some dependencies that do not follow strict
+   * ordering.  For instance corruption must be calculated before 
+   * trade taxes can be counted up, which must occur before the science bonus
+   * is added on.  But the calculation of corruption must include the
+   * trade bonus.  To do this without excessive special casing means that in
+   * this case the bonuses are multiplied on twice (but only saved the second
+   * time).
+   */
+
   output_type_iterate(o) {
     pcity->prod[o] = pcity->citizen_base[o];
   } output_type_iterate_end;
@@ -1999,25 +2012,27 @@
   }
   pcity->prod[O_GOLD] += get_city_tithes_bonus(pcity);
 
-  /* Account for waste.  Waste is only calculated for trade and shields.
-   * Note that waste is calculated BEFORE tax incomes and BEFORE effects
-   * bonuses are included.  This means that shield-waste does not include
-   * the shield-bonus from factories, which is surely a bug. */
-  pcity->waste[O_TRADE] = city_waste(pcity, O_TRADE, pcity->prod[O_TRADE]);
-  pcity->prod[O_TRADE] -= pcity->waste[O_TRADE];
-  pcity->waste[O_SHIELD] = city_waste(pcity, O_SHIELD, pcity->prod[O_SHIELD]);
-  pcity->prod[O_SHIELD] -= pcity->waste[O_SHIELD];
+  /* Account for waste.  Note that waste is calculated before tax income is
+   * calculated, so if you had "science waste" it would not include taxed
+   * science.  However waste is calculated after the bonuses are multiplied
+   * on, so shield waste will include shield bonuses. */
+  output_type_iterate(o) {
+    pcity->waste[o] = city_waste(pcity, o,
+                                pcity->prod[o] * pcity->bonus[o] / 100);
+  } output_type_iterate_end;
 
   /* Convert trade into science/luxury/gold, and add this on to whatever
    * science/luxury/gold is already there. */
-  add_tax_income(city_owner(pcity), pcity->prod);
+  add_tax_income(city_owner(pcity),
+                pcity->prod[O_TRADE] * pcity->bonus[O_TRADE] / 100
+                - pcity->waste[O_TRADE] - pcity->usage[O_TRADE],
+                pcity->prod);
 
-  /* Add on effect bonuses.  Note that this means the waste and tax income
-   * above does NOT include the bonuses.  This works for the default
-   * ruleset but won't work if there is shield-waste or if there were
-   * a trade bonus. */
+  /* Add on effect bonuses and waste.  Note that the waste calculation
+   * (above) already includes the bonus multiplier. */
   output_type_iterate(o) {
     pcity->prod[o] = pcity->prod[o] * pcity->bonus[o] / 100;
+    pcity->prod[o] -= pcity->waste[o];
   } output_type_iterate_end;
 }
 
Index: common/city.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
retrieving revision 1.193
diff -u -r1.193 city.h
--- common/city.h       8 Jan 2005 05:14:17 -0000       1.193
+++ common/city.h       10 Jan 2005 05:58:45 -0000
@@ -519,7 +519,7 @@
 void city_styles_free(void);
 
 void get_citizen_output(const struct city *pcity, int *output);
-void add_tax_income(const struct player *pplayer, int *output);
+void add_tax_income(const struct player *pplayer, int trade, int *output);
 int get_city_tithes_bonus(const struct city *pcity);
 int city_pollution(const struct city *pcity, int shield_total);