[Freeciv-Dev] server/cityturn.c - surplus food when settlers (units with F_CITIES) are built (PR#811)

[michaelz@xxxxxxxxxxxxxxx (Michael Zinn)]

The problem: when settlers are built, the city they are built in consumes needs
    the food for both the settler *and* the extra head of population for that
    turn

More details are in the inserted `patch' below, which isn't really a patch
but rather an opportunity to put the problem in context and describe it in
more detail :P

Attached is also a "patch" which `ought' to work, but seems a little too
obvious, again, as discussed in the comment below.


And the obligatory details:                 

Debian GNU/Linux 2.2r3
freeciv-1.11.5-devel 2001-06-22 (depending on your timezone :P)
gtk+ client          
personal details as in sig


The lets-put-it-in-context-and-comment-on-it-not-really-a-patch:

Index: cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.148
diff -u -2 -0 -r1.148 cityturn.c
--- cityturn.c  2001/05/23 18:35:07     1.148
+++ cityturn.c  2001/06/23 04:53:14
@@ -1427,40 +1427,64 @@
       if (unit_flag(pcity->currently_building, F_CITIES)) {
        if (pcity->size==1) {
 
          /* Should we disband the city? -- Massimo */
          if (pcity->city_options & ((1<<CITYO_DISBAND))) {
            return !disband_city(pcity);
          } else {
            notify_player_ex(pplayer, pcity->x, pcity->y, E_CITY_CANTBUILD,
                             _("Game: %s can't build %s yet."),
                             pcity->name, unit_name(pcity->currently_building));
            return 1;
          }
 
        }
        city_built_city_builder = 1;
       }
       
       pcity->turn_last_built = game.year;
       /* don't update turn_last_built if we returned above for size==1 */
 
+      /* Here's where I suspect the fix would be...
+       * what happens is, create_unit does the whole refresh_city thing, which
+       * includes a set_food_trade_shields and a city_support which together
+       * set the pcity->food_surplus.
+       * The problem with this is, that for this turns food surplus purposes,
+       * the player is paying for *both* the extra head of population *and*
+       * the settler (ie an F_CITIES unit), which is not really what should
+       * happend.
+       * It may seem minor, but I find that in smallpox, its not uncommon to
+       * be building a settler in a size 2 city with 0 food surplus and 0
+       * food in storage, because there is an entertainer at work; with the
+       * code as it is, the settler immediately dies from famine.
+       * This has been checked against civ and civ2 and both do it the way
+       * "I would have expected", ie, not this way :P
+       *
+       * So much for problems, how about solutions? The obvious thing to do
+       * would be to take the if( city_built_city_builder ) block below which
+       * reduces the city size and rearranges the workers and stick it in
+       * right here, or even at the end of the block immediately above where
+       * that flag was set.
+       * However, I assume that there was some reason this wasn't done, so
+       * perhaps there are some consequences I'm not seeing?
+       */
+
       create_unit(pplayer, pcity->x, pcity->y, pcity->currently_building,
                  do_make_unit_veteran(pcity, pcity->currently_building), 
                  pcity->id, -1);
       /* to eliminate micromanagement, we only subtract the unit's cost */
       pcity->before_change_shields-=unit_value(pcity->currently_building); 
       pcity->shield_stock-=unit_value(pcity->currently_building);
 
       if (city_built_city_builder) {
        pcity->size--;
        city_auto_remove_worker(pcity);
       }
 
       notify_player_ex(pplayer, pcity->x, pcity->y, E_UNIT_BUILD,
                       _("Game: %s is finished building %s."), 
                       pcity->name, 
                       unit_types[pcity->currently_building].name);
 
       /* If there's something in the worklist, change the build target. 
         If there's nothing there, worklist_change_build_target won't
         do anything. */

-- 
Michael `Wumpus' Zinn | "The Universe speaks in many languages, but only one
  michael.z@xxxxxxx   |  voice ... it speaks in the language of hope... trust
         <*>          |  ... and compassion; it is the language of the Heart"
                      |    -- G'Kar's Declaration of Principles (Babylon 5)

server_cityturn__settler_food__needs_checking.patch
Description: Text document