Only send bombardment combat when targets are visible

Bombarding cities was very powerful because it was showing every unit
inside the city, something only diplomats and spies can normally do.
Stop sending this around.

Bombing a tile no longer shows every unit on the tile being bombarded
individually. Instead, an explosion is shown on top of the tile without
more information. This required adding a dedicated packet.

client/packhand.cpp

@@ -53,6 +53,7 @@
 #include "spaceship.h"
 #include "specialist.h"
 #include "style.h"
+#include "tile.h"
 #include "traderoutes.h"
 #include "unit.h"
 #include "unitlist.h"
@@ -539,6 +540,74 @@ void handle_unit_combat_info(const struct packet_unit_combat_info *packet)
   }
 }
 
+/**
+ * A bombardment packet. The server tells us the attacker and the bombarded
+ * tile.
+ */
+void handle_unit_bombard_info(int attacker_unit_id, int target_tile_id)
+{
+  const auto attacker = game_unit_by_number(attacker_unit_id);
+  const auto tile = index_to_tile(&(wld.map), target_tile_id);
+  if (!tile) {
+    // We do not do anything with a tile that doesn't exist/is unknown.
+    return;
+  }
+
+  /*
+   * Determine whether we should show something.
+   */
+  bool show = attacker && tile_visible_mapcanvas(unit_tile(attacker));
+  show |= tile_visible_mapcanvas(tile);
+
+  if (gui_options->auto_center_on_combat) {
+    // Only center the map if the player is involved.
+    // The attacking player is for sure involved.
+    bool involved = attacker && unit_owner(attacker) == client.conn.playing;
+
+    // Or if the player's territory gets bombed.
+    involved |= tile->owner == client.conn.playing;
+    involved |= tile->extras_owner == client.conn.playing;
+
+    // Or if the player's unit gets bombed.
+    unit_list_iterate(tile->units, bombed_unit)
+    {
+      if (unit_owner(bombed_unit) == client.conn.playing) {
+        involved = true;
+        break;
+      }
+    }
+    unit_list_iterate_end;
+
+    // Or also the player whose city gets bombed (borders may be disabled).
+    auto city = tile_city(tile);
+    involved |= city && city->owner == client.conn.playing;
+
+    if (involved) {
+      // Center the map
+      queen()->mapview_wdg->center_on_tile(unit_tile(attacker));
+      show = true;
+    }
+  }
+
+  /*
+   * Play the bombing animation.
+   */
+  if (show) {
+    // Play sound
+    if (attacker) {
+      audio_play_sound(unit_type_get(attacker)->sound_fight,
+                       unit_type_get(attacker)->sound_fight_alt);
+    }
+
+    // Display it
+    set_units_in_combat(attacker, nullptr);
+    animate_unit_explosion(tile);
+    set_units_in_combat(nullptr, nullptr);
+    refresh_tile_mapcanvas(tile, false);
+    flush_dirty_overview();
+  }
+}
+
 /**
    Updates a city's list of improvements from packet data.
    "have_impr" specifies whether the improvement should be added (TRUE)

common/networking/packets.def

@@ -1088,6 +1088,11 @@ PACKET_UNIT_COMBAT_INFO = 65; sc, lsend
   BOOL make_def_veteran;
 end
 
+PACKET_UNIT_BOMBARD_INFO = 66; sc, lsend, cap(bombard-info)
+  UNIT attacker_unit_id;
+  TILE target_tile;
+end
+
 PACKET_UNIT_SSCS_SET = 71; cs, dsend
   UNIT unit_id;
   UNIT_DATA_TYPE type;

server/unithand.cpp

@@ -3531,58 +3531,82 @@ void handle_unit_change_activity(struct player *pplayer, int unit_id,
 }
 
 /**
-   Make sure everyone who can see combat does.
+ * Make sure everyone who can see combat does.
+ *
+ * This includes a 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.
+ *
+ * Note these packets must be sent out before unit_versus_unit is called, so
+ * that the original unit stats (HP) will be sent.
  */
