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

I was looking to test some experimental code, and needed to fix the
generic_city_refresh() mess, where refreshing one city can lead to the
side effects of units being changed and other cities being changed and
refreshed in turn. Not to mention passing in a function point to send
out network data. So I came up with the following patch that makes
generic_city_refresh() entirely without side effects, and as long as
it is called with different cities, it should also be re-entrant.

The big change is that trade routes are no longer calculated from the
trade output, but rather from city size and real map distance. This
means there are now very few cases where trade routes actually change,
as opposed to now, where any city operation is likely to impact trade
routes and cause changes in several cities.

It also makes the city code make use of the "amount" field of
EFT_MAKE_CONTENT_MIL_PER which is now ignored, and "FieldUnit" units
now cause happy upkeep amount of unhappiness when in a city instead of
1, which is how you would expect it to work.

On the code side, this fixes some ugliness in savegame.c where we
would update cities' trade route info when some trade partners may yet
be uninitialized (see PR#12498). Since we no longer send upkeep by the
unit to the client, but rather require the client to calculate it on
its own, the amount of network data transmitted has been reduced, at
the cost of some more client computation.

The only "problem" with this is that the server's idea of "which unit
is causing unhappiness" and "which unit is paying upkeep" may differ
from the client's. However, the idea of appointing certain units as
paying and causing unhappiness makes no sense in the first place,
since units falling in under free quotas are just as much to blame for
the upkeep as are those that are randomly appointed as paying the
cost. It only makes a bit of difference to players who may think they
know which units the server will disband if you run out of gold to pay
for maintenance. I actually look at this as an accidential feature ;-)

  - Per

PS If you want to only check out the trade route change, I attached
this as a separate patch.

Index: server/cityturn.c
===================================================================
--- server/cityturn.c	(revision 12804)
+++ server/cityturn.c	(working copy)
@@ -88,7 +88,7 @@
 **************************************************************************/
 void city_refresh(struct city *pcity)
 {
-   generic_city_refresh(pcity, TRUE, send_unit_info);
+   generic_city_refresh(pcity, TRUE);
    /* AI would calculate this 1000 times otherwise; better to do it
       once -- Syela */
    pcity->ai.trade_want
@@ -389,6 +389,8 @@
 **************************************************************************/
 bool city_reduce_size(struct city *pcity, int pop_loss)
 {
+  int i;
+
   if (pop_loss == 0) {
     return TRUE;
   }
@@ -436,6 +438,16 @@
     auto_arrange_workers(pcity);
     sync_cities();
   }
+
+  /* Update cities that have trade routes with us */
+  for (i = 0; i < NUM_TRADEROUTES; i++) {
+    struct city *pcity2 = find_city_by_id(pcity->trade[i]);
+
+    if (pcity2) {
+      city_refresh(pcity2);
+    }
+  }
+
   sanity_check_city(pcity);
   return TRUE;
 }
@@ -460,7 +472,7 @@
 {
   struct player *powner = city_owner(pcity);
   bool have_square;
-  int savings_pct = granary_savings(pcity), new_food;
+  int i, savings_pct = granary_savings(pcity), new_food;
   bool rapture_grow = city_rapture_grow(pcity); /* check before size increase! */
 
   if (!city_can_grow_to(pcity, pcity->size + 1)) { /* need improvement */
@@ -514,6 +526,15 @@
 
   city_refresh(pcity);
 
+  /* Update cities that have trade routes with us */
+  for (i = 0; i < NUM_TRADEROUTES; i++) {
+    struct city *pcity2 = find_city_by_id(pcity->trade[i]);
+
+    if (pcity2) {
+      city_refresh(pcity2);
+    }
+  }
+
   notify_player(powner, pcity->tile, E_CITY_GROWTH,
                    _("%s grows to size %d."), pcity->name, pcity->size);
   script_signal_emit("city_growth", 2,
Index: server/unittools.c
===================================================================
--- server/unittools.c	(revision 12804)
+++ server/unittools.c	(working copy)
@@ -274,14 +274,22 @@
 void pay_for_units(struct player *pplayer, struct city *pcity)
 {
   int potential_gold = 0;
+  int free_upkeep[O_COUNT];
 
+  memset(free_upkeep, 0, O_COUNT * sizeof(*free_upkeep));
+  free_upkeep[O_GOLD] = get_city_output_bonus(pcity, get_output_type(O_GOLD),
+                                              EFT_UNIT_UPKEEP_FREE_PER_CITY);
+
   built_impr_iterate(pcity, pimpr) {
     potential_gold += impr_sell_gold(pimpr);
   } built_impr_iterate_end;
 
   unit_list_iterate_safe(pcity->units_supported, punit) {
+    int upkeep[O_COUNT];
 
-    if (pplayer->economic.gold + potential_gold < punit->upkeep[O_GOLD]) {
+    city_unit_upkeep(punit, upkeep, free_upkeep);
+
+    if (pplayer->economic.gold + potential_gold < upkeep[O_GOLD]) {
       /* We cannot upkeep this unit any longer and selling off city
        * improvements will not help so we will have to disband */
       assert(pplayer->economic.gold + potential_gold >= 0);
@@ -294,7 +302,7 @@
       /* Gold can get negative here as city improvements will be sold
        * afterwards to balance our budget. FIXME: Should units with gold 
        * upkeep give gold when they are disbanded? */
-      pplayer->economic.gold -= punit->upkeep[O_GOLD];
+      pplayer->economic.gold -= upkeep[O_GOLD];
     }
   } unit_list_iterate_safe_end;
 }
@@ -1777,10 +1785,6 @@
   packet->hp = punit->hp;
   packet->activity = punit->activity;
   packet->activity_count = punit->activity_count;
-  packet->unhappiness = punit->unhappiness;
-  output_type_iterate(o) {
-    packet->upkeep[o] = punit->upkeep[o];
-  } output_type_iterate_end;
   packet->ai = punit->ai.control;
   packet->fuel = punit->fuel;
   if (punit->goto_tile) {
Index: server/sanitycheck.c
===================================================================
--- server/sanitycheck.c	(revision 12804)
+++ server/sanitycheck.c	(working copy)
@@ -345,7 +345,7 @@
       }
     }
 
-    generic_city_refresh(pcity, TRUE, NULL);
+    generic_city_refresh(pcity, TRUE);
   }
 }
 
Index: server/savegame.c
===================================================================
--- server/savegame.c	(revision 12804)
+++ server/savegame.c	(working copy)
@@ -3890,21 +3890,11 @@
       } players_iterate_end;
     } players_iterate_end;
 
