Don't revert sectors without military to old owner

Change checksect() not to abandon occupied sectors to the old owner
when there is no military and no land units.  This effectively
restores pre-Chainsaw 3 behavior.  Matching change to would_abandon().

Rationale.  Traditional ways to change sector owner:

(1) Attack, assault, paradrop can transfer a sector to the attacker,
    in take_def().

(2) Guerrilla warfare at the update can transfer a sector to the old
    owner, in guerilla().  This happens when che kill all military and
    the sector is sufficiently disloyal to the owner.

(3) Whenever all civilians, military and land units are removed from a
    sector, no matter how, it silently reverts to the deity, in
    checksect().

Chainsaw 3 added:

(4) Whenever all military and land units are removed from an occupied
    sector, no matter how, it silently reverts to the old owner, in
    checksect().

This addition isn't seamless.  Funnies include:

* When che kill all military and land units, but the sector is loyal,
  (3) doesn't transfer to the old owner.  But since there's no
  military and land units left, (4) transfers it anyway, only without
  telling the lucky old owner.  The latter transfer is buggy:
  checksect() runs only on ef_read() and ef_write(), not the update
  (bug#1010856), so the silent transfer is delayed until the next
  ef_write().  But code using ef_read() sees it right away.  For
  instance, the path finder, which doesn't use ef_read(), can route a
  path through a sector lost that way.  The actual move, which does
  use ef_read(), then chokes on that path.

* When you attack a sector, and get defeated with the help of reacting
  land units, but succeed in killing the *local* defenders, (4) makes
  the sector silently revert to the old owner.  Which might be
  somebody who wasn't involved in the fight, and gets no notification
  whatsoever of his windfall.

* You can abandon a sector to the old-owner by removing all military
  and land units from it, in a myriad of ways.  Some ways ask you for
  confirmation (move, march, load), many don't (navigate, plane
  construction, delivery; arguably bugs), and others simply don't let
  you (paradrop, fly, distribution).

  This problem also exists for abandoning to deity, i.e. through (3)
  instead of (4).  Some ways to move out civilians don't let you do
  that (distribute), but most do.  However, accidentally abandoning an
  empty sector to deity is less serious than a populated one to
  another player.

In my opinion, (4) doesn't add much to the game, and fixing the
funnies isn't worth the effort.

diff --git a/src/lib/commands/move.c b/src/lib/commands/move.c
index e185b7734a037e581a84f63a11aeffad1d21cd3d..706020821580c5f85fb909c4f10d0992948a73c7 100644 (file)
--- a/src/lib/commands/move.c
+++ b/src/lib/commands/move.c
@@ -367,22 +367,19 @@ want_to_abandon(struct sctstr *sp, i_type vtype, int amnt, struct lndstr *lp)
 int
 would_abandon(struct sctstr *sp, i_type vtype, int amnt, struct lndstr *lp)
 {
-    int mil, loyalcivs;
+    int mil, civs;
 
     if (vtype != I_CIVIL && vtype != I_MILIT)
        return 0;
 
     mil = sp->sct_item[I_MILIT];
-    loyalcivs = sp->sct_item[I_CIVIL];
+    civs = sp->sct_item[I_CIVIL];
 
     if (vtype == I_MILIT)
        mil -= amnt;
     if (vtype == I_CIVIL)
-       loyalcivs -= amnt;
-    if (sp->sct_own != sp->sct_oldown)
-       loyalcivs = 0;
+       civs -= amnt;
 
-    return sp->sct_own != 0
-       && loyalcivs <= 0 && mil <= 0
+    return sp->sct_own != 0 && civs <= 0 && mil <= 0
        && !has_units(sp->sct_x, sp->sct_y, sp->sct_own, lp);
 }

diff --git a/src/lib/subs/sect.c b/src/lib/subs/sect.c
index f94dd0d4e015583980ed1f673cb129310acfc702..1aef5baa5eae7785593f1db123f4c29ea0027e83 100644 (file)
--- a/src/lib/subs/sect.c
+++ b/src/lib/subs/sect.c
@@ -86,7 +86,7 @@ item_prewrite(short *item)
 static int
 checksect(struct sctstr *sp)
 {
-    int mil, civs, loyalcivs;
+    int mil, civs;
     natid own;
 
     item_prewrite(sp->sct_item);
@@ -97,30 +97,22 @@ checksect(struct sctstr *sp)
 
     mil = sp->sct_item[I_MILIT];
     civs = sp->sct_item[I_CIVIL];
-    if (sp->sct_own == sp->sct_oldown)
-       loyalcivs = civs;
-    else
-       loyalcivs = 0;
 
     if (sp->sct_own != 0 && !civs) {
        sp->sct_work = 100;
        sp->sct_oldown = sp->sct_own;
     }
-    /* If they have a military unit there, they still own it */
-    if (sp->sct_own && !loyalcivs && !(sp->sct_flags & MOVE_IN_PROGRESS)) {
-       if (!mil && !has_units(sp->sct_x, sp->sct_y, sp->sct_own, 0)) {
-           /* more cruft! */
-           own = sp->sct_own;
-           if (sp->sct_oldown == sp->sct_own) {
-               makelost(EF_SECTOR, sp->sct_own, 0, sp->sct_x, sp->sct_y);
-               sp->sct_own = 0;
-               sp->sct_oldown = 0;
-           } else
-               takeover(sp, sp->sct_oldown);
-           sp->sct_mobil = 0;
-           if (sp->sct_type == SCT_CAPIT || sp->sct_type == SCT_MOUNT)
-               caploss(sp, own, "");
-       }
+
+    if (sp->sct_own && !civs && !mil
+       && !has_units(sp->sct_x, sp->sct_y, sp->sct_own, NULL)
+       && !(sp->sct_flags & MOVE_IN_PROGRESS)) {
+       /* more cruft! */
+       own = sp->sct_own;
+       makelost(EF_SECTOR, sp->sct_own, 0, sp->sct_x, sp->sct_y);
+       sp->sct_own = 0;
+       sp->sct_mobil = 0;
+       if (sp->sct_type == SCT_CAPIT || sp->sct_type == SCT_MOUNT)
+           caploss(sp, own, "");
     }
     return 1;
 }