[Freeciv-Dev] Re: (PR#13912) duplicate orders-cancelled messages

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13912 >

Jason Short wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=13912 >
> 
> Now that these have the same event type they always show up right 
> together: two separate messages appear when orders are cancelled.  This 
> patch fixes it.

This patch fixes the bad assertion.

-jason

Index: server/unittools.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/server/unittools.c,v
retrieving revision 1.383
diff -p -u -r1.383 unittools.c
--- server/unittools.c  5 Sep 2005 15:55:47 -0000       1.383
+++ server/unittools.c  7 Sep 2005 20:37:23 -0000
@@ -1625,7 +1625,6 @@ void kill_unit(struct unit *pkiller, str
   struct player *pplayer   = unit_owner(punit);
   struct player *destroyer = unit_owner(pkiller);
   char *loc_str = get_location_str_in(pplayer, punit->tile);
-  int num_killed[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS];
   int ransom, unitcount = 0;
   
   /* barbarian leader ransom hack */
@@ -1660,45 +1659,87 @@ void kill_unit(struct unit *pkiller, str
     wipe_unit(punit);
   } else { /* unitcount > 1 */
     int i;
+    int num_killed[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS];
+    struct unit *other_killed[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS];
+
     if (!(unitcount > 1)) {
       die("Error in kill_unit, unitcount is %i", unitcount);
     }
     /* initialize */
     for (i = 0; i<MAX_NUM_PLAYERS+MAX_NUM_BARBARIANS; i++) {
       num_killed[i] = 0;
+      other_killed[i] = NULL;
     }
 
     /* count killed units */
     unit_list_iterate(punit->tile->units, vunit) {
       if (pplayers_at_war(unit_owner(pkiller), unit_owner(vunit))) {
        num_killed[vunit->owner->player_no]++;
+       if (vunit != punit) {
+         other_killed[vunit->owner->player_no] = vunit;
+       }
       }
     } unit_list_iterate_end;
 
-    /* inform the owners */
+    /* inform the owners: this only tells about owned units that were killed.
+     * there may have been 20 units who died but if only 2 belonged to the
+     * particular player they'll only learn about those.
+     *
+     * Also if a large number of units die you don't find out what type
+     * they all are. */
     for (i = 0; i<MAX_NUM_PLAYERS+MAX_NUM_BARBARIANS; i++) {
-      if (num_killed[i]>0) {
-       notify_player(get_player(i), punit->tile, E_UNIT_LOST,
-                        PL_("You lost %d unit to an attack "
-                            "from %s's %s%s.",
-                            "You lost %d units to an attack "
-                            "from %s's %s%s.",
-                            num_killed[i]), num_killed[i],
-                        destroyer->name, unit_name(pkiller->type),
-                        loc_str);
+      if (num_killed[i] == 1) {
+       if (i == punit->owner->player_no) {
+         assert(other_killed[i] == NULL);
+         notify_player(get_player(i), punit->tile, E_UNIT_LOST,
+                       /* TRANS: "Cannon lost to an attack from John's
+                        * Destroyer." */
+                       _("%s lost to an attack from %s's %s."),
+                       punit->type->name,
+                       destroyer->name, pkiller->type->name);
+       } else {
+         assert(other_killed[i] != punit);
+         notify_player(get_player(i), punit->tile, E_UNIT_LOST,
+                       /* TRANS: "Cannon lost when John's Destroyer
+                        * attacked Mark's Musketeers." */
+                       _("%s lost when %s's %s attacked %s's %s."),
+                       other_killed[i]->type->name,
+                       destroyer->name, pkiller->type->name,
+                       punit->owner->name, punit->type->name);
+       }
+      } else if (num_killed[i] > 1) {
+       if (i == punit->owner->player_no) {
+         int others = num_killed[i] - 1;
+
+         if (others == 1) {
+           notify_player(get_player(i), punit->tile, E_UNIT_LOST,
+                         _("%s (and %s) lost to an attack from %s's %s."),
+                         punit->type->name, other_killed[i]->type->name,
+                         destroyer->name, pkiller->type->name);
+         } else {
+           notify_player(get_player(i), punit->tile, E_UNIT_LOST,
+                         PL_("%s and %d other unit lost to an attack "
+                             "from %s's %s.",
+                             "%s and %d other units lost to an attack "
+                             "from %s's %s.", others),
+                         punit->type->name, others,
+                         destroyer->name, pkiller->type->name);
+         }
+       } else {
+         notify_player(get_player(i), punit->tile, E_UNIT_LOST,
+                       PL_("%d unit lost when %s's %s attacked %s's %s.",
+                           "%d units lost when %s's %s attacked %s's %s.",
+                           num_killed[i]),
+                       num_killed[i],
+                       destroyer->name, pkiller->type->name,
+                       punit->owner->name, punit->type->name);
+       }
       }
     }
 
     /* remove the units */
     unit_list_iterate(punit->tile->units, punit2) {
       if (pplayers_at_war(unit_owner(pkiller), unit_owner(punit2))) {
-       notify_player(unit_owner(punit2), 
-                        punit2->tile, E_UNIT_LOST,
-                        _("%s lost to an attack"
-                          " from %s's %s."),
-                        unit_type(punit2)->name, destroyer->name,
-                        unit_name(pkiller->type));
-
         gamelog(GAMELOG_UNITLOSS, punit2, destroyer);
        wipe_unit_spec_safe(punit2, FALSE);
       }
@@ -2461,6 +2502,16 @@ static bool unit_survive_autoattack(stru
   }
 }
 
+/****************************************************************************
+  Cancel orders for the unit.
+****************************************************************************/
+static void cancel_orders(struct unit *punit, char *dbg_msg)
+{
+  free_unit_orders(punit);
+  send_unit_info(NULL, punit);
+  freelog(LOG_DEBUG, "%s", dbg_msg);
+}
+
 /*****************************************************************
   Will wake up any neighboring enemy sentry units or patrolling 
   units.
@@ -2500,7 +2551,13 @@ static void wakeup_neighbor_sentries(str
       if (punit != ppatrol
          && unit_has_orders(ppatrol)
          && ppatrol->orders.vigilant) {
-       (void) maybe_cancel_patrol_due_to_enemy(ppatrol);
+       if (maybe_cancel_patrol_due_to_enemy(ppatrol)) {
+         cancel_orders(ppatrol, "  stopping because of nearby enemy");
+         notify_player(ppatrol->owner, ppatrol->tile, E_UNIT_ORDERS,
+                       _("Orders for %s aborted after enemy movement was "
+                         "spotted."),
+                       unit_name(ppatrol->type));
+       }
       }
     } unit_list_iterate_end;
   } square_iterate_end;
@@ -2846,18 +2903,8 @@ bool move_unit(struct unit *punit, struc
 static bool maybe_cancel_goto_due_to_enemy(struct unit *punit, 
                                            struct tile *ptile)
 {
-  struct player *pplayer = unit_owner(punit);
-  
-  if (is_non_allied_unit_tile(ptile, pplayer) 
-      || is_non_allied_city_tile(ptile, pplayer)) {
-    notify_player(pplayer, punit->tile, E_UNIT_ORDERS,
-                     _("%s aborted GOTO "
-                       "as there are units in the way."),
-                     unit_type(punit)->name);
-    return TRUE;
-  }
-
-  return FALSE;
+  return (is_non_allied_unit_tile(ptile, punit->owner) 
+         || is_non_allied_city_tile(ptile, punit->owner));
 }
 
 /**************************************************************************
@@ -2891,27 +2938,10 @@ static bool maybe_cancel_patrol_due_to_e
     }
   } square_iterate_end;
 
-  if (cancel) {
-    handle_unit_activity_request(punit, ACTIVITY_IDLE);
-    notify_player(unit_owner(punit), punit->tile, E_UNIT_ORDERS, 
-                    _("Your %s cancelled patrol order because it "
-                      "encountered a foreign unit."), unit_name(punit->type));
-  }
-
   return cancel;
 }
 
 /****************************************************************************
-  Cancel orders for the unit.
-****************************************************************************/
-static void cancel_orders(struct unit *punit, char *dbg_msg)
-{
-  free_unit_orders(punit);
-  send_unit_info(NULL, punit);
-  freelog(LOG_DEBUG, "%s", dbg_msg);
-}
-
-/****************************************************************************
   Executes a unit's orders stored in punit->orders.  The unit is put on idle
   if an action fails or if "patrol" is set and an enemy unit is encountered.