-
     /* Update all city information.  This must come after all cities are
      * loaded (in player_load) but before player (dumb) cities are loaded
-     * (in player_map_load).
-     *
-     * This is a bit ugly since generic_city_refresh assumes a city that
-     * has already been refreshed at some point (even if it's out of date).
-     * Here we follow the simplest method of just refreshing all cities,
-     * and updating trade routes at the same time.  This could lead to memory
-     * issues because the refresh might look at some data of another city
-     * that hasn't itself been refreshed yet.  However this shouldn't cause
-     * any problems because in the end all cities are refreshed once (or
-     * more) and recursive dependencies are all taken care of. */
+     * (in player_map_load). */
     cities_iterate(pcity) {
-      generic_city_refresh(pcity, TRUE, NULL);
+      generic_city_refresh(pcity, TRUE);
     } cities_iterate_end;
 
     /* Since the cities must be placed on the map to put them on the
Index: common/unit.c
===================================================================
--- common/unit.c	(revision 12804)
+++ common/unit.c	(working copy)
@@ -1263,8 +1263,6 @@
   }
   punit->goto_tile = NULL;
   punit->veteran = veteran_level;
-  memset(punit->upkeep, 0, O_COUNT * sizeof(*punit->upkeep));
-  punit->unhappiness = 0;
   /* A unit new and fresh ... */
   punit->foul = FALSE;
   punit->debug = FALSE;
Index: common/unit.h
===================================================================
--- common/unit.h	(revision 12804)
+++ common/unit.h	(working copy)
@@ -130,8 +130,6 @@
   int moves_left;
   int hp;
   int veteran;
-  int unhappiness;
-  int upkeep[O_MAX];
   int fuel;
   int birth_turn;
   int bribe_cost;
Index: common/packets.def
===================================================================
--- common/packets.def	(revision 12804)
+++ common/packets.def	(working copy)
@@ -734,7 +734,7 @@
   UNIT_TYPE type;
   UNIT transported_by; /* Only valid if transported is set. */
   UINT8 movesleft, hp, fuel, activity_count;
-  UINT8 unhappiness, upkeep[O_MAX], occupy;
+  UINT8 occupy;
   COORD goto_dest_x,goto_dest_y;
   ACTIVITY activity;
   SPECIAL activity_target;
