[Freeciv-Dev] Re: (PR#5071) Phantom unit in city

["Gregory Berkolaiko" <Gregory.Berkolaiko@xxxxxxxxxxxx>]

On Thu, 14 Aug 2003, Gregory Berkolaiko wrote:

> I traced this bug to handle_unit_info code.  A patch will follow soon.

Here is the patch.  Please check.

No, really.

I am not joking.

The underlying reason for the bug is that 
        player_can_see_unit_at_location 
is confusingly named.  What it does is only a check whether units' 
invisibility (like submarin's) is working.

So to avoid future mistakes I ask someone to make a patch 
1. Renaming
player_can_see_unit_at_location -> player_ignores_unit_invisibility_at
player_can_see_unit -> player_ignores_unit_invisibility
2. Removing TODO in can_player_see_unit_at header comment
3 (Optional) Renaming
can_player_see_unit_at -> player_can_see_unit_at

But only after the current patch is committed.

G.

Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.325
diff -u -r1.325 packhand.c
--- client/packhand.c   2003/08/12 17:17:57     1.325
+++ client/packhand.c   2003/08/14 13:27:04
@@ -997,19 +997,20 @@
       old_x = punit->x;
       old_y = punit->y;
       moved = TRUE;
-      
-      if(tile_get_known(packet->x, packet->y) == TILE_KNOWN
-         && player_can_see_unit_at_location(game.player_ptr, punit, 
-                                            packet->x, packet->y)) {
-       do_move_unit(punit, packet);
-       update_unit_focus();
-      }
-      else {
-       do_move_unit(punit, packet); /* nice to see where a unit is going */
+
+      /* nice to see where a unit is going */
+      do_move_unit(punit, packet);
+
+      if (!can_player_see_unit_at(game.player_ptr, punit, 
+                                 packet->x, packet->y)) {
+        /* Remove unit to avoid seeing phantoms */
        client_remove_unit(punit);
        refresh_tile_mapcanvas(packet->x, packet->y, FALSE);
         return;
       }
+
+      update_unit_focus();
+
       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
Index: common/player.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.c,v
retrieving revision 1.124
diff -u -r1.124 player.c
--- common/player.c     2003/08/08 22:11:41     1.124
+++ common/player.c     2003/08/14 13:27:05
@@ -257,6 +257,29 @@
   return player_can_see_unit_at_location(pplayer, punit, punit->x, punit->y);
 }
 
+/**************************************************************************
+  A helper function for send_info_to_onlookers below.  
+  Checks if a unit can be seen by pplayer at (x,y).
+  A player can see a unit if he:
+  (a) can see the tile AND
+  (b) can see the unit at the tile (i.e. unit not invisible at this tile) AND
+  (c) the unit is not in an unallied city
+
+  TODO: the name is confusingly similar to player_can_see_unit_at_location
+  But we need to rename p_c_s_u_a_t because it is really 
+  is_unit_visible_to_player_at or player_ignores_unit_invisibility_at.
+**************************************************************************/
+bool can_player_see_unit_at(struct player *pplayer, struct unit *punit,
+                            int x, int y)
+{
+  bool see_tile = (map_get_known2(x, y, pplayer) == TILE_KNOWN);
+  bool see_unit = player_can_see_unit_at_location(pplayer, punit, x, y);
+  struct city *pcity = map_get_city(x, y);
+  bool in_city = (pcity && !pplayers_allied(unit_owner(punit), pplayer));
+  
+  return (see_tile && see_unit && !in_city);
+}
+
 /***************************************************************
  If the specified player owns the city with the specified id,
  return pointer to the city struct.  Else return NULL.
Index: common/player.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/player.h,v
retrieving revision 1.104
diff -u -r1.104 player.h
--- common/player.h     2003/08/11 02:24:03     1.104
+++ common/player.h     2003/08/14 13:27:05
@@ -226,6 +226,8 @@
 bool player_can_see_unit_at_location(struct player *pplayer, 
                                      struct unit *punit, 
                                      int x, int y);
+bool can_player_see_unit_at(struct player *pplayer, struct unit *punit,
+                            int x, int y);
 
 bool player_owns_city(struct player *pplayer, struct city *pcity);
 
Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.240
diff -u -r1.240 unittools.c
--- server/unittools.c  2003/08/12 17:34:19     1.240
+++ server/unittools.c  2003/08/14 13:27:05
@@ -1852,34 +1852,6 @@
 }
 
 /**************************************************************************
-  A helper function for send_info_to_onlookers below.  
-  Checks if a unit can be seen by pplayer at (x,y).
-  A player can see a unit if he:
-  (a) can see the tile AND
-  (b) can see the unit at the tile (i.e. unit not invisible at this tile) AND
-  (c) the unit is not in an unallied city
-
-  TODO: the name is confusingly similar to player_can_see_unit_at_location
-  But we need to rename p_c_s_u_a_t because it is really 
-  is_unit_visible_to_player_at.
-**************************************************************************/
-static bool can_player_see_unit_at(struct player *pplayer, struct unit *punit,
-                                   int x, int y)
-{
-  if (!pplayer) {
-    freelog(LOG_ERROR, "NULL pointer in can_player_see_unit_at");
-    return FALSE;
-  } else {
-    bool see_tile = map_is_known_and_seen(x, y, pplayer);
-    bool see_unit = player_can_see_unit_at_location(pplayer, punit, x, y);
-    bool not_in_city = (pplayers_allied(unit_owner(punit), pplayer)
-                       || !map_get_city(x, y));
-
-    return (see_tile && see_unit && not_in_city);
-  }
-}
-
-/**************************************************************************
   Send the unit into to those connections in dest which can see the units
   at it's position, or the specified (x,y) (if different).
   Eg, use x and y as where the unit came from, so that the info can be