tend: Fail more nicely when ships can't tend

Target ships that can't be tended at all are silently ignored.  This
may leave the player guessing why the command did nothing.  Report
them, but only when explicitly named.

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

diff --git a/src/lib/commands/tend.c b/src/lib/commands/tend.c
index a009c6597ca438dce2a5f32b62f57a5ff850f14f..6af48461a8af7a7fc5f99112f179722cdb9655ad 100644 (file)
--- a/src/lib/commands/tend.c
+++ b/src/lib/commands/tend.c
@@ -41,9 +41,9 @@
 #include "plague.h"
 #include "ship.h"
 
-static int can_tend_to(struct shpstr *, struct shpstr *);
+static int can_tend_to(struct shpstr *, int, struct shpstr *, int);
 static void expose_ship(struct shpstr *s1, struct shpstr *s2);
-static int tend_land(struct shpstr *tenderp, char *units);
+static int tend_land(struct shpstr *tenderp, int, char *units);
 
 int
 tend(void)
@@ -100,7 +100,8 @@ tend(void)
                continue;
            if (!check_ship_ok(&tender))
                return RET_SYN;
-           if (0 != (retval = tend_land(&tender, p)))
+           retval = tend_land(&tender, tenders.sel == NS_LIST, p);
+           if (retval)
                return retval;
            continue;
        }