Index: common/city.c
===================================================================
--- common/city.c	(revision 12804)
+++ common/city.c	(working copy)
@@ -868,8 +868,8 @@
   int bonus = 0;
 
   if (pc1 && pc2) {
-    bonus = (pc1->citizen_base[O_TRADE]
-	     + pc2->citizen_base[O_TRADE] + 4) / 8;
+    bonus = real_map_distance(pc1->tile, pc2->tile) + pc1->size + pc2->size;
+    bonus /= 8;
 
     /* Double if on different continents. */
     if (tile_get_continent(pc1->tile) != tile_get_continent(pc2->tile)) {
@@ -957,20 +957,6 @@
   return cost;
 }
 
-/*************************************************************************
-  Calculate how much is needed to pay for units in this city.
-*************************************************************************/
-int city_unit_upkeep(const struct city *pcity, Output_type_id otype)
-{
-  int cost = 0;
-
-  unit_list_iterate(pcity->units_supported, punit) {
-    cost += punit->upkeep[otype];
-  } unit_list_iterate_end;
-
-  return cost;
-}
-
 /**************************************************************************
   Return TRUE iff this city is its nation's capital.  The capital city is
   special-cased in a number of ways.
@@ -1963,16 +1949,73 @@
 }
 
 /**************************************************************************
+  Query unhappiness caused by a given unit.
+**************************************************************************/
+int city_unit_unhappiness(struct unit *punit, int *free_unhappy)
+{
+  struct city *pcity = find_city_by_id(punit->homecity);
+  struct unit_type *ut = unit_type(punit);
+  struct player *plr = punit->owner;
+  int happy_cost = utype_happy_cost(ut, plr);
+
+  if (!punit || !pcity || !free_unhappy || happy_cost <= 0) {
+    return 0;
+  }
+  assert(free_unhappy >= 0);
+
+  happy_cost -= get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL_PER);
+
+  if (!unit_being_aggressive(punit) && !is_field_unit(punit)) {
+    return 0;
+  }
+  if (happy_cost <= 0) {
+    return 0;
+  }
+  if (*free_unhappy > happy_cost) {
+    *free_unhappy -= happy_cost;
+    return 0;
+  }
+  return happy_cost;
+}
+
+/**************************************************************************
+  Calculate upkeep of a given unit.
+**************************************************************************/
+void city_unit_upkeep(struct unit *punit, int *outputs, int *free_upkeep)
+{
+  struct city *pcity = find_city_by_id(punit->homecity);
+  struct unit_type *ut = unit_type(punit);
+  struct player *plr = punit->owner;
+
+  assert(punit != NULL && pcity != NULL && ut != NULL 
+         && free_upkeep != NULL && outputs != NULL);
+  memset(outputs, 0, O_COUNT * sizeof(*outputs));
+  output_type_iterate(o) {
+    outputs[o] = utype_upkeep_cost(ut, plr, o);
+  } output_type_iterate_end;
+
+  /* set current upkeep on unit to zero */
+
+  output_type_iterate(o) {
+    int cost = utype_upkeep_cost(ut, plr, o);
+    if (cost > 0) {
+      if (free_upkeep[o] > cost) {
+        free_upkeep[o] -= cost;
+        continue;
+      }
+      outputs[o] = cost;
+    }
+  } output_type_iterate_end;
+}
+
+/**************************************************************************
   Calculate upkeep costs.  This builds the pcity->usage[] array as well
   as setting some happiness values.
 **************************************************************************/
