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

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Marko Lindqvist wrote:
> Jason Short wrote:
> 
> 
>>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...
>> 
>>
> 
>  If min_dist_bw_cities=1 (civ1 rules), there can be city both in source 
> and destination tile. State of occupied flag should be sent fro both cities.

Hmm, I considered that but then decided to take the easy way out.  Thanks.

Here are updates of both variants, which I have called A and B.  Again, 
A is simple but leads to spurious packets being sent every time units 
move into/out of a city.  B is slightly more complicated and only sends 
update packets when needed.

jason

? rc
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:43:29
@@ -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,17 @@
       ) {
     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.  Note there may
+   * be cities at both src and dest under some rulesets. */
   send_unit_info_to_onlookers(NULL, punit, src_x, src_y, FALSE);
+  if ((pcity = map_get_city(src_x, src_y))) {
+    send_city_info(NULL, pcity); /* inefficient */
+  }
+  if ((pcity = map_get_city(dest_x, dest_y))) {
+    send_city_info(NULL, pcity); /* inefficient */
+  }
 
   /* The hidden units might not have been previously revealed 
    * because when we did the unfogging, the unit was still 

? 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:41:57
@@ -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:41:57
@@ -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:41:57
@@ -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,17 @@
       ) {
     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.  Note there may
+   * be cities at both src and dest under some rulesets. */
   send_unit_info_to_onlookers(NULL, punit, src_x, src_y, FALSE);
+  if ((pcity = map_get_city(src_x, src_y))) {
+    refresh_dumb_city(pcity);
+  }
+  if ((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