[freeciv-ai] (PR#3546) "Arhus is bugged:" - crash

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

[jdorje - Fri Feb 28 21:21:02 2003]:

> Raimar Falke wrote:
> > On Fri, Feb 28, 2003 at 03:41:55AM -0800, ChrisK@xxxxxxxx wrote:
> 
> > I'm not able to solve it. The problem is that auto_arrange_workers
> > spends more workers that the city has. With the attached patch you get
> > an output like:
> > 
> > 2: auto_arrange_workers finished
> > 2: auto_arrange_workers for Århus, size=5
> > 2: all workers removed
> > 2: Århus: placing worker at (1, 1)
> > 2: Århus: placing worker at (3, 0)
> > 2: Århus: placing worker at (1, 3)
> > 2: Århus: placing worker at (1, 0)
> > 2: Århus: placing worker at (2, 3)
> > 2: Århus: placing worker at (2, 4)
> > 2: Århus: placing worker at (2, 0)
> > 2: Århus: placing worker at (2, 1)
> > 2: auto_arrange_workers finished
> > 1: Århus is bugged: size:5 workers:7 elvis: 0 tax:0 sci:0
> > 
> > Reason unknown.
> 
> The actual error happens because worker_loop calls 
> server_set_worker_city, which calls server_set_tile_city, which calls 
> update_city_tile_status, which calls add_adjust_workers, which calls 
> worker_loop.  Thus we place citizens 5, 4 and 3, then re-enter the loop 
> to place 2 and 1, then end that loop and return to the original loop 
> where we place 2 and 1 again.
> 
> problem is two-fold:
> 
> - worker_loop calls server_set_worker_city to make its change. 
> server_set_worker_city is _way_ too high-level a function to be called 
> by worker loop; it does all sorts of error handling on its own 
> (including the call to add_adjust_workers).
> 
> - worker_loop is trying to place a unit on a (seemingly) invalid tile. 
> worker_loop calls can_place_worker_here, which return TRUE...but 
> update_city_tile_status calls city_can_work_tile to perform the 
> (seemingly) identical check, and this return FALSE.  Looking at those 
> two functions, it seems likely that the error is that the citymap is not 
> updated correctly somewhere else.  But I see no way to track this down 
> aside from adding sanity checks all over the place.
> 
> The attached patch should help to clear up the problem.  I'd suggest 
> that for 1.14 worker_loop should call city_can_work_tile to *verify* 
> that it is valid before trying to use it.  For cvs HEAD we should fix 
> both of the above problems.

Arhus tries to place a worker onto a seemingly available tile that is
actually occupied by an enemy unit.  This causes the function being
called to catch the error and try again to place the worker.

The easiest solution is to check to see if the tile is actually
available before placing the worker there.  A further fix might be to
change things so the recursion isn't possible at all, or is handled
correctly - although this seems harder and is likely to slow the game
down in the common case.

jason

Index: server/cityturn.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/cityturn.c,v
retrieving revision 1.208
diff -u -r1.208 cityturn.c
--- server/cityturn.c   2003/03/07 05:14:41     1.208
+++ server/cityturn.c   2003/04/07 21:42:29
@@ -193,7 +193,8 @@
     best = 0;
 
     city_map_iterate_outwards(x, y) {
-      if(can_place_worker_here(pcity, x, y)) {
+      if (can_place_worker_here(pcity, x, y)
+         && city_can_work_tile(pcity, x, y) {
         cur = city_tile_value(pcity,x,y,*foodneed,*prodneed) - conflict[x][y];
        if (cur > best) {
          bx = x;