-static inline void city_support(struct city *pcity, 
-	 		        void (*send_unit_info) (struct player *pplayer,
-						        struct unit *punit))
+static inline void city_support(struct city *pcity)
 {
-  struct player *plr = city_owner(pcity);
   int free_upkeep[O_COUNT];
-  int free_happy = get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL);
+  int free_unhappy = get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL);
 
   output_type_iterate(o) {
     free_upkeep[o] = get_city_output_bonus(pcity, get_output_type(o), 
@@ -1988,15 +2031,6 @@
   pcity->usage[O_GOLD] += city_building_upkeep(pcity, O_GOLD);
   pcity->usage[O_FOOD] += game.info.food_cost * pcity->size;
 
-  /*
-   * If you modify anything here these places might also need updating:
-   * - ai/aitools.c : ai_assess_military_unhappiness
-   *   Military discontentment evaluation for AI.
-   *
-   * P.S.  This list is by no means complete.
-   * --SKi
-   */
-
   /* military units in this city (need _not_ be home city) can make
      unhappy citizens content
    */
@@ -2013,102 +2047,34 @@
     pcity->martial_law *= get_city_bonus(pcity, EFT_MARTIAL_LAW_EACH);
   }
 
-  /* loop over units, subtracting appropriate amounts of food, shields,
-   * gold etc -- SKi */
   unit_list_iterate(pcity->units_supported, this_unit) {
-    struct unit_type *ut = unit_type(this_unit);
-    int upkeep_cost[O_COUNT], old_upkeep[O_COUNT];
-    int happy_cost = utype_happy_cost(ut, plr);
-    bool changed = FALSE;
+    int upkeep_cost[O_COUNT];
+    int happy_cost = city_unit_unhappiness(this_unit, &free_unhappy);
 
-    /* Save old values so we can decide if the unit info should be resent */
-    int old_unhappiness = this_unit->unhappiness;
+    city_unit_upkeep(this_unit, upkeep_cost, free_upkeep);
 
     output_type_iterate(o) {
-      upkeep_cost[o] = utype_upkeep_cost(ut, plr, o);
-      old_upkeep[o] = this_unit->upkeep[o];
+      pcity->usage[o] += upkeep_cost[o];
     } output_type_iterate_end;
-
-    /* set current upkeep on unit to zero */
-    this_unit->unhappiness = 0;
-    memset(this_unit->upkeep, 0, O_COUNT * sizeof(*this_unit->upkeep));
-
-    /* This is how I think it should work (dwp)
-     * Base happy cost (unhappiness) assumes unit is being aggressive;
-     * non-aggressive units don't pay this, _except_ that field units
-     * still pay 1.  Should this be always 1, or modified by other
-     * factors?   Will treat as flat 1.
-     */
-    if (happy_cost > 0 && !unit_being_aggressive(this_unit)) {
-      if (is_field_unit(this_unit)) {
-	happy_cost = 1;
-      } else {
-	happy_cost = 0;
-      }
-    }
-    if (happy_cost > 0
-	&& get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL_PER) > 0) {
-      happy_cost--;
-    }
-
-    /* subtract values found above from city's resources -- SKi */
-    if (happy_cost > 0) {
-      adjust_city_free_cost(&free_happy, &happy_cost);
-      if (happy_cost > 0) {
-	pcity->unit_happy_upkeep += happy_cost;
-	this_unit->unhappiness = happy_cost;
-      }
-    }
-    changed |= (old_unhappiness != happy_cost);
-
-    output_type_iterate(o) {
-      if (upkeep_cost[o] > 0) {
-	adjust_city_free_cost(&free_upkeep[o], &upkeep_cost[o]);
-	if (upkeep_cost[o] > 0) {
-	  pcity->usage[o] += upkeep_cost[o];
-	  this_unit->upkeep[o] = upkeep_cost[o];
-	}
-      }
-      changed |= (old_upkeep[o] != upkeep_cost[o]);
-    } output_type_iterate_end;
-
-    /* Send unit info if anything has changed */
-    if (send_unit_info && changed) {
-      send_unit_info(unit_owner(this_unit), this_unit);
-    }
+    pcity->unit_happy_upkeep += happy_cost;
   } unit_list_iterate_end;
 }
 
 /**************************************************************************
   Refreshes the internal cached data in the city structure.
 
-  There are two possible levels of refresh: a partial refresh and a full
-  refresh.  A partial refresh is faster but can only be used in a few
-  places.
-
-  A full refresh updates all cached data: including but not limited to
-  ppl_happy[], surplus[], waste[], citizen_base[], usage[], trade[],
-  bonus[], and tile_output[].
-
-  A partial refresh will not update tile_output[] or bonus[].  These two
-  values do not need to be recalculated when moving workers around or when
-  a trade route has changed.  A partial refresh will also not refresh any
-  cities that have trade routes with us.  Any time a partial refresh is
-  done it should be considered temporary: when finished, the city should
-  be reverted to its original state.
+  !full_refresh will not update tile_output[] or bonus[].  These two
+  values do not need to be recalculated when moving workers around, for
+  example.
 **************************************************************************/
-void generic_city_refresh(struct city *pcity,
-			  bool full_refresh,
-			  void (*send_unit_info) (struct player * pplayer,
-						  struct unit * punit))
+void generic_city_refresh(struct city *pcity, bool full_refresh)
 {
   struct player *pplayer = city_owner(pcity);
-  int prev_tile_trade = pcity->citizen_base[O_TRADE];
 
   if (full_refresh) {
     set_city_bonuses(pcity);	/* Calculate the bonus[] array values. */
     set_city_tile_output(pcity); /* Calculate the tile_output[] values. */
-    city_support(pcity, send_unit_info); /* manage settlers, and units */
+    city_support(pcity); /* manage settlers, and units */
   }
 
   /* Calculate output from citizens. */
@@ -2136,49 +2102,9 @@
                       &pcity->ppl_angry[4]);
   unhappy_city_check(pcity);
   set_surpluses(pcity);
