[Freeciv-Dev] Re: (PR#13909) "you lost 2 units to an attack"

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Christian Knoke wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=13909 >
> 
> On Wed, Sep 07, 2005 at 01:01:24AM -0700, Jason Short wrote:
> 
>><URL: http://bugs.freeciv.org/Ticket/Display.html?id=13909 >
>>
>>"You lost 2 units to an attack from Liu Bang's Dragoons outside Beijing."
>>"Diplomat lost to an attack from Liu Bang's Dragoons."
>>"Musketeers lost to an attack by Liu Bang's Dragoons."
>>
>>The bug here is that these messages are in the reverse order - or at 
>>least they're shown in the reverse order in the messages dialog.
> 
> Maybe you can even combine them into a single message?

Here's a patch.

There are some drawbacks, as explained in the FIXME comment.  I think 
maybe there *should* be a "your combat" event that's separate from 
unit-lost-defense and unit-lost-attack events.  But those latter events 
should be disabled by default so only the combined message is normally 
displayed.

-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 18:59:35 -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] == punit);
+         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);
       }