Fix the "stuck pets" bug (github issue #329)

This commit is intended to fix the bug where a pet will get fixated on
an unmoving monster and stop moving itself. I described the cause in the
github issue; the gist is that the pet AI chooses the unmoving monster
as its ranged target, doesn't do anything when it calls mattackm
(because it doesn't have ranged attacks), then returns a value
indicating it didn't move and can't take further actions.

I initially implemented a fix that refactored mattackm to distinguish
between "attacker missed" and "attacker did nothing", which the pet AI
could then use to determine whether the pet could continue doing things.
But then I realized that if mattackm is called with non-adjacent
monsters, a return of MM_MISS more or less unambiguously indicates that
the attacker did nothing (because the ranged functions it calls like
breamm don't actually check to see whether the target was hit, just
whether the monster initiated the attack.) So, this only really needed
to check whether mattackm returned with MM_MISS.

I also found a probable bug in mattackm, in that the thrwmm call isn't
treated the same as breamm or spitmm. In the latter two, mattackm
returns MM_HIT even though it doesn't check whether the ranged attack
actually hit its target. But there was no logic doing the same for
thrwmm, so this commit also adds that. (Otherwise, a pet could possibly
use a ranged weapon attack and then get to keep moving on its turn.)

diff --git a/src/dogmove.c b/src/dogmove.c
index ad24f9674eb8329a6fbd33250b63fa3bc8a3c9c6..30a2f8725928939e8aa72406eefa2aaf18b3a888 100644 (file)
--- a/src/dogmove.c
+++ b/src/dogmove.c
@@ -1156,11 +1156,17 @@ int after; /* this is extra fast monster movement */
 
         /* Hungry pets are unlikely to use breath/spit attacks */
         if (mtarg && (!hungry || !rn2(5))) {
-            int mstatus;
+            int mstatus = MM_MISS;
 
             if (mtarg == &g.youmonst) {
                 if (mattacku(mtmp))
                     return 2;
+                /* Treat this as the pet having initiated an attack even if it
+                 * didn't, so it will lose its move. This isn't entirely fair,
+                 * but mattacku doesn't distinguish between "did not attack" and
+                 * "attacked but didn't die" cases, and this is preferable to
+                 * letting the pet attack the player and continuing to move */
+                mstatus = MM_HIT;
             } else {
                 mstatus = mattackm(mtmp, mtarg);
 
@@ -1187,7 +1193,18 @@ int after; /* this is extra fast monster movement */
                     }
                 }
             }
-            return 3;
+            /* Only return 3 if the pet actually made a ranged attack, and thus
+             * should lose the rest of its move.
+             * There's a chain of assumptions here:
+             * 1. score_targ and best_target will never select a monster that
+             *    can be attacked in melee, so the mattackm call can only ever
+             *    try ranged options
+             * 2. if the only attacks available to mattackm are ranged options,
+             *    and the monster cannot make a ranged attack, it will return
+             *    MM_MISS.
+             */
+            if (mstatus != MM_MISS)
+                return 3;
         }
     }
 

diff --git a/src/mhitm.c b/src/mhitm.c
index 37cf537eba7d6710198ec4660c9a12745569960e..e848c1d9947f9b1e67a3550e740e0e09ec3a4e23 100644 (file)
--- a/src/mhitm.c
+++ b/src/mhitm.c
@@ -368,6 +368,9 @@ register struct monst *magr, *mdef;
             if (distmin(magr->mx, magr->my, mdef->mx, mdef->my) > 1) {
                 /* D: Do a ranged attack here! */
                 strike = thrwmm(magr, mdef);
+                if (strike)
+                    /* We don't really know if we hit or not; pretend we did. */
+                    res[i] |= MM_HIT;
                 if (DEADMONSTER(mdef))
                     res[i] = MM_DEF_DIED;
                 if (DEADMONSTER(magr))