-
-  if (full_refresh
-      && pcity->citizen_base[O_TRADE] != prev_tile_trade) {
-    int i;
-
-    for (i = 0; i < NUM_TRADEROUTES; i++) {
-      struct city *pcity2 = find_city_by_id(pcity->trade[i]);
-
-      if (pcity2) {
-	/* We used to pass FALSE in here to avoid multiple recursion.  This
-	 * made it impossible to initialize a city for the first time
-	 * however, since it's not safe to recurse on an unitialized
-	 * city without doing a full refresh.  See PR#12498. */
-	generic_city_refresh(pcity2, TRUE, send_unit_info);
-      }
-    }
-  }
 }
 
 /**************************************************************************
-  Here num_free is eg government->free_unhappy, and this_cost is
-  the unhappy cost for a single unit.  We subtract this_cost from
-  num_free as much as possible. 
-
-  Note this treats the free_cost as number of eg happiness points,
-  not number of eg military units.  This seems to make more sense
-  and makes results not depend on order of calculation. --dwp
-**************************************************************************/
-void adjust_city_free_cost(int *num_free, int *this_cost)
-{
-  if (*num_free <= 0 || *this_cost <= 0) {
-    return;
-  }
-  if (*num_free >= *this_cost) {
-    *num_free -= *this_cost;
-    *this_cost = 0;
-  } else {
-    *this_cost -= *num_free;
-    *num_free = 0;
-  }
-}
-
-/**************************************************************************
   Give corruption/waste generated by city.  otype gives the output type
   (O_SHIELD/O_TRADE).  'total' gives the total output of this type in the
   city.
Index: common/city.h
===================================================================
--- common/city.h	(revision 12804)
+++ common/city.h	(working copy)
@@ -364,7 +364,8 @@
 struct player *city_owner(const struct city *pcity);
 int city_population(const struct city *pcity);
 int city_building_upkeep(const struct city *pcity, Output_type_id otype);
-int city_unit_upkeep(const struct city *pcity, Output_type_id otype);
+int city_unit_unhappiness(struct unit *punit, int *free_happy);
+void city_unit_upkeep(struct unit *punit, int *outputs, int *free_upkeep);
 int city_buy_cost(const struct city *pcity);
 bool city_happy(const struct city *pcity);  /* generally use celebrating instead */
 bool city_unhappy(const struct city *pcity);                /* anarchy??? */
@@ -490,11 +491,8 @@
 void city_remove_improvement(struct city *pcity, Impr_type_id impr);
 
 /* city update functions */
-void generic_city_refresh(struct city *pcity,
-			  bool full_refresh,
-			  void (*send_unit_info) (struct player * pplayer,
-						  struct unit * punit));
-void adjust_city_free_cost(int *num_free, int *this_cost);
+void generic_city_refresh(struct city *pcity, bool full_refresh);
+
 int city_waste(const struct city *pcity, Output_type_id otype, int total);
 int city_specialists(const struct city *pcity);                 /* elv+tax+scie */
 Specialist_type_id best_specialist(Output_type_id otype,
Index: common/aicore/cm.c
===================================================================
--- common/aicore/cm.c	(revision 12804)
+++ common/aicore/cm.c	(working copy)
@@ -672,7 +672,7 @@
   }
 
   /* Finally we must refresh the city to reset all the precomputed fields. */
-  generic_city_refresh(pcity, FALSE, 0);
+  generic_city_refresh(pcity, FALSE);
   assert(sumworkers == pcity->size);
 }
 
@@ -1788,7 +1788,7 @@
   /* Refresh the city.  Otherwise the CM can give wrong results or just be
    * slower than necessary.  Note that cities are often passed in in an
    * unrefreshed state (which should probably be fixed). */
-  generic_city_refresh(pcity, TRUE, NULL);
+  generic_city_refresh(pcity, TRUE);
 
   cm_find_best_solution(state, param, result);
   cm_free_state(state);
Index: ai/aitools.c
===================================================================
--- ai/aitools.c	(revision 12804)
+++ ai/aitools.c	(working copy)
@@ -1203,39 +1203,17 @@
 **********************************************************************/
 bool ai_assess_military_unhappiness(struct city *pcity)
 {
-  int free_happy;
+  int free_unhappy = get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL);
   int unhap = 0;
 
   /* bail out now if happy_cost is 0 */
   if (get_player_bonus(city_owner(pcity), EFT_UNHAPPY_FACTOR) == 0) {
     return FALSE;
   }
