update/revolt: Fix how land units take casualties

take_casualties() applies a number of casualties to sector military
and land units.  It is utterly confused about land units.

Consider a land unit with efficiency eff that has mil out of maxmil
military.  Issues:

* To apply N casualties without destroying it, take_casualties() tries
  to kill N * maxmil / mil military.  Makes no sense.  It's more than
  asked for unless mil equals maxmil.  It can even be more than mil.

  It reduces efficiency by N * 100 / mil points.  Note that ordinary
  ground combat reduces by N * 100 / maxmil points.  See
  lnd_take_casualty().

  Example: the update test's inf#25 is 100% efficient and has 20m out
  of 100m.  take_casualties() tries to apply up to 22 casualties out
  of the 60 remaining casualties to apply, but decides to apply only
  12 for now, to keep efficiency above to 40%.  It reduces efficiency
  by 12 * 100 / 20 = 60 to 40%, and tries to kill 12 * 100 / 20 = 60
  mil, which kills off the 20 that actually exist.  It nevertheless
  reduces the number of casualties still to apply only by 12.

  Example: the update test's linf#28 is 100% efficient and has 20m out
  of 25m.  take_casualties() tries to apply up to 8 casualties.  It
  reduces efficiency by 8 * 100 / 20 = 40 points to 60%, and tries to
  kill 8 * 25 / 20 = 10 military.

* When it destroys a land unit, it reduces the number of casualties
  still to apply by mil * eff/100.0 instead of mil.

  Example: the update test's inf#27 is 10% efficient and has 20m out
  of 100m.  take_casualties() still has 34 casualties to apply, so it
  destroys it, killing all 20m.  But it reduces the number of
  casualties to apply only by 2.

Broken when 4.0.0 made land unit military loadable.  Not sure it fully
worked before that, but it's definitely bonkers since.

Fix it as follows:

* To apply casualties to a land unit without destroying it, limit its
  losses to its actual number of military, and so that efficiency
  stays above 40%.  Then simply kill that number.

* Reduce the number of casualties to apply by the exact number killed.

The update test now kills only 8m in linf#28.  Still two more than it
should, but that's separate bug, to be fixed next.  The fix has no
visible effect for inf#25, because that one gets destroyed later.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>

diff --git a/src/lib/update/revolt.c b/src/lib/update/revolt.c
index 906fc79537a3c9c0d1c56a65c6261d667f0ad5e3..183289a6dfdd39172b64d2231af27c00f84807ed 100644 (file)
--- a/src/lib/update/revolt.c
+++ b/src/lib/update/revolt.c
@@ -419,7 +419,7 @@ take_casualties(struct sctstr *sp, int mc)
 {
     int orig_mil, taken;
     int cantake;
-    int nunits = 0, each, deq;
+    int nunits = 0, each, deq, dam;
     struct lndstr *lp;
     struct nstr_item ni;
 
@@ -451,7 +451,7 @@ take_casualties(struct sctstr *sp, int mc)
        nunits++;
     }
 
-    if (nunits == 0)
+    if (CANT_HAPPEN(!nunits))
        return taken;
 
     each = (mc / nunits) + 2;
@@ -467,20 +467,16 @@ take_casualties(struct sctstr *sp, int mc)
            continue;
 
        cantake = ((lp->lnd_effic - 40) / 100.0) * lp->lnd_item[I_MILIT];
+       cantake = MIN(lp->lnd_item[I_MILIT], cantake);
+       deq = MIN(cantake, each);
+       if (deq <= 0)
+           continue;
 
-       if (cantake >= each) {
-           deq = ((double)each / lp->lnd_item[I_MILIT]) * 100.0;
-           mc -= each;
-       } else if (cantake > 0) {
-           deq = ((double)cantake / lp->lnd_item[I_MILIT]) * 100.0;
-           mc -= cantake;
-       } else
-           deq = 0;
-
-       lp->lnd_effic -= deq;
-       lp->lnd_mobil -= deq / 2;
-       deq = lchr[(int)lp->lnd_type].l_item[I_MILIT] * (deq / 100.0);
-       taken += MIN(deq, lp->lnd_item[I_MILIT]);
+       mc -= deq;
+       taken += deq;
+       dam = ((double)deq / lp->lnd_item[I_MILIT]) * 100.0;
+       lp->lnd_effic -= dam;
+       lp->lnd_mobil -= dam / 2;
        lnd_submil(lp, deq);
        if (mc <= 0)
            return taken;
@@ -497,20 +493,16 @@ take_casualties(struct sctstr *sp, int mc)
            continue;
 
        cantake = ((lp->lnd_effic - 40) / 100.0) * lp->lnd_item[I_MILIT];
+       cantake = MIN(lp->lnd_item[I_MILIT], cantake);
+       deq = MIN(cantake, each);
+       if (deq <= 0)
+           continue;
 
-       if (cantake >= each) {
-           deq = ((double)each / lp->lnd_item[I_MILIT]) * 100.0;
-           mc -= each;
-       } else if (cantake > 0) {
-           deq = ((double)cantake / lp->lnd_item[I_MILIT]) * 100.0;
-           mc -= cantake;
-       } else
-           deq = 0;
-
-       lp->lnd_effic -= deq;
-       lp->lnd_mobil -= deq / 2;
-       deq = lchr[(int)lp->lnd_type].l_item[I_MILIT] * (deq / 100.0);
-       taken += MIN(deq, lp->lnd_item[I_MILIT]);
+       mc -= deq;
+       taken += deq;
+       dam = ((double)deq / lp->lnd_item[I_MILIT]) * 100.0;
+       lp->lnd_effic -= dam;
+       lp->lnd_mobil -= dam / 2;
        lnd_submil(lp, deq);
        if (mc <= 0)
            return taken;
