[Freeciv-Dev] Re: CMA failed assertion caused by Freight (PR#1684)

["Per I. Mathisen" <per@xxxxxxxxxxx>]

On Sat, 14 Sep 2002, Raimar Falke wrote:
> > > This is a nasty one: the freight moves and makes a tile to Oulu
> > > unavailable and later available again. The worker is removed the first
> > > time and isn't put to work the second time. First problem: Oulu isn't
> > > recalulated the second time. Second problem: the client isn't informed
> > > about this. However if the unit arrives at Tampere the server also
> > > recalculates Oulu which now have a worker less (than the current info
> > > of the client) causing reduces trade which causes reduces trade from
> > > trade route which causes a mismatch between server and client.
> >
> > > There is IMHO no easy fix and recalculating the cities at the server
> > > (which will be part of the fix) will make the server slower. So since
> > > it uncommon and not critical I would vote for post-release.
> >
> > And the patch. AI-people please comment on the use of
> > auto_arrange_workers.

Index: server/citytools.c
===================================================================
--- server/citytools.c  2002/09/02 02:19:54     1.189
+++ server/citytools.c  2002/09/14 15:42:14
@@ -2146,12 +2148,23 @@
       server_set_tile_city(pcity, city_x, city_y, C_TILE_UNAVAILABLE);
       add_adjust_workers(pcity); /* will place the displaced */
       city_refresh(pcity);
+      send_city_info(NULL, pcity);
     }
     break;

   case C_TILE_UNAVAILABLE:
     if (is_available) {
+      /* Was auto_arrange_workers called? This avoids recursion. */
+      static bool aaw_called = FALSE;
+
       server_set_tile_city(pcity, city_x, city_y, C_TILE_EMPTY);
+
+      if (!aaw_called) {
+       aaw_called = TRUE;
+       auto_arrange_workers(pcity);
+       aaw_called = FALSE;
+       send_city_info(NULL, pcity);
+      }
     }
     break;

This looks very wrong. I don't like the way it is currently done, and your
patch is another bad kludge on top of it. The use of of a static variable
here screams of wrongness.

I don't understand why you cannot leave it at sending the new information
to the client. The use of auto arrange workers here should be unnecessary
for the CMA.

As I said earlier, auto arrange worker code should not be called this deep
in the code. That also applies to current case C_TILE_WORKER. Instead, I
suggest displaced workers are turned into elvises, and that a new city
bitvector is used to tell if a tile has been previously displaced, so that
if the tile becomes available again, we remove the last specialist and put
a worker in the tile instead.

Yours
Per

"I don't see why people are so upset about cloning sheep. American
television networks have been doing that to their audiences for years."
-- Jello Biafra