From 2a6fafa15eab3958f4fa1f60d3080858e1e20d55 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sat, 21 Jan 2017 13:12:34 +0100 Subject: [PATCH] - 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 | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/p_interaction.cpp b/src/p_interaction.cpp index 0320b0feb..c33cfb080 100644 --- a/src/p_interaction.cpp +++ b/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)