- don't let P_DamageMobj return negative values.

This serves no purpose, there's not a single place in the code which checks for it, but if that negative values goes unchecked to functions that expect an actually meaningful value for damage inflicted, some bad results can happen. If no damage is inflicted, a proper 0 needs to be returned so that the value can actually be worked with. The return value was a ZDoom invention, it is completely unclear why -1 was chosen if some kind of protection rendered the damage ineffective.

src/p_interaction.cpp

@@ -979,7 +979,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 
 	if (target == NULL || !((target->flags & MF_SHOOTABLE) || (target->flags6 & MF6_VULNERABLE)))
 	{ // Shouldn't happen
-		return -1;
+		return 0;
 	}
 
 	//Rather than unnecessarily call the function over and over again, let's be a little more efficient.
@@ -991,14 +991,14 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 	{
 		if (inflictor == NULL || !(inflictor->flags4 & MF4_SPECTRAL))
 		{
-			return -1;
+			return 0;
 		}
 	}
 	if (target->health <= 0)
 	{
 		if (inflictor && mod == NAME_Ice && !(inflictor->flags7 & MF7_ICESHATTER))
 		{
-			return -1;
+			return 0;
 		}
 		else if (target->flags & MF_ICECORPSE) // frozen
 		{
@@ -1006,7 +1006,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 			target->flags6 |= MF6_SHATTERING;
 			target->Vel.Zero();
 		}
-		return -1;
+		return 0;
 	}
 	// [MC] Changed it to check rawdamage here for consistency, even though that doesn't actually do anything
 	// different here. At any rate, invulnerable is being checked before type factoring, which is then being 
@@ -1027,7 +1027,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 					goto fakepain;
 				}
 				else
-					return -1;
+					return 0;
 			}
 		}
 		else
@@ -1038,7 +1038,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 				if (fakedPain)
 					plrDontThrust = 1;
 				else
-					return -1;
+					return 0;
 			}
 		}
 		
@@ -1068,7 +1068,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 		if (target->flags2 & MF2_DORMANT)
 		{
 			// Invulnerable, and won't wake up
-			return -1;
+			return 0;
 		}
 
 		if (!telefragDamage || (target->flags7 & MF7_LAXTELEFRAGDMG)) // TELEFRAG_DAMAGE may only be reduced with LAXTELEFRAGDMG or it may not guarantee its effect.
@@ -1086,19 +1086,19 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 					if (player != NULL)
 					{
 						if (!deathmatch && inflictor->FriendPlayer > 0)
-							return -1;
+							return 0;
 					}
 					else if (target->flags4 & MF4_SPECTRAL)
 					{
 						if (inflictor->FriendPlayer == 0 && !target->IsHostile(inflictor))
-							return -1;
+							return 0;
 					}
 				}
 
 				damage = inflictor->CallDoSpecialDamage(target, damage, mod);
 				if (damage < 0)
 				{
-					return -1;
+					return 0;
 				}
 			}
 
@@ -1141,7 +1141,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 					{
 						goto fakepain;
 					}
-					return -1;
+					return 0;
 				}
 			}
 		}
@@ -1153,7 +1153,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 	if (damage < 0)
 	{
 		// any negative value means that something in the above chain has cancelled out all damage and all damage effects, including pain.
-		return -1;
+		return 0;
 	}
 	// Push the target unless the source's weapon's kickback is 0.
 	// (i.e. Gauntlets/Chainsaw)
@@ -1244,7 +1244,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 			damage = (int)(damage * level.teamdamage);
 			if (damage < 0)
 			{
-				return damage;
+				return 0;
 			}
 			else if (damage == 0)
 			{
@@ -1256,7 +1256,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 				{
 					goto fakepain;
 				}
-				return -1;
+				return 0;
 			}
 		}
 	}
@@ -1294,14 +1294,14 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 				//Make sure no godmodes and NOPAIN flags are found first.
 				//Then, check to see if the player has NODAMAGE or ALLOWPAIN, or inflictor has CAUSEPAIN.
 				if ((player->cheats & CF_GODMODE) || (player->cheats & CF_GODMODE2) || (player->mo->flags5 & MF5_NOPAIN))
-					return -1;
+					return 0;
 				else if ((((player->mo->flags7 & MF7_ALLOWPAIN) || (player->mo->flags5 & MF5_NODAMAGE)) || ((inflictor != NULL) && (inflictor->flags7 & MF7_CAUSEPAIN))))
 				{
 					invulpain = true;
 					goto fakepain;
 				}
 				else
-					return -1;
+					return 0;
 			}
 			// Armor for players.
 			if (!(flags & DMG_NO_ARMOR) && player->mo->Inventory != NULL)
@@ -1321,7 +1321,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 				{
 					// [MC] Godmode doesn't need checking here, it's already being handled above.
 					if ((target->flags5 & MF5_NOPAIN) || (inflictor && (inflictor->flags5 & MF5_PAINLESS)))
-						return damage;
+						return 0;
 					
 					// If MF6_FORCEPAIN is set, make the player enter the pain state.
 					if ((inflictor && (inflictor->flags6 & MF6_FORCEPAIN)))
@@ -1332,7 +1332,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 						invulpain = true;
 						goto fakepain;
 					}
-					return damage;
+					return 0;
 				}
 			}
 			
@@ -1396,7 +1396,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 				if (fakedPain)
 					goto fakepain;
 				else
-					return damage;
+					return 0;
 			}
 		}
 	
@@ -1468,7 +1468,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 				}
 			}
 			target->CallDie (source, inflictor, flags);
-			return damage;
+			return MAX(0, damage);
 		}
 	}
 
@@ -1480,7 +1480,7 @@ static int DamageMobj (AActor *target, AActor *inflictor, AActor *source, int da
 		if (target->health <= woundhealth)
 		{
 			target->SetState (woundstate);
-			return damage;
+			return MAX(0, damage);
 		}
 	}
 
@@ -1579,9 +1579,9 @@ dopain:
 
 	if (invulpain) //Note that this takes into account all the cheats a player has, in terms of invulnerability.
 	{
-		return -1; //NOW we return -1!
+		return 0; //NOW we return -1!
 	}
-	return damage;
+	return MAX(0, damage);
 }
 
 DEFINE_ACTION_FUNCTION(AActor, DamageMobj)