[Freeciv-Dev] Re: (PR#7616) decrease_unit_hp_smooth is broken

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Jason Short wrote:

> This may not be the best solution.  Another solution would be to split
> up the sending of the unit packets, so the unit info packets would be
> sent before unit_versus_unit is called.  I think this would make the
> logic more confusing, though.  No doubt other solutions are possible as
> well.

OK, I take it back.  This solution is better.

This patch moves the unit packet sending up above unit_versus_unit. 
Only short packets are sent, and only to players who can't already see 
the unit (other players, obviously, already know about the unit).

Also can_player_see_unit_at is replaced with can_player_see_unit.  The 
handling of non-player observers is simplified a little.  (Except I'm 
pretty sure there are no non-player observers, right?)

jason

? Womoks
Index: server/unithand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unithand.c,v
retrieving revision 1.290
diff -u -r1.290 unithand.c
--- server/unithand.c   8 Mar 2004 03:00:22 -0000       1.290
+++ server/unithand.c   26 Mar 2004 21:35:40 -0000
@@ -612,7 +612,6 @@
   struct city *pcity;
   int moves_used, def_moves_used; 
   int def_x = pdefender->x, def_y = pdefender->y;
-  struct packet_unit_info unit_att_full_packet, unit_def_full_packet;
   struct packet_unit_short_info unit_att_short_packet, unit_def_short_packet;
   int old_unit_vet, old_defender_vet, vet;
 
@@ -653,6 +652,35 @@
   moves_used = unit_move_rate(punit) - punit->moves_left;
   def_moves_used = unit_move_rate(pdefender) - pdefender->moves_left;
 
+  /* 
+   * Special case for attacking/defending:
+   * 
+   * Normally the player doesn't get the information about the units inside a
+   * city. However for attacking/defending the player has to know the unit of
+   * the other side.  After the combat a remove_unit packet will be sent
+   * to the client to tidy up.
+   */
+  package_short_unit(punit, &unit_att_short_packet, FALSE,
+                    UNIT_INFO_IDENTITY, 0, FALSE);
+  package_short_unit(pdefender, &unit_def_short_packet, FALSE,
+                    UNIT_INFO_IDENTITY, 0, FALSE);
+  players_iterate(other_player) {
+    if (map_is_known_and_seen(punit->x, punit->y, other_player) ||
+       map_is_known_and_seen(def_x, def_y, other_player)) {
+      if (!can_player_see_unit(other_player, punit)) {
+       assert(other_player->player_no != punit->owner);
+       lsend_packet_unit_short_info(&other_player->connections,
+                                    &unit_att_short_packet);
+      }
+
+      if (!can_player_see_unit(other_player, pdefender)) {
+       assert(other_player->player_no != pdefender->owner);
+       lsend_packet_unit_short_info(&other_player->connections,
+                                    &unit_def_short_packet);
+      }
+    }
+  } players_iterate_end;
+
   old_unit_vet = punit->veteran;
   old_defender_vet = pdefender->veteran;
   unit_versus_unit(punit, pdefender);
@@ -698,60 +726,28 @@
   combat.defender_hp=pdefender->hp;
   combat.make_winner_veteran=vet;
 
-  package_unit(punit, &unit_att_full_packet, FALSE);
-  package_unit(pdefender, &unit_def_full_packet, FALSE);
-  package_short_unit(punit, &unit_att_short_packet, FALSE,
-                    UNIT_INFO_IDENTITY, 0, FALSE);
-  package_short_unit(pdefender, &unit_def_short_packet, FALSE,
-                    UNIT_INFO_IDENTITY, 0, FALSE);
-  
   players_iterate(other_player) {
     if (map_is_known_and_seen(punit->x, punit->y, other_player) ||
        map_is_known_and_seen(def_x, def_y, other_player)) {
-
-      /* 
-       * Special case for attacking/defending:
-       * 
-       * Normally the player doesn't get the information about the
-       * units inside a city. However for attacking/defending the
-       * player has to know the unit of the other side.
-       */
-
-      if (other_player->player_no == punit->owner) {
-       lsend_packet_unit_info(&other_player->connections,
-                              &unit_att_full_packet);
-      } else {
-       lsend_packet_unit_short_info(&other_player->connections,
-                                    &unit_att_short_packet);
-      }
-
-      if (other_player->player_no == pdefender->owner) {
-       lsend_packet_unit_info(&other_player->connections,
-                              &unit_def_full_packet);
-      } else {
-       lsend_packet_unit_short_info(&other_player->connections,
-                                    &unit_def_short_packet);
-      }
-
       lsend_packet_unit_combat_info(&other_player->connections, &combat);
 
       /* 
-       * Remove the client knowledge of the units. 
+       * Remove the client knowledge of the units.  This corresponds to the
+       * send_packet_unit_short_info calls up above.
        */
-      if (!can_player_see_unit_at(other_player, punit, punit->x, punit->y)) {
+      if (!can_player_see_unit(other_player, punit)) {
        unit_goes_out_of_sight(other_player, punit);
       }
-      if (!can_player_see_unit_at
-         (other_player, pdefender, pdefender->x, pdefender->y)) {
+      if (!can_player_see_unit(other_player, pdefender)) {
        unit_goes_out_of_sight(other_player, pdefender);
       }
     }
   } players_iterate_end;
 
+  /* Send combat info to non-player observers as well.  They already know
+   * about the unit so no unit_info is needed. */
   conn_list_iterate(game.game_connections, pconn) {
     if (!pconn->player && pconn->observer) {
-      send_packet_unit_info(pconn, &unit_att_full_packet);
-      send_packet_unit_info(pconn, &unit_def_full_packet);
       send_packet_unit_combat_info(pconn, &combat);
     }
   } conn_list_iterate_end;