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

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Wed, Aug 15, 2001 at 12:11:53AM -0400, Patrick Smith wrote:
> 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.
>
> 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);
>  }

I can now confirm the problem. And it bites the CMA. I'm not sure if
Patrick's solution is a good one.

Here is what I see:
 - server sets happy[4]=4
 - city celebrates
 - city gets food according to a celebrating city
 - server sets happy[4]=3
 - the city celebrates no longer
 - server sends a city_info packet with food for a celebrating city
 and people which indicate that the city doesn't celebrate
 - client (CMA) gets the food prod per tile of the current
 (non-celebrating) city and do all calulcations based on these: BOOM

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Using only the operating-system that came with your computer is just
  like only playing the demo-disc that came with your CD-player."