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

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

And, here's an updated/fixed patch.

-jason

? freeciv-2.0.99-devel.tar.gz
Index: ai/aicity.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/ai/aicity.c,v
retrieving revision 1.194
diff -u -r1.194 aicity.c
--- ai/aicity.c 14 Feb 2005 21:47:35 -0000      1.194
+++ ai/aicity.c 17 Feb 2005 21:47:09 -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: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.60
diff -u -r1.60 citydlg_common.c
--- client/citydlg_common.c     4 Feb 2005 02:12:32 -0000       1.60
+++ client/citydlg_common.c     17 Feb 2005 21:47:10 -0000
@@ -408,9 +408,7 @@
 
   /* Hack to get around the ugliness of add_tax_income. */
   memset(tax, 0, O_COUNT * sizeof(*tax));
-  tax[O_TRADE] = pcity->prod[O_TRADE];
-  add_tax_income(city_owner(pcity), tax);
-  tax[O_TRADE] = 0;
+  add_tax_income(city_owner(pcity), pcity->prod[O_TRADE], tax);
   if (tax[otype] != 0) {
     cat_snprintf(buf, bufsz, _("%+4d : Taxed from trade\n"), tax[otype]);
     total += tax[otype];
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.314
diff -u -r1.314 city.c
--- common/city.c       10 Feb 2005 17:55:08 -0000      1.314
+++ common/city.c       17 Feb 2005 21:47:11 -0000
@@ -1628,7 +1628,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];
@@ -1650,7 +1650,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];
@@ -2053,6 +2053,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;
@@ -2065,25 +2078,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.201
diff -u -r1.201 city.h
--- common/city.h       10 Feb 2005 17:55:08 -0000      1.201
+++ common/city.h       17 Feb 2005 21:47:11 -0000
@@ -516,7 +516,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_types(const struct city *pcity, int shield_total,
                         int *pollu_prod, int *pollu_pop, int *pollu_mod);