[Freeciv-Dev] (PR#11144) 7 science points expected but only 6 received

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=11144 >

Here is a better version of the original patch.

The original patch just made update_city_tile_status() always return
FALSE so there was no aaw by the callers.  Of course aaw was already
done inside the function so it was especially stupid to make the
tile-got-available case a special one.  Some, but only a few, of the
callers would call aaw if a tile got available.  Of course this is bad
for all the reasons previously discussed in the ticket.

This patch removes the return value of update_city_tile_status and
update_city_tile_status_map completely.  The only aaw is done inside the
function and it's not done when a tile becomes available.

As for the rest of the thread discussion, it's outside the scope of this
bug.  (The bug where an enemy AI move causes a tile to be covered up and
the human has no chance to fix it is solved by alternating movement.) 
For further discussion of this or other CM issues, let's start a new ticket.

-jason

Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.309
diff -u -r1.309 citytools.c
--- server/citytools.c  1 Apr 2005 00:42:27 -0000       1.309
+++ server/citytools.c  8 Apr 2005 01:34:32 -0000
@@ -61,7 +61,7 @@
                                  struct player *pplayer);
 static void server_set_tile_city(struct city *pcity, int city_x, int city_y,
                                 enum city_tile_type type);
-static bool update_city_tile_status(struct city *pcity, int city_x,
+static void update_city_tile_status(struct city *pcity, int city_x,
                                    int city_y);
 
 /****************************************************************************
@@ -1949,19 +1949,17 @@
   Updates the worked status of the tile (in map coordinates) for the city.
   If the status changes auto_arrange_workers may be called.  The caller needs
   to call sync_cities afterward for the affected city to be synced with the
-  client.
+  client.  auto_arrange_workers may be called within this function if a
+  worker is displaced.
 
-  It is safe to pass an out-of-range tile to this function.  The function
-  returns TRUE if the tile is made unavailable.
+  It is safe to pass an out-of-range tile to this function.
 ****************************************************************************/
-bool update_city_tile_status_map(struct city *pcity, struct tile *ptile)
+void update_city_tile_status_map(struct city *pcity, struct tile *ptile)
 {
   int city_x, city_y;
 
   if (map_to_city_map(&city_x, &city_y, pcity, ptile)) {
-    return update_city_tile_status(pcity, city_x, city_y);
-  } else {
-    return FALSE;
+    update_city_tile_status(pcity, city_x, city_y);
   }
 }
 
@@ -1970,15 +1968,12 @@
 city_x, city_y is in city map coords.
 You need to call sync_cities for the affected cities to be synced with the
 client.
-
-Returns TRUE iff a tile got available.
 **************************************************************************/
-static bool update_city_tile_status(struct city *pcity, int city_x,
+static void update_city_tile_status(struct city *pcity, int city_x,
                                    int city_y)
 {
   enum city_tile_type current;
   bool is_available;
-  bool result = FALSE;
 
   assert(is_valid_city_coords(city_x, city_y));
 
@@ -1999,7 +1994,9 @@
   case C_TILE_UNAVAILABLE:
     if (is_available) {
       server_set_tile_city(pcity, city_x, city_y, C_TILE_EMPTY);
-      result = TRUE;
+      /* We used to do an auto_arrange_workers in this case also.  But
+       * that's a bad idea since it will spuriously move around workers that
+       * have already been carefully placed. */
     }
     break;
 
@@ -2009,8 +2006,6 @@
     }
     break;
   }
-
-  return result;
 }
 
 /**************************************************************************
Index: server/citytools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.h,v
retrieving revision 1.62
diff -u -r1.62 citytools.h
--- server/citytools.h  16 Feb 2005 18:06:29 -0000      1.62
+++ server/citytools.h  8 Apr 2005 01:34:32 -0000
@@ -82,7 +82,7 @@
 bool city_can_work_tile(struct city *pcity, int city_x, int city_y);
 void server_remove_worker_city(struct city *pcity, int city_x, int city_y);
 void server_set_worker_city(struct city *pcity, int city_x, int city_y);
-bool update_city_tile_status_map(struct city *pcity, struct tile *ptile);
+void update_city_tile_status_map(struct city *pcity, struct tile *ptile);
 void sync_cities(void);
 bool can_place_worker_here(struct city *pcity, int city_x, int city_y);
 void check_city_workers(struct player *pplayer);
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.335
diff -u -r1.335 unittools.c
--- server/unittools.c  5 Apr 2005 20:36:10 -0000       1.335
+++ server/unittools.c  8 Apr 2005 01:34:33 -0000
@@ -2720,17 +2720,16 @@
   map_city_radius_iterate(src_tile, tile1) {
     struct city *pcity = map_get_city(tile1);
 
-    if (pcity && update_city_tile_status_map(pcity, src_tile)) {
-      auto_arrange_workers(pcity);
-      send_city_info(NULL, pcity);
+    if (pcity) {
+      update_city_tile_status_map(pcity, src_tile);
     }
   } map_city_radius_iterate_end;
   /* Then check cities near the destination. */
   map_city_radius_iterate(dst_tile, tile1) {
     struct city *pcity = map_get_city(tile1);
-    if (pcity && update_city_tile_status_map(pcity, dst_tile)) {
-      auto_arrange_workers(pcity);
-      send_city_info(NULL, pcity);
+
+    if (pcity) {
+      update_city_tile_status_map(pcity, dst_tile);
     }
   } map_city_radius_iterate_end;
   sync_cities();