@@ -527,7 +519,7 @@ take_casualties(struct sctstr *sp, int mc)
        if (lchr[(int)lp->lnd_type].l_flags & L_SECURITY)
            continue;
 
-       mc -= (lp->lnd_effic / 100.0) * lp->lnd_item[I_MILIT];
+       mc -= lp->lnd_item[I_MILIT];
        taken += lp->lnd_item[I_MILIT];
        lnd_dies_fighting_che(lp);
        if (mc <= 0)
@@ -545,14 +537,14 @@ take_casualties(struct sctstr *sp, int mc)
        if (!(lchr[(int)lp->lnd_type].l_flags & L_SECURITY))
            continue;
 
-       mc -= (lp->lnd_effic / 100.0) * lp->lnd_item[I_MILIT];
+       mc -= lp->lnd_item[I_MILIT];
        taken += lp->lnd_item[I_MILIT];
        lnd_dies_fighting_che(lp);
        if (mc <= 0)
            return taken;
     }
 
-    /* Hmm.. everyone dead.. too bad */
+    CANT_REACH();
     return taken;
 }
 

diff --git a/tests/update/99-POGO b/tests/update/99-POGO
index 8e9e6c00bc151bd3da73cd192347f78db1dd7ac5..28e00ad9d50d0f3e83de07936b4861b32fa48202 100644 (file)
--- a/tests/update/99-POGO
+++ b/tests/update/99-POGO
@@ -182,7 +182,7 @@ land 0:31,-16:-1
 | BUG: "Sector -12,-8 has been retaken!" instead of takeover
 | -10,-8 che win, don't take over
 | -8,-8 che lose
-| BUG: che kill 4m more than they should
+| BUG: che kill 2m more than they should
 | -6,-8 che lose
 | BUG: inf#29 shouldn't die
 | plague stage 4 (dying) -16:-1,-6 ship#10 land#10

diff --git a/tests/update/final.xdump b/tests/update/final.xdump
index 69bc5d0f18a1cdbcb580ba198fedc85c54c26ae2..d821757357a0ff2cd1c275b045b020d140388954 100644 (file)
--- a/tests/update/final.xdump
+++ b/tests/update/final.xdump
@@ -381,7 +381,7 @@ uid owner xloc yloc type effic mobil off tech opx opy mission radius army ship h
 25 0 -12 -8 20 0 -30 0 170 0 0 none 0 "" -1 0 42 () "" 0 0 0 0 0 0 0 0 9 0 0 0 0 0 healthy 0 -1 0
 26 0 -12 -8 2 0 -30 0 50 0 0 none 0 "" -1 0 42 () "" 0 0 0 0 0 0 0 0 10 0 0 0 0 0 healthy 0 -1 0
 27 0 -12 -8 20 0 0 0 170 0 0 none 0 "" -1 0 42 () "" 0 0 0 0 0 0 0 0 9 0 0 0 0 0 healthy 0 -1 0
-28 4 -8 -8 1 60 40 0 50 0 0 none 0 "" -1 0 42 () "" 0 10 0 0 0 0 0 0 9 0 0 0 0 0 healthy 0 -1 0
+28 4 -8 -8 1 60 40 0 50 0 0 none 0 "" -1 0 42 () "" 0 12 0 0 0 0 0 0 9 0 0 0 0 0 healthy 0 -1 0
 29 0 -6 -8 2 0 0 0 50 0 0 none 0 "" -1 0 42 () "" 0 0 0 0 0 0 0 0 8 0 0 0 0 0 healthy 0 -1 0
 30 2 -16 0 2 88 60 0 50 0 0 none 0 "" -1 0 42 () "" 0 100 0 0 0 0 0 0 7 0 0 0 0 0 healthy 0 -1 0
 31 2 -16 0 2 10 60 0 50 0 0 none 0 "" -1 0 42 () "" 0 100 0 0 0 0 0 0 7 0 0 0 0 0 healthy 0 -1 0

diff --git a/tests/update/journal.log b/tests/update/journal.log
index b0c472f83607e2accfae98e7659442d86a4b657f..4d84ded7ab1e1f6d9c99d09fd11ae92f61829e91 100644 (file)
--- a/tests/update/journal.log
+++ b/tests/update/journal.log
@@ -1374,7 +1374,7 @@
     Play#0 output Play#0 1 Guerrilla warfare in -10,-8
     Play#0 output Play#0 1   body count: troops: 10, rebels: 5
     Play#0 output Play#0 1 Guerrilla warfare in -8,-8
-    Play#0 output Play#0 1   body count: troops: 11, rebels: 22
+    Play#0 output Play#0 1   body count: troops: 9, rebels: 22
     Play#0 output Play#0 1 inf  infantry #29 dies fighting guerrillas in -6,-8
     Play#0 output Play#0 1 Guerrilla warfare in -6,-8
     Play#0 output Play#0 1   body count: troops: 50, rebels: 52
@@ -1682,7 +1682,7 @@
     Play#0 output Play#0 1   4   19 inf  infantry    -10,-6      100% 100   0  60   7  50  42%  0  0
     Play#0 output Play#0 1   5   20 inf  infantry    -16,-8       20%   0   0  60  10  50  42%  0  0
     Play#0 output Play#0 1   5   23 inf  infantry    -14,-8       20%   0   0  60  10  50  42%  0  0
-    Play#0 output Play#0 1   4   28 linf light infa   -8,-8       60%  10   0  40   9  50  42%  0  0
+    Play#0 output Play#0 1   4   28 linf light infa   -8,-8       60%  12   0  40   9  50  42%  0  0
     Play#0 output Play#0 1 13 units
     Play#0 output Play#0 6 0 640
     Play#0 input read 5