foxygit / doom Log in
commit 82f4fee5f1e353027ef3b5f4bff5cc497d32097c
Author:     Michael Day <contact@michaelcday.com>
AuthorDate: Mon Feb 19 13:48:33 2024 -0500
Commit:     Michael Day <contact@michaelcday.com>
CommitDate: Mon Feb 19 13:48:33 2024 -0500

    hexen/strife: Fix P_LookForPlayers infinite loop

    Fix potential lock-up that can occur when there are more than four
    players.
---
 src/hexen/p_enemy.c  | 29 +++++++++++++++++++++++++++++
 src/strife/p_enemy.c | 29 +++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/src/hexen/p_enemy.c b/src/hexen/p_enemy.c
index 4da539c1..fa8d0942 100644
--- a/src/hexen/p_enemy.c
+++ b/src/hexen/p_enemy.c
@@ -528,17 +528,46 @@ boolean P_LookForPlayers(mobj_t * actor, boolean allaround)
     player_t *player;
     angle_t an;
     fixed_t dist;
+    int consecutive_missing = 0; // for breaking infinite loop

     if (!netgame && players[0].health <= 0)
     {                           // Single player game and player is dead, look for monsters
         return (P_LookForMonsters(actor));
     }
     c = 0;
+
+    // The 3 below is probably a mistake (it should be MAXPLAYERS - 1, or 7)
+    // and in vanilla this can potentially cause an infinite loop in
+    // multiplayer. Unfortunately we can't correct the mistake - doing so will
+    // cause desyncs. Upon spawning, each enemy's lastlook is initialized to a
+    // random value between 0 and 7 (i.e MAXPLAYERS - 1). There's a chance
+    // that the first call of this function for that enemy will return early
+    // courtesy of the actor->lastlook == stop condition. In a single-player
+    // game this occurs when (actor->lastlook - 1) & 3 equals 0, or when
+    // lastlook equals 1 or 5.
+
+    // If you use MAXPLAYERS - 1, it has the side effect of altering which
+    // enemies are affected by an early actor->lastlook == stop return. Now it
+    // happens when (actor->lastlook - 1) & 7 equals 0, or when lastlook equals
+    // 1, *not* 1 and 5 as above.
+
     stop = (actor->lastlook - 1) & 3;
     for (;; actor->lastlook = (actor->lastlook + 1) & 3)
     {
         if (!playeringame[actor->lastlook])
+        {
+            // Break the vanilla infinite loop here. It can occur if there are
+            // > 4 players and players 0 - 3 all quit the game. Error out
+            // instead.
+            if (consecutive_missing == 4)
+            {
+                I_Error("P_LookForPlayers: No player 1 - 4.\n");
+            }
+            consecutive_missing++;
             continue;
+        }
+
+        consecutive_missing = 0;

         if (c++ == 2 || actor->lastlook == stop)
             return false;       // done looking
diff --git a/src/strife/p_enemy.c b/src/strife/p_enemy.c
index 213f7195..7763a87b 100644
--- a/src/strife/p_enemy.c
+++ b/src/strife/p_enemy.c
@@ -731,6 +731,7 @@ P_LookForPlayers
     angle_t     an;
     fixed_t     dist;
     mobj_t  *   master = players[actor->miscdata].mo;
+    int consecutive_missing = 0; // for breaking infinite loop

     // haleyjd 09/05/10: handle Allies
     if(actor->flags & MF_ALLY)
@@ -787,12 +788,40 @@ P_LookForPlayers
     }

     c = 0;
+
+    // The 3 below is probably a mistake (it should be MAXPLAYERS - 1, or 7)
+    // and in vanilla this can potentially cause an infinite loop in
+    // multiplayer. Unfortunately we can't correct the mistake - doing so will
+    // cause desyncs. Upon spawning, each enemy's lastlook is initialized to a
+    // random value between 0 and 7 (i.e MAXPLAYERS - 1). There's a chance
+    // that the first call of this function for that enemy will return early
+    // courtesy of the actor->lastlook == stop condition. In a single-player
+    // game this occurs when (actor->lastlook - 1) & 3 equals 0, or when
+    // lastlook equals 1 or 5.
+
+    // If you use MAXPLAYERS - 1, it has the side effect of altering which
+    // enemies are affected by an early actor->lastlook == stop return. Now it
+    // happens when (actor->lastlook - 1) & 7 equals 0, or when lastlook equals
+    // 1, *not* 1 and 5 as above.
+
     stop = (actor->lastlook-1)&3;

     for ( ; ; actor->lastlook = (actor->lastlook+1)&3 )
     {
         if (!playeringame[actor->lastlook])
+        {
+            // Break the vanilla infinite loop here. It can occur if there are
+            // > 4 players and players 0 - 3 all quit the game. Error out
+            // instead.
+            if (consecutive_missing == 4)
+            {
+                I_Error("P_LookForPlayers: No player 1 - 4.\n");
+            }
+            consecutive_missing++;
             continue;
+        }
+
+        consecutive_missing = 0;

         if (c++ == 2
             || actor->lastlook == stop)