Refactored bounce_unit to allow reporting using function callbacks

server/citytools.cpp

@@ -624,7 +624,11 @@ static void transfer_unit(struct unit *punit, struct city *tocity,
 
       if (utype_has_flag(unit_type_get(punit), UTYF_GAMELOSS)) {
         // Try to save game loss units.
-        bounce_unit(punit, verbose);
+        if (verbose) {
+          bounce_unit(punit);
+        } else {
+          bounce_unit_silently(punit);
+        }
       } else {
         // Kill the unique unit.
 
@@ -755,7 +759,11 @@ void transfer_city_units(struct player *pplayer, struct player *pvictim,
         unit_list_remove(units, vunit);
       } else if (!pplayers_allied(pplayer, unit_owner(vunit))) {
         // the owner of vunit is allied to pvictim but not to pplayer
-        bounce_unit(vunit, verbose);
+        if (verbose) {
+          bounce_unit(vunit);
+        } else {
+          bounce_unit_silently(vunit);
+        }
       }
     }
     unit_list_iterate_safe_end;
@@ -794,7 +802,7 @@ void transfer_city_units(struct player *pplayer, struct player *pvictim,
       transfer_unit(vunit, pcity, true, verbose);
       if (unit_tile(vunit) == ptile && !pplayers_allied(pplayer, pvictim)) {
         // Unit is inside city being transferred, bounce it
-        bounce_unit(vunit, true);
+        bounce_unit(vunit);
       }
     } else {
       /* The unit is lost.  Call notify_player (in all other cases it is

server/diplomats.cpp

@@ -645,7 +645,7 @@ bool diplomat_bribe(struct player *pplayer, struct unit *pdiplomat,
       ((nullptr != pcity && !pplayers_allied(city_owner(pcity), pplayer))
        || 1 < unit_list_size(unit_tile(pvictim)->units));
   if (bounce) {
-    bounce_unit(pvictim, true);
+    bounce_unit(pvictim);
   }
 
   // This costs!

server/maphand.cpp

@@ -1788,7 +1788,20 @@ static void check_units_single_tile(struct tile *ptile)
   {
     if (unit_tile(punit) == ptile && !unit_transported(punit)
         && !can_unit_exist_at_tile(&(wld.map), punit, ptile)) {
-      bounce_unit(punit, true, bounce_reason::terrain_change, 1);
+      bounce_unit(
+          punit, 1,
+          [](struct bounce_event bevent) -> void {
+            notify_player(unit_owner(bevent.bunit), bevent.to_tile,
+                          E_UNIT_RELOCATED, ftc_server,
+                          _("Moved your %s due to changing terrain."),
+                          unit_link(bevent.bunit));
+          },
+          [](struct bounce_disband_event bevent) -> void {
+            notify_player(unit_owner(bevent.bunit), unit_tile(bevent.bunit),
+                          E_UNIT_LOST_MISC, ftc_server,
+                          _("Disbanded your %s due to changing terrain."),
+                          unit_tile_link(bevent.bunit));
+          });
     }
   }
   unit_list_iterate_safe_end;

server/savegame/savegame2.cpp

@@ -5065,7 +5065,7 @@ static void sg_load_sanitycheck(struct loaddata *loading)
                unit_rule_name(punit),
                terrain_rule_name(unit_tile(punit)->terrain),
                TILE_XY(unit_tile(punit)));
-        bounce_unit(punit, true);
+        bounce_unit(punit);
       }
     }
     unit_list_iterate_safe_end;

server/savegame/savegame3.cpp

@@ -7509,7 +7509,7 @@ static void sg_load_sanitycheck(struct loaddata *loading)
                unit_rule_name(punit),
                terrain_rule_name(unit_tile(punit)->terrain),
                TILE_XY(unit_tile(punit)));
-        bounce_unit(punit, true);
+        bounce_unit(punit);
       }
     }
     unit_list_iterate_safe_end;

server/unithand.cpp

@@ -349,7 +349,7 @@ static bool do_capture_units(struct player *pplayer, struct unit *punit,
 
     if (nullptr != pcity) {
       // The captured unit is in a city. Bounce it.
-      bounce_unit(to_capture, true);
+      bounce_unit(to_capture);
     }
   }
   unit_list_iterate_end;

server/unittools.cpp

@@ -15,6 +15,7 @@
 
 #include <cstdlib>
 #include <cstring>
+#include <functional>
 
 // utility
 #include "bitvector.h"
@@ -1319,18 +1320,33 @@ bool bounce_path_constraint::is_allowed(
 /**
  * Move or remove a unit due to stack conflicts. This function will try to
  * find a random safe tile within a given distance of the unit's current tile
- * and move the unit there. If no tiles are found, the unit is disbanded. If
- * 'verbose' is true, a message is sent to the unit owner regarding what
- * happened (the exact message depends on 'reason'). The maximum distance it
- * 2 by default for backward compatibility.
+ * and move the unit there. If no tiles are found, the unit is disbanded. The
+ * maximum distance it 2 by default for backward compatibility. A function
+ * reference may be provided to handle successfully finding a bounce
+ * location. By default this will report the bounce to the unit's owner
+ * before the unit is moved. A function reference may also be provided to
+ * handle failing to find a bounce location. By default this will report the
+ * failed bounce to the unit's owner before the unit is disbanded.
+ *
+ * See `bounce_unit_silently` to attempt a bounce without reporting it to the
+ * unit's owner.
+ *
+ * \note
+ * The unit may have died even if the bounce was successful.
+ *
+ * \note
+ * If a transport fails to bounce, all units in the transport will be
+ * bounced, triggering on_success or on_fail for each.
  */
-void bounce_unit(struct unit *punit, bool verbose, bounce_reason reason,
-                 int max_distance)
+void bounce_unit(struct unit *punit, int max_distance,
+                 std::function<void(struct bounce_event)> on_success,
+                 std::function<void(struct bounce_disband_event)> on_failure)
 {
   if (!punit) {
     return;
   }
 
+  const int unit_id = punit->id;
   const auto pplayer = unit_owner(punit);
   const auto punit_tile = unit_tile(punit);
 
@@ -1356,20 +1372,8 @@ void bounce_unit(struct unit *punit, bool verbose, bounce_reason reason,
     const auto path = paths[fc_rand(paths.size())];
     const auto steps = path.steps();
     const auto end_tile = path.steps().back().location;
-
-    if (verbose) {
-      switch (reason) {
-      case bounce_reason::generic:
-        notify_player(pplayer, end_tile, E_UNIT_RELOCATED, ftc_server,
-                      // TRANS: A unit is moved to resolve stack conflicts.
-                      _("Moved your %s."), unit_link(punit));
-        break;
-      case bounce_reason::terrain_change:
-        notify_player(pplayer, end_tile, E_UNIT_RELOCATED, ftc_server,
-                      _("Moved your %s due to changing terrain."),
-                      unit_link(punit));
-        break;
-      }
+    if (on_success) {
+      on_success({.bunit = punit, .to_tile = end_tile});
     }
 
     // Execute the orders making up the path. See control.cpp in the client
@@ -1394,6 +1398,10 @@ void bounce_unit(struct unit *punit, bool verbose, bounce_reason reason,
 
       handle_unit_orders(pplayer, &packet);
 
+      if (!unit_exists(unit_id)) {
+        return; // Unit died while executing orders
+      }
+
       // Restore unit wait time
       punit->action_timestamp = timestamp;
 
@@ -1405,31 +1413,70 @@ void bounce_unit(struct unit *punit, bool verbose, bounce_reason reason,
     }
   }
 
+  if (on_failure) {
+    on_failure({.bunit = punit});
+  }
+
   /* Didn't find a place to bounce the unit, going to disband it.
    * Try to bounce transported units. */
   if (0 < get_transporter_occupancy(punit)) {
     const auto pcargo_units = unit_transport_cargo(punit);
-    unit_list_iterate(pcargo_units, pcargo) { bounce_unit(pcargo, verbose); }
-    unit_list_iterate_end;
-  }
+    unit_list_iterate_safe(pcargo_units, pcargo)
+    {
+      bounce_unit(pcargo, max_distance, on_success, on_failure);
+    }
+    unit_list_iterate_safe_end;
 
-  if (verbose) {
-    switch (reason) {
-    case bounce_reason::generic:
-      notify_player(pplayer, punit_tile, E_UNIT_LOST_MISC, ftc_server,
-                    // TRANS: A unit is disbanded to resolve stack conflicts.
-                    _("Disbanded your %s."), unit_tile_link(punit));
-      break;
-    case bounce_reason::terrain_change:
-      notify_player(pplayer, punit_tile, E_UNIT_LOST_MISC, ftc_server,
-                    _("Disbanded your %s due to changing terrain."),
-                    unit_tile_link(punit));
-      break;
+    if (!unit_exists(unit_id)) {
+      return; // Unit died while unloading cargo
     }
   }
+
   wipe_unit(punit, ULR_STACK_CONFLICT, nullptr);
 }
 
+/**
+ * Move or remove a unit due to stack conflicts. This does not report the
+ * move to the unit owner.
+ *
+ * See: `bounce_unit`
+ */
+void bounce_unit_silently(struct unit *punit, int max_distance)
+{
+  bounce_unit(punit, max_distance, nullptr, nullptr);
+}
+
+/**
+ * Reports that a unit was bounced due to stack conflicts to the unit's
+ * owner.
+ */
+void report_unit_bounced_to_resolve_stack_conflicts(
+    struct bounce_event bevent)
+{
+  notify_player(unit_owner(bevent.bunit), bevent.to_tile, E_UNIT_RELOCATED,
+                ftc_server,
+                // TRANS: A unit is moved to resolve stack conflicts.
+                _("Moved your %s."), unit_link(bevent.bunit));
+}
+
+/**
+ * Reports that a unit was disbanded due to stack conflicts to the unit's
+ * owner.
+ */
+void report_unit_disbanded_to_resolve_stack_conflicts(
+    struct bounce_disband_event bevent)
+{
+  notify_player(unit_owner(bevent.bunit), unit_tile(bevent.bunit),
+                E_UNIT_LOST_MISC, ftc_server,
+                // TRANS: A unit is moved to resolve stack conflicts.
+                _("Disbanded your %s."), unit_tile_link(bevent.bunit));
+}
+
+/**
+ * Checks if a unit (still) exists.
+ */
+bool unit_exists(int unit_id) { return !!idex_lookup_unit(&wld, unit_id); }
+
 /**
    Throw pplayer's units from non allied cities
 
@@ -1473,7 +1520,11 @@ static void throw_units_from_illegal_cities(struct player *pplayer,
     if (nullptr != pcity && !pplayers_allied(city_owner(pcity), pplayer)) {
       ptrans = unit_transport_get(punit);
       if (nullptr == ptrans || pplayer != unit_owner(ptrans)) {
-        bounce_unit(punit, verbose);
+        if (verbose) {
+          bounce_unit(punit);
+        } else {
+          bounce_unit_silently(punit);
+        }
       }
     }
   }
@@ -1514,7 +1565,11 @@ static void resolve_stack_conflicts(struct player *pplayer,
       {
         if (unit_owner(aunit) == pplayer || unit_owner(aunit) == aplayer
             || !can_unit_survive_at_tile(&(wld.map), aunit, ptile)) {
-          bounce_unit(aunit, verbose);
+          if (verbose) {
+            bounce_unit(aunit);
+          } else {
+            bounce_unit_silently(aunit);
+          }
         }
       }
       unit_list_iterate_safe_end;

server/unittools.h

@@ -12,6 +12,8 @@
       \____/        ********************************************************/
 #pragma once
 
+#include <functional>
+
 #include "fc_types.h"
 
 #include "packets.h" // enum unit_info_use
@@ -105,14 +107,26 @@ int get_unit_vision_at(struct unit *punit, const struct tile *ptile,
 void unit_refresh_vision(struct unit *punit);
 void unit_list_refresh_vision(struct unit_list *punitlist);
 
-/// Why do we need to bounce a unit?
-enum class bounce_reason {
-  generic,        ///< We just need to do it
-  terrain_change, ///< We need to do it because of changing terrain
+struct bounce_event {
+  struct unit *bunit;
+  struct tile *to_tile;
+};
+struct bounce_disband_event {
+  struct unit *bunit;
 };
-void bounce_unit(struct unit *punit, bool verbose,
-                 bounce_reason reason = bounce_reason::generic,
-                 int max_distance = 2);
+void report_unit_bounced_to_resolve_stack_conflicts(
+    struct bounce_event bevent);
+void report_unit_disbanded_to_resolve_stack_conflicts(
+    struct bounce_disband_event bevent);
+void bounce_unit(
+    struct unit *punit, int max_distance = 2,
+    std::function<void(struct bounce_event)> on_success =
+        report_unit_bounced_to_resolve_stack_conflicts,
+    std::function<void(struct bounce_disband_event)> on_failure =
+        report_unit_disbanded_to_resolve_stack_conflicts);
+void bounce_unit_silently(struct unit *punit, int max_distance = 2);
+bool unit_exists(int unit_id);
+
 bool unit_activity_needs_target_from_client(enum unit_activity activity);
 void unit_assign_specific_activity_target(struct unit *punit,
                                           enum unit_activity *activity,