[Freeciv-Dev] Re: (PR#4345) pcity->occupied not properly updated

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Jason Short wrote:
> [glip - Sun Jun  1 19:09:27 2003]:
> 
>>When the last unit moves out of the city, the "occupied" flag is not
>>removed.  I used Per's
>>      allywithallyofenemy.sav.gz
>>savegame (PR#4202) and GTK2 clients.  Connect as Mutsuhito and
>>Elizabeth.
> 
> 
> This is because, well, the occupied flag is not removed!
> 
> When your unit moves into sight of a foreign city, the server will send
> you a short city packet with the updated occupied flag.  But if a
> foreign unit moves into or out of the city (whether or not you can see
> the unit move) no new packet is sent.
> 
> The easiest solution is to just broadcast the information any time a
> unit moves into or out of a city.  Of course this is quite a bit
> inefficient...

This patch may be better; it avoids spurious rebroadcasting of packets 
every time units move in and out of cities...

jason

? rc
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.219
diff -u -r1.219 citytools.c
--- server/citytools.c  2003/06/04 19:24:36     1.219
+++ server/citytools.c  2003/07/16 03:32:19
@@ -1331,6 +1331,29 @@
 }
 
 /**************************************************************************
+  Update plrtile information about the city, and send out information to
+  the clients if it has changed.
+**************************************************************************/
+void refresh_dumb_city(struct city *pcity)
+{
+  players_iterate(pplayer) {
+    /* This loop includes the city owner, just for consistency. */
+    if (map_get_known_and_seen(pcity->x, pcity->y, pplayer)
+       || player_has_traderoute_with_city(pplayer, pcity)) {
+      if (update_dumb_city(pplayer, pcity)) {
+       struct packet_short_city sc_pack;
+
+       package_dumb_city(pplayer, pcity->x, pcity->y, &sc_pack);
+       lsend_packet_short_city(&pplayer->connections, &sc_pack);
+      }
+    }
+  } players_iterate_end;
+
+  /* Don't send to non-player observers since they don't have 'dumb city'
+   * information. */
+}
+
+/**************************************************************************
   Broadcast info about a city to all players who observe the tile. 
   If the player can see the city we update the city info first.
   If not we just use the info from the players private map.
@@ -1595,17 +1618,32 @@
 updates a players knowledge about a city. If the player_tile already
 contains a city it must be the same city (avoid problems by always calling
 reality_check city first)
+
+Returns TRUE iff anything has changed for the player city (i.e., if the
+client needs to be updated with a *short* city packet).  This information
+is only used in refresh_dumb_cities; elsewhere the data is (of necessity)
+broadcast regardless.
 **************************************************************************/
-void update_dumb_city(struct player *pplayer, struct city *pcity)
+bool update_dumb_city(struct player *pplayer, struct city *pcity)
 {
   struct player_tile *plrtile = map_get_player_tile(pcity->x, pcity->y,
                                                    pplayer);
-  struct dumb_city *pdcity;
+  struct dumb_city *pdcity = plrtile->city;
+
+  if (pdcity
+      && pdcity->id == pcity->id
+      && strcmp(pdcity->name, pcity->name) == 0
+      && pdcity->size == pcity->size
+      && pdcity->has_walls == city_got_citywalls(pcity)
+      && pdcity->occupied == pcity->occupied 
+      && pdcity->owner == pcity->owner) {
+    return FALSE;
+  }
+
   if (!plrtile->city) {
-    plrtile->city = fc_malloc(sizeof(struct dumb_city));
+    pdcity = plrtile->city = fc_malloc(sizeof(struct dumb_city));
     plrtile->city->id = pcity->id;
   }
-  pdcity = plrtile->city;
   if (pdcity->id != pcity->id) {
     freelog(LOG_ERROR, "Trying to update old city (wrong ID)"
            " at %i,%i for player %s", pcity->x, pcity->y, pplayer->name);
@@ -1617,6 +1655,8 @@
   pdcity->occupied =
       (unit_list_size(&(map_get_tile(pcity->x, pcity->y)->units)) > 0);
   pdcity->owner = pcity->owner;
+
+  return TRUE;
 }
 
 /**************************************************************************
Index: server/citytools.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.h,v
retrieving revision 1.45
diff -u -r1.45 citytools.h
--- server/citytools.h  2003/04/04 15:47:49     1.45
+++ server/citytools.h  2003/07/16 03:32:19
@@ -67,7 +67,8 @@
                  bool dipl_invest);
 
 void reality_check_city(struct player *pplayer,int x, int y);
-void update_dumb_city(struct player *pplayer, struct city *pcity);
+bool update_dumb_city(struct player *pplayer, struct city *pcity);
+void refresh_dumb_city(struct city *pcity);
 
 void create_city(struct player *pplayer, const int x, const int y,
                 const char *name);
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.229
diff -u -r1.229 unittools.c
--- server/unittools.c  2003/07/02 19:32:47     1.229
+++ server/unittools.c  2003/07/16 03:32:19
@@ -2770,6 +2770,7 @@
   struct player *pplayer = get_player(playerid);
   struct tile *psrctile = map_get_tile(src_x, src_y);
   struct tile *pdesttile = map_get_tile(dest_x, dest_y);
+  struct city *pcity;
 
   CHECK_MAP_POS(dest_x, dest_y);
 
@@ -2868,7 +2869,14 @@
       ) {
     set_unit_activity(punit, ACTIVITY_SENTRY);
   }
+
+  /* Send updated information to anyone watching.  If the unit moves
+   * in or out of a city we update the 'occupied' field. */
   send_unit_info_to_onlookers(NULL, punit, src_x, src_y, FALSE);
+  if ((pcity = map_get_city(src_x, src_y))
+      || (pcity = map_get_city(dest_x, dest_y))) {
+    refresh_dumb_city(pcity);
+  }
 
   /* The hidden units might not have been previously revealed 
    * because when we did the unfogging, the unit was still