@@ -135,13 +136,15 @@ tend(void)
            return RET_SYN;
        total = 0;
        while (nxtitem(&targets, &target)) {
-           if (ip->i_uid == I_CIVIL && tender.shp_own != target.shp_own)
-               continue;
            if (amt < 0) {
                /* take from target and give to tender */
-               if (!player->owner)
+               if (!player->owner) {
+                   if (targets.sel == NS_LIST)
+                       pr("You don't own ship #%d!\n", target.shp_uid);
                    continue;
-               if (!can_tend_to(&target, &tender))
+               }
+               if (!can_tend_to(&target, targets.sel == NS_LIST,
+                                &tender, tenders.sel == NS_LIST))
                    continue;
                ontarget = target.shp_item[ip->i_uid];
                if (ontarget == 0) {
@@ -157,7 +160,10 @@ tend(void)
                total += transfer;
            } else {
                /* give to target from tender */
-               if (!can_tend_to(&tender, &target))
+               if (!can_tend_to(&tender, tenders.sel == NS_LIST,
+                                &target, targets.sel == NS_LIST))
+                   continue;
+               if (ip->i_uid == I_CIVIL && tender.shp_own != target.shp_own)
                    continue;
                ontarget = target.shp_item[ip->i_uid];
                vbase = &mchr[(int)target.shp_type];
@@ -193,17 +199,51 @@ tend(void)
 }
 
 static int
-can_tend_to(struct shpstr *from, struct shpstr *to)
+can_tend_to(struct shpstr *from, int noisy_from,
+           struct shpstr *to, int noisy_to)
 {
-    if (!to->shp_own)
+    /*
+     * Careful: error messages must not disclose anything on foreign
+     * @to the player doesn't already know, or could trivially learn.
+     */
+    if (!to->shp_own) {
+       if (noisy_to)
+           pr("You don't own ship #%d!\n", to->shp_uid);
        return 0;
-    if (to->shp_own != player->cnum && !player->god
-       && relations_with(to->shp_own, player->cnum) < FRIENDLY)
+    }
+    if (from->shp_uid == to->shp_uid) {
+       if (noisy_from && noisy_to)
+           pr("%s won't tend to itself\n", prship(from));
        return 0;
-    if (from->shp_uid == to->shp_uid)
+    }
+    if (from->shp_x != to->shp_x || from->shp_y != to->shp_y) {
+       if (noisy_from && noisy_to) {
+           /* Don't disclose foreign @to exists elsewhere */
+           if (player->god || to->shp_own == player->cnum)
+               pr("%s is not in the same sector as %s\n",
+                  prship(to), prship(from));
+           else
+               pr("You don't own ship #%d!\n", to->shp_uid);
+       }
        return 0;
-    if (from->shp_x != to->shp_x || from->shp_y != to->shp_y)
+    }
+    if (!player->god && to->shp_own != player->cnum
+       && relations_with(to->shp_own, player->cnum) < FRIENDLY) {
+       if (noisy_to) {
+           /*
+            * Don't disclose unfriendly @to exists here unless
+            * lookout from @from would see it.
+            */
+           if ((mchr[from->shp_type].m_flags & M_SUB)
+               || (mchr[to->shp_type].m_flags & M_SUB))
+               pr("You don't own ship #%d!\n", from->shp_uid);
+           else
+               pr("You are not on friendly terms with"
+                  " the owner of ship #%d!\n",
+                  to->shp_uid);
+       }
        return 0;
+    }
     return 1;
 }
 
@@ -217,7 +257,7 @@ expose_ship(struct shpstr *s1, struct shpstr *s2)
 }
 
 static int
-tend_land(struct shpstr *tenderp, char *units)
+tend_land(struct shpstr *tenderp, int noisy, char *units)
 {
     struct nstr_item lni;
     struct nstr_item targets;
@@ -250,7 +290,8 @@ tend_land(struct shpstr *tenderp, char *units)
        if (!check_ship_ok(tenderp) || !check_land_ok(&land))
            return RET_SYN;
        while (nxtitem(&targets, &target)) {
-           if (!can_tend_to(tenderp, &target))
+           if (!can_tend_to(tenderp, noisy,
+                            &target, targets.sel == NS_LIST))
                continue;
 
            /* Fit unit on ship */

diff --git a/tests/load-tend/03-tend-1 b/tests/load-tend/03-tend-1
index a9f83e25feeb6be067e56f1fb26d5cea326f6b52..86bc2fb9b9ce4377ba5e339acedb9b75ee3bc410 100644 (file)
--- a/tests/load-tend/03-tend-1
+++ b/tests/load-tend/03-tend-1
@@ -43,7 +43,6 @@ tend f 150 1 n
 tend f 150 1 150
 | target not in same sector:
 tend f 150 1 0
-| usability: does nothing silently
 |
 || from own ships to own ships:
 | #150/151 give 4m each
@@ -142,7 +141,6 @@ tend land 151 301 151
 tend land 166 330 164
 | target not in same sector
 tend land 151 301 0
-| usability: does nothing silently
 |
 || lands that can't be tended
 | no lands match

diff --git a/tests/load-tend/journal.log b/tests/load-tend/journal.log
index d46a85a322ce2e123acc2c61903de555d6327430..a5d739710b65785291cfd6c6d3ee55901fd27ed0 100644 (file)
--- a/tests/load-tend/journal.log
+++ b/tests/load-tend/journal.log
@@ -2116,10 +2116,12 @@
     Play#1 output Play#1 6 0 420
     Play#1 input tend f 150 1 150
     Play#1 command tend
+    Play#1 output Play#1 1 cs   cargo ship (#150) won't tend to itself
     Play#1 output Play#1 1 0 total food transferred off of cs   cargo ship (#150)
     Play#1 output Play#1 6 0 419
     Play#1 input tend f 150 1 0
     Play#1 command tend
+    Play#1 output Play#1 1 ls   landing ship (#0) is not in the same sector as cs   cargo ship (#150)
     Play#1 output Play#1 1 0 total food transferred off of cs   cargo ship (#150)
     Play#1 output Play#1 6 0 418
     Play#1 input tend m 150/151 1 t
@@ -2144,6 +2146,8 @@
     Play#1 output Play#1 6 0 415
     Play#1 input tend f 166 1 160/162/163/164
     Play#1 command tend
+    Play#1 output Play#1 1 You don't own ship #160!
+    Play#1 output Play#1 1 You are not on friendly terms with the owner of ship #164!
     Play#1 output Play#1 1 2 total food transferred off of cs   cargo ship (#166)
     Play#1 output Play#1 6 0 414
     Play#1 input tend f 166 1 u
@@ -2168,6 +2172,8 @@
     Play#1 output Play#1 6 0 410
     Play#1 input tend c 166 1 160/162/163/164
     Play#1 command tend
+    Play#1 output Play#1 1 You don't own ship #160!
+    Play#1 output Play#1 1 You are not on friendly terms with the owner of ship #164!
     Play#1 output Play#1 1 0 total civilians transferred off of cs   cargo ship (#166)
     Play#1 output Play#1 6 0 409
     Play#1 input tend f 160/162/163/164 -1 166
@@ -2179,6 +2185,10 @@
     Play#1 output Play#1 6 0 408
     Play#1 input tend f 166 -1 160/162/163/164
     Play#1 command tend
+    Play#1 output Play#1 1 You don't own ship #160!
+    Play#1 output Play#1 1 You don't own ship #162!
+    Play#1 output Play#1 1 You don't own ship #163!
+    Play#1 output Play#1 1 You don't own ship #164!
     Play#1 output Play#1 1 0 total food transferred to cs   cargo ship (#166)
     Play#1 output Play#1 6 0 407
     Play#1 input tend h 170/171 1 150
@@ -2313,12 +2323,15 @@
     Play#1 output Play#1 6 0 392
     Play#1 input tend land 151 301 151
     Play#1 command tend
+    Play#1 output Play#1 1 cs   cargo ship (#151) won't tend to itself
     Play#1 output Play#1 6 0 391
     Play#1 input tend land 166 330 164
     Play#1 command tend
+    Play#1 output Play#1 1 You are not on friendly terms with the owner of ship #164!
     Play#1 output Play#1 6 0 390
     Play#1 input tend land 151 301 0
     Play#1 command tend
+    Play#1 output Play#1 1 ls   landing ship (#0) is not in the same sector as cs   cargo ship (#151)
     Play#1 output Play#1 6 0 389
     Play#1 input tend land 150 n 151
     Play#1 command tend
@@ -2455,6 +2468,7 @@
     Play#0 output Play#0 6 0 639
     Play#0 input tend f 166 1 160/164
     Play#0 command tend
+    Play#0 output Play#0 1 You don't own ship #160!
     Play#0 output Play#0 1 1 total food transferred off of cs   cargo ship (#166)
     Play#0 output Play#0 6 0 638
     Play#0 input tend c 160/164 1 166
@@ -2464,6 +2478,7 @@
     Play#0 output Play#0 6 0 637
     Play#0 input tend c 166 1 160/164
     Play#0 command tend
+    Play#0 output Play#0 1 You don't own ship #160!
     Play#0 output Play#0 1 0 total civilians transferred off of cs   cargo ship (#166)
     Play#0 output Play#0 6 0 636
     Play#0 input tend land 163/164 313/314 166
@@ -2475,6 +2490,7 @@
     Play#0 output Play#0 6 0 635
     Play#0 input tend land 162 312 160
     Play#0 command tend
+    Play#0 output Play#0 1 You don't own ship #160!
     Play#0 output Play#0 6 0 634
     Play#0 input ctld
     Play#0 output Play#0 1 Bye-bye