[Freeciv-Dev] (PR#10317) Tile marked as worked but occupied by an enemy unit!

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=10317 >

Preliminary analysis:

Player 4's unit invades Player 8's city 155.  This causes a civil war. 
The new player is player 16.  Player 8's city 1225 is transferred to
player 16.  auto_arrange_workers is called when the borders are updated,
because a border change makes one of the workers invalid.  However the
units inside the city aren't transferred until a couple of lines later.
 Hence the error.

My fix is to move the border recalculation down a bit.  It is done after
the transfer of units (which is what's causing this bug).  It is also
done after the resolving of stacks (which I guess will bounce any
previously-allied units that are inside the city).

However I'm confused why this bug doesn't happen all the time, every
time there is a civil war.  Maybe the border recalculation only
occasionally causes a rearrangement of workers.  Or maybe cities only
occasionally contain units...

It's also possible this isn't a bug.  However I'm not going to touch
that question.  auto_arrange_workers is ugly enough as it is.

jason

? fctest
? freeciv-2.0.0-beta1.tar.gz
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.276
diff -u -r1.276 citytools.c
--- server/citytools.c  29 Sep 2004 02:24:23 -0000      1.276
+++ server/citytools.c  13 Oct 2004 03:58:54 -0000
@@ -781,9 +781,6 @@
   pcity->owner = ptaker->player_no;
   city_list_insert(&ptaker->cities, pcity);
 
-  /* Update the national borders. */
-  map_update_borders_city_change(pcity);
-
   transfer_city_units(ptaker, pgiver, &old_city_units,
                      pcity, NULL,
                      kill_outside, transfer_unit_verbose);
@@ -795,6 +792,11 @@
     resolve_unit_stacks(pgiver, ptaker, transfer_unit_verbose);
   }
 
+  /* Update the national borders.  Note this can cause city workers to be
+   * reallocated so we want to transfer the units and resolve the stacks
+   * first. */
+  map_update_borders_city_change(pcity);
+
   /* Update the city's trade routes. */
   for (i = 0; i < NUM_TRADEROUTES; i++)
     old_trade_routes[i] = pcity->trade[i];