-static void see_combat(struct unit *pattacker, struct unit *pdefender)
+static void see_combat_unit(struct unit *punit)
 {
-  struct packet_unit_short_info unit_att_short_packet, unit_def_short_packet;
-  struct packet_unit_info unit_att_packet, unit_def_packet;
+  packet_unit_short_info short_packet;
+  packet_unit_info full_packet;
 
-  /*
-   * 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.
-   *
-   * Note these packets must be sent out before unit_versus_unit is called,
-   * so that the original unit stats (HP) will be sent.
-   */
-  package_short_unit(pattacker, &unit_att_short_packet, UNIT_INFO_IDENTITY,
-                     0);
-  package_short_unit(pdefender, &unit_def_short_packet, UNIT_INFO_IDENTITY,
-                     0);
-  package_unit(pattacker, &unit_att_packet);
-  package_unit(pdefender, &unit_def_packet);
+  package_short_unit(punit, &short_packet, UNIT_INFO_IDENTITY, 0);
+  package_unit(punit, &full_packet);
 
   conn_list_iterate(game.est_connections, pconn)
   {
     struct player *pplayer = pconn->playing;
 
     if (pplayer != nullptr) {
-      /* NOTE: this means the player can see combat between submarines even
-       * if neither sub is visible.  See similar comment in send_combat. */
-      if (map_is_known_and_seen(unit_tile(pattacker), pplayer, V_MAIN)
-          || map_is_known_and_seen(unit_tile(pdefender), pplayer, V_MAIN)) {
-        /* Units are sent even if they were visible already. They may
-         * have changed orientation for combat. */
-        if (pplayer == unit_owner(pattacker)) {
-          send_packet_unit_info(pconn, &unit_att_packet);
-        } else {
-          send_packet_unit_short_info(pconn, &unit_att_short_packet, false);
-        }
-
-        if (pplayer == unit_owner(pdefender)) {
-          send_packet_unit_info(pconn, &unit_def_packet);
+      // NOTE: this means the player can see combat between submarines even
+      // if neither sub is visible. See similar comment in send_combat.
+      if (map_is_known_and_seen(unit_tile(punit), pplayer, V_MAIN)) {
+        // Units are sent even if they were visible already. They may have
+        // changed orientation for combat.
+        if (players_on_same_team(pplayer, unit_owner(punit))) {
+          send_packet_unit_info(pconn, &full_packet);
         } else {
-          send_packet_unit_short_info(pconn, &unit_def_short_packet, false);
+          send_packet_unit_short_info(pconn, &short_packet, false);
         }
       }
     } else if (pconn->observer) {
       // Global observer sees everything...
-      send_packet_unit_info(pconn, &unit_att_packet);
-      send_packet_unit_info(pconn, &unit_def_packet);
+      send_packet_unit_info(pconn, &full_packet);
+    }
+  }
+  conn_list_iterate_end;
+}
+
+/**
+ * Send bombardment info to players.
+ */
+static void send_bombardment(const unit *pattacker, const tile *ptarget)
+{
+  struct packet_unit_bombard_info info;
+  info.attacker_unit_id = pattacker->id;
+  info.target_tile = ptarget->index;
+
+  players_iterate(other_player)
+  {
+    /* NOTE: this means the player can see combat between submarines even
+     * if neither sub is visible.  See similar comment in see_combat. */
+    if (map_is_known_and_seen(unit_tile(pattacker), other_player, V_MAIN)
+        || map_is_known_and_seen(ptarget, other_player, V_MAIN)) {
+      lsend_packet_unit_bombard_info(other_player->connections, &info);
+
+      // Remove the client knowledge of the units. This corresponds to the
+      // send_packet_unit_short_info calls in see_combat.
+      if (!can_player_see_unit(other_player, pattacker)) {
+        unit_goes_out_of_sight(other_player, pattacker);
+      }
+    }
+  }
+  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.est_connections, pconn)
+  {
+    if (nullptr == pconn->playing && pconn->observer) {
+      send_packet_unit_bombard_info(pconn, &info);
     }
   }
   conn_list_iterate_end;
@@ -3592,7 +3616,7 @@ static void see_combat(struct unit *pattacker, struct unit *pdefender)
    Send combat info to players.
  */
 static void send_combat(struct unit *pattacker, struct unit *pdefender,
-                        int att_veteran, int def_veteran, int bombard)
+                        int att_veteran, int def_veteran)
 {
   struct packet_unit_combat_info combat;
 
@@ -3614,7 +3638,7 @@ static void send_combat(struct unit *pattacker, struct unit *pdefender,
 
       /*
        * Remove the client knowledge of the units.  This corresponds to the
-       * send_packet_unit_short_info calls up above.
+       * send_packet_unit_short_info calls in see_combat.
        */
       if (!can_player_see_unit(other_player, pattacker)) {
         unit_goes_out_of_sight(other_player, pattacker);
@@ -3724,15 +3748,9 @@ static bool unit_bombard(struct unit *punit, struct tile *ptile,
           nation_adjective_for_player(pplayer),
           unit_name_translation(punit));
 
-      see_combat(punit, pdefender);
-
       punit->hp = att_hp;
       pdefender->hp = def_hp;
 
-      send_combat(punit, pdefender, 0, 0, 1);
-
-      send_unit_info(nullptr, pdefender);
-
       // May cause an incident
       action_consequence_success(paction, unit_owner(punit),
                                  unit_owner(pdefender), unit_tile(pdefender),
@@ -3741,6 +3759,19 @@ static bool unit_bombard(struct unit *punit, struct tile *ptile,
   }
   unit_list_iterate_safe_end;
 
+  // Notify the client
+  see_combat_unit(punit);
+  send_bombardment(punit, ptile);
+
+  // Send units about affected units
+  unit_list_iterate_safe(ptile->units, pdefender)
+  {
+    if (is_unit_reachable_at(pdefender, punit, ptile)) {
+      send_unit_info(nullptr, pdefender);
+    }
+  }
+  unit_list_iterate_safe_end;
+
   unit_did_action(punit);
   unit_forget_last_activity(punit);
   unit_attack_civilian_casualties(punit, pcity, paction, "bombard");
@@ -3989,7 +4020,8 @@ static bool do_attack(struct unit *punit, struct tile *def_tile,
     unit_transport_unload_send(punit);
   }
 
-  see_combat(punit, pdefender);
+  see_combat_unit(punit);
+  see_combat_unit(pdefender);
 
   punit->hp = att_hp;
   pdefender->hp = def_hp;
@@ -4021,7 +4053,7 @@ static bool do_attack(struct unit *punit, struct tile *def_tile,
                              def_tile, unit_link(pdefender));
 
   send_combat(punit, pdefender, punit->veteran - old_unit_vet,
-              pdefender->veteran - old_defender_vet, 0);
+              pdefender->veteran - old_defender_vet);
 
   // Neither died
   if (punit->hp > 0 && pdefender->hp > 0) {

utility/fc_version.h.in

@@ -14,7 +14,7 @@
 
 #define NETWORK_CAPSTRING                                                   \
   "+Freeciv21.21April13 killunhomed-is-game-info player-intel-visibility " \
-  "bought-shields"
+  "bought-shields bombard-info"
 
 #ifndef FOLLOWTAG
 #define FOLLOWTAG "S_HAXXOR"