[Freeciv-Dev] (PR#7341) new functions for players looking into cities

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=7341 >

This patch introduces two new functions to common/player:

--  can_player_see_units_in_city(pplayer, pcity)  --

This function returns TRUE if the player can see units inside this city. 
  Currently the owner and their allies can see this.

--  can_player_see_city_internals(pplayer, pcity)  --

This function returns TRUE if the player can see the city internals; 
that is, if the player gets a full city packet and can popu the city 
dialog.  Currently only the owner can see this.

-----

These functions are used in several places, cleaning up the logic that's 
in place now and fixing one bug (PR#7327).

In the future I would like to make changes to this logic.

- I'd propose that shared vision should allow 
can_player_see_units_in_city.  Of course, this is a question for the DB.

- Players on teams should allow can_player_see_city_internals.  This 
will require minor changes to the citydlg to prevent one player from 
trying to control their teammate's cities.  These changes might seem 
like a waste of time, but...

- When a diplomat investigates the city, this should give the diplomat's 
owner can_player_see_city_internals and can_player_see_units_in_city for 
the duration of the turn.  This should replace the hack that is in place 
now.  At the end of the turn this vision should be reset.  This will be 
a substantial improvement to the usability of investigations.

- Arguably all allies (or allies with shared vision) should have 
can_player_see_city_internals.  This is another question for the DB.

But these are just ideas for future work in this area.  For now, these 
functions are needed so that callers can follow consistent logic.

jason

Index: client/climisc.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/climisc.c,v
retrieving revision 1.126
diff -u -r1.126 climisc.c
--- client/climisc.c    2004/01/24 16:45:18     1.126
+++ client/climisc.c    2004/01/28 22:18:36
@@ -99,8 +99,10 @@
 
   pcity = map_get_city(x, y);
   if (pcity) {
-    pcity->client.occupied =
+    if (can_player_see_units_in_city(game.player_ptr, pcity)) {
+      pcity->client.occupied =
        (unit_list_size(&(map_get_tile(pcity->x, pcity->y)->units)) > 0);
+    }
 
     refresh_city_dialog(pcity);
     freelog(LOG_DEBUG, "map city %s, %s, (%d %d)", pcity->name,
Index: client/control.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/control.c,v
retrieving revision 1.124
diff -u -r1.124 control.c
--- client/control.c    2004/01/20 21:52:06     1.124
+++ client/control.c    2004/01/28 22:18:36
@@ -1302,7 +1302,7 @@
     }
   }
   /* Otherwise use popups. */
-  else if (pcity && game.player_idx==pcity->owner) {
+  else if (pcity && can_player_see_city_internals(game.player_ptr, pcity)) {
     popup_city_dialog(pcity, FALSE);
   }
   else if (unit_list_size(&ptile->units) == 0 && !pcity
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.345
diff -u -r1.345 packhand.c
--- client/packhand.c   2004/01/20 21:52:07     1.345
+++ client/packhand.c   2004/01/28 22:18:37
@@ -502,8 +502,10 @@
 
   /* Since we can see inside the city, just determine the client status
    * from what we know. */
+  assert(can_player_see_units_in_city(game.player_ptr, pcity));
   pcity->client.occupied =
       (unit_list_size(&(map_get_tile(pcity->x, pcity->y)->units)) > 0);
+
   pcity->client.happy = city_happy(pcity);
   pcity->client.unhappy = city_unhappy(pcity);
 
@@ -1049,16 +1051,15 @@
       do_move_unit(punit, packet_unit, carried);
 
       if(pcity)  {
-       /* Unit moved out of a city - update the occupied status.  The
-        * logic is a little shaky since it's not clear whether we can
-        * see the internals of the city or not; however, the server should
-        * send us a city update to clear things up. */
-       bool new_occupied =
-         (unit_list_size(&(map_get_tile(pcity->x, pcity->y)->units)) > 0);
-
-       if (pcity->client.occupied != new_occupied) {
-         pcity->client.occupied = new_occupied;
-         refresh_tile_mapcanvas(pcity->x, pcity->y, FALSE);
+       if (can_player_see_units_in_city(game.player_ptr, pcity)) {
+         /* Unit moved out of a city - update the occupied status. */
+         bool new_occupied =
+           (unit_list_size(&(map_get_tile(pcity->x, pcity->y)->units)) > 0);
+
+         if (pcity->client.occupied != new_occupied) {
+           pcity->client.occupied = new_occupied;
+           refresh_tile_mapcanvas(pcity->x, pcity->y, FALSE);
+         }
        }
 
         if(pcity->id==punit->homecity)
@@ -1068,10 +1069,12 @@
       }
       
       if((pcity=map_get_city(punit->x, punit->y)))  {
-       /* Unit moved into a city - obviously it's occupied. */
-       if (!pcity->client.occupied) {
-         pcity->client.occupied = TRUE;
-         refresh_tile_mapcanvas(pcity->x, pcity->y, FALSE);
+       if (can_player_see_units_in_city(game.player_ptr, pcity)) {
+         /* Unit moved into a city - obviously it's occupied. */
+         if (!pcity->client.occupied) {
+           pcity->client.occupied = TRUE;
+           refresh_tile_mapcanvas(pcity->x, pcity->y, FALSE);
+         }
        }
 
         if(pcity->id == punit->homecity)
Index: common/player.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.c,v
retrieving revision 1.130
diff -u -r1.130 player.c
--- common/player.c     2003/11/28 17:37:21     1.130
+++ common/player.c     2004/01/28 22:18:37
@@ -276,7 +276,7 @@
 
   return ((map_get_known(x, y, pplayer) == TILE_KNOWN)
          && player_can_see_unit_at_location(pplayer, punit, x, y)
-         && (!pcity || pplayers_allied(city_owner(pcity), pplayer)));
+         && (!pcity || can_player_see_units_in_city(pplayer, pcity)));
 }
 
 /***************************************************************
@@ -293,6 +293,32 @@
   }
 
   return can_player_see_unit_at(pplayer, punit, x, y);
+}
+
+/****************************************************************************
+  Return TRUE iff the player can see units in the city.  Either they
+  can see all units or none.
+
+  Note that can_player_see_city_internals => can_player_see_units_in_city.
+  Otherwise the player would not know anything about the city's units at
+  all.
+****************************************************************************/
+bool can_player_see_units_in_city(struct player *pplayer,
+                                 struct city *pcity)
+{
+  return (can_player_see_city_internals(pplayer, pcity)
+         || pplayers_allied(pplayer, city_owner(pcity)));
+}
+
+/****************************************************************************
+  Return TRUE iff the player can see the city's internals.  This means the
+  full city packet is sent to the client, who should then be able to popup
+  a dialog for it.
+****************************************************************************/
+bool can_player_see_city_internals(struct player *pplayer,
+                                  struct city *pcity)
+{
+  return (pplayer == city_owner(pcity));
 }
 
 /***************************************************************
Index: common/player.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.h,v
retrieving revision 1.110
diff -u -r1.110 player.h
--- common/player.h     2003/11/28 17:37:21     1.110
+++ common/player.h     2004/01/28 22:18:37
@@ -232,6 +232,11 @@
 bool can_player_see_unit_at2(struct player *pplayer, struct unit *punit,
                             int x, int y);
 
+bool can_player_see_units_in_city(struct player *pplayer,
+                                 struct city *pcity);
+bool can_player_see_city_internals(struct player *pplayer,
+                                  struct city *pcity);
+
 bool player_owns_city(struct player *pplayer, struct city *pcity);
 
 struct city *player_find_city_by_id(struct player *pplayer, int city_id);
Index: server/citytools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/citytools.c,v
retrieving revision 1.248
diff -u -r1.248 citytools.c
--- server/citytools.c  2004/01/28 18:03:15     1.248
+++ server/citytools.c  2004/01/28 22:18:38
@@ -1494,27 +1494,21 @@
   struct packet_city_info packet;
   struct packet_city_short_info sc_pack;
 
-  /* nocity_send is used to inhibit sending cities to the owner between
-   * turn updates
-   */
-  if (!nocity_send) {    /* first send all info to the owner */
-    update_dumb_city(powner, pcity);
-    package_city(pcity, &packet, FALSE);
-    lsend_packet_city_info(&powner->connections, &packet);
-  }
-
-  /* send to all others who can see the city: */
+  /* Send to everyone who can see the city. */
   players_iterate(pplayer) {
-    if (city_owner(pcity) == pplayer) {
-      /* Already sent above. */
-      continue;
-    }
-
-    if (map_is_known_and_seen(pcity->x, pcity->y, pplayer)
-       || player_has_traderoute_with_city(pplayer, pcity)) {
-      update_dumb_city(pplayer, pcity);
-      package_dumb_city(pplayer, pcity->x, pcity->y, &sc_pack);
-      lsend_packet_city_short_info(&pplayer->connections, &sc_pack);
+    if (can_player_see_city_internals(pplayer, pcity)) {
+      if (!nocity_send || pplayer != city_owner(pcity)) {
+       update_dumb_city(powner, pcity);
+       package_city(pcity, &packet, FALSE);
+       lsend_packet_city_info(&powner->connections, &packet);
+      }
+    } else {
+      if (map_is_known_and_seen(pcity->x, pcity->y, pplayer)
+         || player_has_traderoute_with_city(pplayer, pcity)) {
+       update_dumb_city(pplayer, pcity);
+       package_dumb_city(pplayer, pcity->x, pcity->y, &sc_pack);
+       lsend_packet_city_short_info(&pplayer->connections, &sc_pack);
+      }
     }
   } players_iterate_end;