[Freeciv-Dev] Bug fix: incorrect production with Hanging Gardens

[Patrick Smith <patsmith@xxxxxxxxx>]

Sometimes the food, trade, and shield totals for a city are not correct,
even though the production from each individual worker is correct.  This
happens when moving workers shifts the city between celebrating and not
celebrating.

Where I've seen this is when I have the Hanging Gardens, and am still in
a Despotism.  In a city of size 3, that has either just grown to size 3,
or that was celebrating last turn, if I click repeatedly on a tile, then
I see:

- with 3 workers, the city does not celebrate, and each worker produces
accordingly.  But the city totals are what they would be if the city
were celebrating and those three tiles were being used.

- with 2 workers, the city does celebrate, and each worker produces
accordingly.  But the city totals are what they would be if the city
were not celebrating.

I've attached a patch that fixes this.
-- 
patsmith@xxxxxxxxx

cvs server: Diffing .
cvs server: Diffing ai
cvs server: Diffing amiga
cvs server: Diffing client
cvs server: Diffing client/gui-beos
cvs server: Diffing client/gui-beos/lib
cvs server: Diffing client/gui-beos/lib/Bdh
cvs server: Diffing client/gui-beos/lib/BdhDialog
cvs server: Diffing client/gui-beos/lib/BdhGame
cvs server: Diffing client/gui-gtk
cvs server: Diffing client/gui-mui
cvs server: Diffing client/gui-stub
cvs server: Diffing client/gui-win32
cvs server: Diffing client/gui-xaw
cvs server: Diffing client/include
cvs server: Diffing common
cvs server: Diffing data
cvs server: Diffing data/civ1
cvs server: Diffing data/civ2
cvs server: Diffing data/classic
cvs server: Diffing data/default
cvs server: Diffing data/engels
cvs server: Diffing data/hires
cvs server: Diffing data/misc
cvs server: Diffing data/nation
cvs server: Diffing data/scenario
cvs server: Diffing data/trident
cvs server: Diffing debian
cvs server: Diffing doc
cvs server: Diffing gclient
cvs server: Diffing intl
cvs server: Diffing po
cvs server: Diffing server
Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.153
diff -u -r1.153 cityturn.c
--- server/cityturn.c   2001/08/13 11:09:19     1.153
+++ server/cityturn.c   2001/08/14 06:23:23
@@ -379,16 +379,60 @@
 **************************************************************************/
 int city_refresh(struct city *pcity)
 {
-  set_food_trade_shields(pcity);
-  citizen_happy_size(pcity);
-  set_tax_income(pcity);                  /* calc base luxury, tax & bulbs */
-  add_buildings_effect(pcity);            /* marketplace, library wonders.. */
-  set_pollution(pcity);
-  citizen_happy_luxury(pcity);            /* with our new found luxuries */
-  citizen_happy_buildings(pcity);         /* temple cathedral colosseum */
-  city_support(pcity);                    /* manage settlers, and units */
-  citizen_happy_wonders(pcity);           /* happy wonders */
-  unhappy_city_check(pcity);
+  /* Watch out for two complications involving the city's state (disorder,
+   * peace, celebrating) here.
+   *
+   * First, the calculations can be recursive -- the state depends on the
+   * luxury.  And in some governments the amount of trade, and hence luxury,
+   * depends on the city's state.
+   *
+   * Second, changing the number of workers or entertainers can change the
+   * city's state.  But the call to set_food_trade_shields at the beginning
+   * of this function uses the old state of the city.
+   *
+   * We handle these by checking whether the calculations done in this
+   * function have changed the city's state.  If so, we repeat the 
computations.
+   * In most cases, the city's state does not change and the loop is executed
+   * only once.
+   *
+   * In the standard game, there shouldn't be any danger of an infinite loop
+   * here.  As there are only three states, an infinite loop would have to
+   * involve changes both for the better and for the worse.  But if the state
+   * improves on one iteration, this might create more luxury, but should never
+   * reduce the amount of luxury.  So the state can't deteriorate on the next
+   * iteration.  Just to be safe, we hard code a limit on the number of
+   * times through the loop anyway.
+   */
+
+  char state = 'x';     /* Different from any valid value */
+  char old_state;
+  int iterations;       /* Don't allow too many iterations */
+
+  for (iterations = 0; iterations < 5; iterations++) {
+     old_state = state;
+
+     if (city_unhappy(pcity))
+       state = 'd';    /* Disorder */
+     else if (city_celebrating(pcity))
+       state = 'c';    /* Celebrating */
+     else
+       state = 'p';    /* Peace */
+
+     if (state == old_state)
+       break;
+
+     set_food_trade_shields(pcity);
+     citizen_happy_size(pcity);
+     set_tax_income(pcity);            /* calc base luxury, tax & bulbs */
+     add_buildings_effect(pcity);      /* marketplace, library wonders.. */
+     set_pollution(pcity);
+     citizen_happy_luxury(pcity);      /* with our new found luxuries */
+     citizen_happy_buildings(pcity);   /* temple cathedral colosseum */
+     city_support(pcity);              /* manage settlers, and units */
+     citizen_happy_wonders(pcity);     /* happy wonders */
+     unhappy_city_check(pcity);
+  }
+
   return (city_happy(pcity) && pcity->was_happy);
 }
 
cvs server: Diffing vms