Gotchas of `AActor::AbsorbDamage()` in `DamageMobj()`

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.
ZzZombo
Posts: 315
Joined: Mon Jul 16, 2012 2:02 am

Gotchas of `AActor::AbsorbDamage()` in `DamageMobj()`

Post by ZzZombo »

`AActor::AbsorbDamage()`:

Code: Select all

for (AActor *item = Inventory; item != nullptr; item = next)
It already performs all the work of traversing the inventory chain and calling `AbsorbDamage()` on each item, gracefully handling `nullptr` `Inventory`.

Now, enter `DamageMobj()`:

Code: Select all

		// Armor for monsters.
		if (!(flags & (DMG_NO_ARMOR|DMG_FORCED)) && target->Inventory != NULL && damage > 0)
If the actor lacks inventory, the function will refuse to call `AActor::AbsorbDamage()`, potentially skipping overridden in subclasses versions of the virtual function. Given there is no harm in just calling the latter this check could be omitted altogether while also fixing this subtle bug.

Also, as seen from the comment originally this piece of code was meant for armor items. However, since then `AActor::AbsorbDamage()` got exposed to ZScript and as far as I know, there is no requirement to adhere to use it strictly for that. Using `DMG_NO_ARMOR` bypasses any custom logic in overridden versions of the function, just like empty inventory. I suggest to make `DMG_NO_ARMOR` ignore any reduced damage returned from the called function, but still call it.

`DMG_FORCED` also subverts any custom handling of incoming damage in the virtual function, so I'd suggest again to make it ignore any reduced damage returned by it if it's in effect.

All these notes apply also to the player-specific handling above: https://github.com/ZDoom/gzdoom/blob/2c ... .cpp#L1311.
Professor Hastig
Posts: 237
Joined: Mon Jan 09, 2023 2:02 am
Graphics Processor: nVidia (Modern GZDoom)

Re: Gotchas of `AActor::AbsorbDamage()` in `DamageMobj()`

Post by Professor Hastig »

Can you write such code without breaking anything?
ZzZombo
Posts: 315
Joined: Mon Jul 16, 2012 2:02 am

Re: Gotchas of `AActor::AbsorbDamage()` in `DamageMobj()`

Post by ZzZombo »

I could, but first, I'd like to hear from the other developers what do they think. This is a critical code, so I'd tread lightly here.
User avatar
Major Cooke
Posts: 8182
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: Gotchas of `AActor::AbsorbDamage()` in `DamageMobj()`

Post by Major Cooke »

You can't just ignore the result, because the whole point of overriding is to put the power in the hands of the modder who has the final say for what the damage output is.

For example, some of my super bosses I only want the player to be able to deal any form of damage at all. That means disabling all damage and ignoring all flags from other monsters. If the internal function ignored this result, then what's the point of overriding the functions in the first place? I don't want ANYONE overriding MY DamageMobj overrides, which is how it should be. My mods, my rules.

That being said, I do wish Modify/AbsorbDamage() functions were called no matter what and just respected the flag on ZScript's side, but unfortunately I think changing them now may do more harm than good.
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49096
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: Gotchas of `AActor::AbsorbDamage()` in `DamageMobj()`

Post by Graf Zahl »

There's nothing wrong here.

The internal 'AbsorbDamage' function is merely a helper that isn't even natively virtual, much less scripted. It was merely separated out for keeping the code clean. Think of it as a static local function.

The 'duplicate' checks for existing inventory are just because there's also other things that depend on it.

Return to “Closed Bugs [GZDoom]”