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)