[Freeciv-Dev] solution to bug #163

[Thue Janus Kristensen <Thue.kristensen@xxxxxxxxxx>]

The bug was that when a city was disbanded by the function

    static void disband_city(struct city *pcity)

in server/cityturn.c, the function

void transfer_city_units(struct player *pplayer, struct player *pvictim,

    struct city *pcity, struct city *vcity,
    int kill_outside, int verbose)

in server/citytools was used. This function is also used when a city is
bougth by a diplomat or a spy. The effect is that not only was all the
units supported by the city reevaluated, but all units present in the
city was transfered to the nearest city, even thougth they may have a
perfectly valid homecity allready.(when you buy out a city you get all
the units in it)

A fhe fix for this bug, maybe a little ugly, but better than including
another parameter in the transfer_city_units function, is to check if
the owner of pcity is the same as the owner of vcity. If this is the
case then it is not a hostile takeover, and you only need to run thougth

the supported units.

Now I am not the greatest C programmer (actually I haven't tried it
before:)), but I encapsulated the codeblok

  /* Transfer units in the city to the new owner */
  unit_list_iterate(map_get_tile(x, y)->units, vunit)  {
    freelog(LOG_VERBOSE, "Transfered %s in %s from %s to %s",
     unit_name(vunit->type), vcity->name, pvictim->name, pplayer->name);

    if (verbose) {
      notify_player(pvictim, _("Game: Transfered %s in %s from %s to
%s."),
      unit_name(vunit->type), vcity->name,
      pvictim->name, pplayer->name);
    }
    create_unit_full(pplayer, x, y, vunit->type, vunit->veteran,
  pcity->id, vunit->moves_left, vunit->hp);
    wipe_unit(0, vunit);
  } unit_list_iterate_end;

in an "if" blok:
if (city_owner(pcity)!=city_owner(vcity)){
    ...
}

I actually tested this, and it worked!(well, only disbanding and not
buying, but I can see why buing cities shouldn't work(famous last
words))
I even added a comment in the file to say what I did.

I have included a diff -u between the 1.9 version and my improved
version of citytools.c . Is that what you normally do, or do you use a
development snapshot? Who will make sure this is incotorated into
Freeciv?
I really haven't tried this before!

-Thue

--- /usr/local/freeciv-1.9.0/server/citytools.c Mon Oct  4 14:50:26 1999
+++ server/citytools.c  Sat Oct 30 20:12:20 1999
@@ -567,6 +567,7 @@
   return 0;
 }
 
+
 /*
  *  Units in a bought city are transferred to the new owner, units 
  * supported by the city, but held in other cities are updated to
@@ -580,6 +581,11 @@
  *
  * If verbose is true, send extra messages to players detailing what
  * happens to all the units.  --dwp
+ *
+ * If statement added to see if the owner of pcity is the same as vcity. If so,
+ * then the city is disbanded and not bougth, and we only need to evaluate the
+ * units supported by the city, and not those actually present.
+ * - Thue <thue.kristensen@xxxxxxxxxx>
  */
 void transfer_city_units(struct player *pplayer, struct player *pvictim, 
                         struct city *pcity, struct city *vcity, 
@@ -588,19 +594,21 @@
   int x = vcity->x;
   int y = vcity->y;
 
-  /* Transfer units in the city to the new owner */
-  unit_list_iterate(map_get_tile(x, y)->units, vunit)  {
-    freelog(LOG_VERBOSE, "Transfered %s in %s from %s to %s",
-           unit_name(vunit->type), vcity->name, pvictim->name, pplayer->name);
-    if (verbose) {
-      notify_player(pvictim, _("Game: Transfered %s in %s from %s to %s."),
-                   unit_name(vunit->type), vcity->name,
-                   pvictim->name, pplayer->name);
-    }
-    create_unit_full(pplayer, x, y, vunit->type, vunit->veteran,
-               pcity->id, vunit->moves_left, vunit->hp);
-    wipe_unit(0, vunit);
-  } unit_list_iterate_end;
+  if (city_owner(pcity)!=city_owner(vcity)){ /* true=>bougth, false 
=>disbanded */
+    /* Transfer units in the city to the new owner */
+    unit_list_iterate(map_get_tile(x, y)->units, vunit)  {
+      freelog(LOG_VERBOSE, "Transfered %s in %s from %s to %s",
+             unit_name(vunit->type), vcity->name, pvictim->name, 
pplayer->name);
+      if (verbose) {
+       notify_player(pvictim, _("Game: Transfered %s in %s from %s to %s."),
+                     unit_name(vunit->type), vcity->name,
+                     pvictim->name, pplayer->name);
+      }
+      create_unit_full(pplayer, x, y, vunit->type, vunit->veteran,
+                      pcity->id, vunit->moves_left, vunit->hp);
+      wipe_unit(0, vunit);
+    } unit_list_iterate_end;
+  }
 
   /* Any remaining units supported by the city are either given new home
    * cities or maybe destroyed */