-  free_happy = get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL);
 
   unit_list_iterate(pcity->units_supported, punit) {
-    int happy_cost = utype_happy_cost(unit_type(punit), city_owner(pcity));
+    int happy_cost = city_unit_unhappiness(punit, &free_unhappy);
 
-    if (happy_cost <= 0) {
-      continue;
-    }
-
-    /* See discussion/rules in common/city.c:city_support() */
-    if (!unit_being_aggressive(punit)) {
-      if (is_field_unit(punit)) {
-	happy_cost = 1;
-      } else {
-	happy_cost = 0;
-      }
-    }
-    if (happy_cost <= 0) {
-      continue;
-    }
-
-    if (get_city_bonus(pcity, EFT_MAKE_CONTENT_MIL_PER) > 0) {
-      happy_cost--;
-    }
-    adjust_city_free_cost(&free_happy, &happy_cost);
-    
     if (happy_cost > 0) {
       unhap += happy_cost;
     }
Index: ai/aicity.c
===================================================================
--- ai/aicity.c	(revision 12804)
+++ ai/aicity.c	(working copy)
@@ -1775,7 +1775,7 @@
 
   unit_list_iterate_safe(pcity->units_supported, punit) {
     if (city_unhappy(pcity)
-        && punit->unhappiness != 0
+/*        && punit->unhappiness != 0  FIXME -- this was STUPID, it was not THIS unit that caused, it could be ANY unit */
         && punit->ai.passenger == 0) {
       UNIT_LOG(LOG_EMERGENCY, punit, "is causing unrest, disbanded");
       handle_unit_disband(pplayer, punit->id);
Index: ai/aihand.c
===================================================================
--- ai/aihand.c	(revision 12804)
+++ ai/aihand.c	(working copy)
@@ -228,7 +228,7 @@
           cm_query_result(pcity, &cmp, &cmr);
           if (cmr.found_a_valid) {
             apply_cmresult_to_city(pcity, &cmr);
-            generic_city_refresh(pcity, TRUE, NULL);
+            generic_city_refresh(pcity, TRUE);
             if (!city_happy(pcity)) {
               CITY_LOG(LOG_ERROR, pcity, "is NOT happy when it should be!");
             }
@@ -243,7 +243,7 @@
          * were clobbered in cm_query_result, after the tax rates 
          * were changed.  This is because the cm_query_result() calls
          * generic_city_refresh(). */
-        generic_city_refresh(pcity, TRUE, NULL);
+        generic_city_refresh(pcity, TRUE);
       } city_list_iterate_end;
     }
   }
@@ -351,7 +351,7 @@
     pplayer->government = current_gov;
     city_list_iterate(pplayer->cities, acity) {
       /* This isn't strictly necessary since it's done in aaw. */
-      generic_city_refresh(acity, TRUE, NULL);
+      generic_city_refresh(acity, TRUE);
       auto_arrange_workers(acity);
     } city_list_iterate_end;
     ai->govt_reeval = CLIP(5, city_list_size(pplayer->cities), 20);
Index: client/gui-gtk-2.0/citydlg.c
===================================================================
--- client/gui-gtk-2.0/citydlg.c	(revision 12804)
+++ client/gui-gtk-2.0/citydlg.c	(working copy)
@@ -1643,7 +1643,15 @@
   struct unit_node_vector *nodes;
   int n, m, i;
   char buf[30];
+  int free_upkeep[O_COUNT];
+  int free_unhappy = get_city_bonus(pdialog->pcity, EFT_MAKE_CONTENT_MIL);
 
+  memset(free_upkeep, 0, O_COUNT * sizeof(*free_upkeep));
+  output_type_iterate(o) {
+    free_upkeep[o] = get_city_output_bonus(pdialog->pcity, get_output_type(o),
+                                           EFT_UNIT_UPKEEP_FREE_PER_CITY);
+  } output_type_iterate_end;
+
   if (game.player_ptr && pdialog->pcity->owner != game.player_ptr) {
     units = pdialog->pcity->info_units_supported;
   } else {
@@ -1702,6 +1710,10 @@
   i = 0;
   unit_list_iterate(units, punit) {
     struct unit_node *pnode;
+    int upkeep_cost[O_COUNT];
+    int happy_cost = city_unit_unhappiness(punit, &free_unhappy);
+
+    city_unit_upkeep(punit, upkeep_cost, free_upkeep);
     
     pnode = unit_node_vector_get(nodes, i);
     if (pnode) {
@@ -1712,7 +1724,7 @@
 
       gtk_pixcomm_freeze(GTK_PIXCOMM(pix));
       put_unit_gpixmap(punit, GTK_PIXCOMM(pix));
-      put_unit_gpixmap_city_overlays(punit, GTK_PIXCOMM(pix));
+      put_unit_gpixmap_city_overlays(punit, GTK_PIXCOMM(pix), upkeep_cost, happy_cost);
       gtk_pixcomm_thaw(GTK_PIXCOMM(pix));
 
       g_signal_handlers_disconnect_matched(cmd,
Index: client/gui-gtk-2.0/mapview.c
===================================================================
--- client/gui-gtk-2.0/mapview.c	(revision 12804)
+++ client/gui-gtk-2.0/mapview.c	(working copy)
@@ -486,7 +486,8 @@
   unit, the proper way to do this is probably something like what Civ II does.
   (One food/shield/mask drawn N times, possibly one top of itself. -- SKi 
 **************************************************************************/
-void put_unit_gpixmap_city_overlays(struct unit *punit, GtkPixcomm *p)
+void put_unit_gpixmap_city_overlays(struct unit *punit, GtkPixcomm *p,
+                                    int *upkeep_cost, int happy_cost)
 {
   struct canvas store;
  
@@ -495,7 +496,8 @@
 
   gtk_pixcomm_freeze(p);
 
-  put_unit_city_overlays(punit, &store, 0, tileset_tile_height(tileset));
+  put_unit_city_overlays(punit, &store, 0, tileset_tile_height(tileset),
+                         upkeep_cost, happy_cost);
 
   gtk_pixcomm_thaw(p);
 }
Index: client/gui-gtk-2.0/mapview.h
===================================================================
--- client/gui-gtk-2.0/mapview.h	(revision 12804)
+++ client/gui-gtk-2.0/mapview.h	(working copy)
@@ -37,7 +37,8 @@
 
 void put_unit_gpixmap(struct unit *punit, GtkPixcomm *p);
 
-void put_unit_gpixmap_city_overlays(struct unit *punit, GtkPixcomm *p);
+void put_unit_gpixmap_city_overlays(struct unit *punit, GtkPixcomm *p,
+                                    int *upkeep_cost, int happy_cost);
 
 void scrollbar_jump_callback(GtkAdjustment *adj, gpointer hscrollbar);
 void update_map_canvas_scrollbars_size(void);
Index: client/gui-gtk-2.0/repodlgs.c
===================================================================
--- client/gui-gtk-2.0/repodlgs.c	(revision 12804)
+++ client/gui-gtk-2.0/repodlgs.c	(working copy)
@@ -1194,14 +1194,26 @@
     gtk_list_store_clear(activeunits_store);
 
     memset(unitarray, '\0', sizeof(unitarray));
-    unit_list_iterate(game.player_ptr->units, punit) {
-      (unitarray[punit->type->index].active_count)++;
-      if (punit->homecity) {
-	output_type_iterate(o) {
-	  unitarray[punit->type->index].upkeep[o] += punit->upkeep[o];
-	} output_type_iterate_end;
-      }
-    } unit_list_iterate_end;
+    city_list_iterate(game.player_ptr->cities, pcity) {
+      int free_upkeep[O_COUNT];
+
+      output_type_iterate(o) {
+        free_upkeep[o] = get_city_output_bonus(pcity, get_output_type(o),
+                                               EFT_UNIT_UPKEEP_FREE_PER_CITY);
+      } output_type_iterate_end;
+
+      unit_list_iterate(pcity->units_supported, punit) {
+        int upkeep_cost[O_COUNT];
+
+        city_unit_upkeep(punit, upkeep_cost, free_upkeep);
+        (unitarray[punit->type->index].active_count)++;
+        if (punit->homecity) {
+	  output_type_iterate(o) {
+	    unitarray[punit->type->index].upkeep[o] += upkeep_cost[o];
+	  } output_type_iterate_end;
+        }
+      } unit_list_iterate_end;
+    } city_list_iterate_end;
     city_list_iterate(game.player_ptr->cities,pcity) {
       if (pcity->production.is_unit) {
 	(unitarray[pcity->production.value].building_count)++;
Index: client/repodlgs_common.c
===================================================================
--- client/repodlgs_common.c	(revision 12804)
+++ client/repodlgs_common.c	(working copy)
@@ -102,6 +102,7 @@
 				   int *num_entries_used, int *total_cost)
 {
   int count, cost, partial_cost;
+  int free_upkeep[O_COUNT];
 
   *num_entries_used = 0;
   *total_cost = 0;
@@ -109,6 +110,7 @@
   if (!game.player_ptr) {
     return;
   }
+  memset(free_upkeep, 0, O_COUNT * sizeof(*free_upkeep));
 
   unit_type_iterate(unittype) {
     cost = utype_upkeep_cost(unittype, game.player_ptr, O_GOLD);
@@ -122,11 +124,17 @@
     partial_cost = 0;
 
     city_list_iterate(game.player_ptr->cities, pcity) {
+      free_upkeep[O_GOLD] = get_city_output_bonus(pcity, get_output_type(O_GOLD),
+                                                  EFT_UNIT_UPKEEP_FREE_PER_CITY);
+
       unit_list_iterate(pcity->units_supported, punit) {
+        int upkeep_cost[O_COUNT];
 
+        city_unit_upkeep(punit, upkeep_cost, free_upkeep);
+
 	if (punit->type == unittype) {
 	  count++;
-	  partial_cost += punit->upkeep[O_GOLD];
+	  partial_cost += upkeep_cost[O_GOLD];
 	}
 
       } unit_list_iterate_end;
Index: client/packhand.c
===================================================================
--- client/packhand.c	(revision 12804)
+++ client/packhand.c	(working copy)
@@ -96,10 +96,6 @@
   punit->hp = packet->hp;
   punit->activity = packet->activity;
   punit->activity_count = packet->activity_count;
-  punit->unhappiness = packet->unhappiness;
-  output_type_iterate(o) {
-    punit->upkeep[o] = packet->upkeep[o];
-  } output_type_iterate_end;
   punit->ai.control = packet->ai;
   punit->fuel = packet->fuel;
   if (is_normal_map_pos(packet->goto_dest_x, packet->goto_dest_y)) {
@@ -1198,16 +1194,6 @@
 
     }  /*** End of Change position. ***/
 
-    if (punit->unhappiness != packet_unit->unhappiness) {
-      punit->unhappiness = packet_unit->unhappiness;
-      repaint_city = TRUE;
-    }
-    output_type_iterate(o) {
-      if (punit->upkeep[o] != packet_unit->upkeep[o]) {
-	punit->upkeep[o] = packet_unit->upkeep[o];
-	repaint_city = TRUE;
-      }
-    } output_type_iterate_end;
     if (repaint_city || repaint_unit) {
       /* We repaint the city if the unit itself needs repainting or if
        * there is a special city-only redrawing to be done. */
Index: client/tilespec.c
===================================================================
--- client/tilespec.c	(revision 12804)
+++ client/tilespec.c	(working copy)
@@ -4716,9 +4716,10 @@
   May return NULL if there's no unhappiness.
 ****************************************************************************/
 struct sprite *get_unit_unhappy_sprite(const struct tileset *t,
-				       const struct unit *punit)
+				       const struct unit *punit,
+				       int happy_cost)
 {
-  const int unhappy = CLIP(0, punit->unhappiness, 2);
+  const int unhappy = CLIP(0, happy_cost, 2);
 
   if (unhappy > 0) {
     return t->sprites.upkeep.unhappy[unhappy - 1];
@@ -4735,9 +4736,10 @@
 ****************************************************************************/
 struct sprite *get_unit_upkeep_sprite(const struct tileset *t,
 				      Output_type_id otype,
-				      const struct unit *punit)
+				      const struct unit *punit,
+				      const int *upkeep_cost)
 {
-  const int upkeep = CLIP(0, punit->upkeep[otype], 2);
+  const int upkeep = CLIP(0, upkeep_cost[otype], 2);
 
   if (upkeep > 0) {
     return t->sprites.upkeep.output[otype][upkeep - 1];
Index: client/tilespec.h
===================================================================
--- client/tilespec.h	(revision 12804)
+++ client/tilespec.h	(working copy)
@@ -241,10 +241,12 @@
 				    enum indicator_type indicator,
 				    int index);
 struct sprite *get_unit_unhappy_sprite(const struct tileset *t,
-				       const struct unit *punit);
+				       const struct unit *punit,
+				       int happy_cost);
 struct sprite *get_unit_upkeep_sprite(const struct tileset *t,
 				      Output_type_id otype,
-				      const struct unit *punit);
+				      const struct unit *punit,
+				      const int *upkeep_cost);
 struct sprite *get_basic_fog_sprite(const struct tileset *t);
 
 struct sprite* lookup_sprite_tag_alt(struct tileset *t,
Index: client/mapview_common.c
===================================================================
--- client/mapview_common.c	(revision 12804)
+++ client/mapview_common.c	(working copy)
@@ -954,17 +954,18 @@
 ****************************************************************************/
 void put_unit_city_overlays(struct unit *punit,
 			    struct canvas *pcanvas,
-			    int canvas_x, int canvas_y)
+			    int canvas_x, int canvas_y, int *upkeep_cost,
+                            int happy_cost)
 {
   struct sprite *sprite;
 
-  sprite = get_unit_unhappy_sprite(tileset, punit);
+  sprite = get_unit_unhappy_sprite(tileset, punit, happy_cost);
   if (sprite) {
     canvas_put_sprite_full(pcanvas, canvas_x, canvas_y, sprite);
   }
 
   output_type_iterate(o) {
-    sprite = get_unit_upkeep_sprite(tileset, o, punit);
+    sprite = get_unit_upkeep_sprite(tileset, o, punit, upkeep_cost);
     if (sprite) {
       canvas_put_sprite_full(pcanvas, canvas_x, canvas_y, sprite);
     }
Index: client/mapview_common.h
===================================================================
--- client/mapview_common.h	(revision 12804)
+++ client/mapview_common.h	(working copy)
@@ -248,8 +248,9 @@
 		 struct canvas *pcanvas, int canvas_x, int canvas_y);
 
 void put_unit_city_overlays(struct unit *punit,
-			    struct canvas *pcanvas,
-			    int canvas_x, int canvas_y);
+                            struct canvas *pcanvas,
+                            int canvas_x, int canvas_y, int *upkeep_cost,
+                            int happy_cost);
 void toggle_city_color(struct city *pcity);
 void toggle_unit_color(struct unit *punit);
 
Index: common/city.c
===================================================================
--- common/city.c	(revision 12804)
+++ common/city.c	(working copy)
@@ -868,8 +868,8 @@
   int bonus = 0;
 
   if (pc1 && pc2) {
-    bonus = (pc1->citizen_base[O_TRADE]
-	     + pc2->citizen_base[O_TRADE] + 4) / 8;
+    bonus = real_map_distance(pc1->tile, pc2->tile) + pc1->size + pc2->size;
+    bonus /= 8;
 
     /* Double if on different continents. */
     if (tile_get_continent(pc1->tile) != tile_get_continent(pc2->tile)) {
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev

Reply via email to