Fix PoisonedBehavior damage-over-time ignoring POISON resistance (#1616) - #2987
Closed
Nagyhoho1234 wants to merge 2 commits into
Closed
Fix PoisonedBehavior damage-over-time ignoring POISON resistance (#1616)#2987Nagyhoho1234 wants to merge 2 commits into
Nagyhoho1234 wants to merge 2 commits into
Conversation
…SuperHackers#1616) PoisonedBehavior::update()'s periodic damage-over-time tick dealt DAMAGE_UNRESISTABLE instead of DAMAGE_POISON, so a target's actual POISON resistance (e.g. Chemical Suits) was never applied to it, even though the effect is conceptually poison damage. The existing developer comment explained this was to avoid PoisonedBehavior::onDamage() re-arming/ re-triggering itself off its own tick. Changed the periodic tick to deal real DAMAGE_POISON (gated behind #if !RETAIL_COMPATIBLE_CRC, since this changes actual damage dealt - a CRC-relevant simulation outcome - so it stays dormant by default for multiplayer/replay compatibility with retail 1.04/1.08). Replaced the old damage-type workaround with an explicit reentrancy guard: a new m_dealingPeriodicDamage member is set for the duration of the synchronous getObject()->attemptDamage() call in update(), and onDamage() ignores POISON damage while that flag is set. This could not be done by comparing damageInfo->in.m_sourceID against getObject()->getID() (self), since the periodic tick's damage.in.m_sourceID is m_poisonSource - the original poisoner, kept for XP-credit purposes (see the existing Stubbjax fix in startPoisonedEffects) - not the poisoned object itself; a sourceID-based self-check would never match and would also incorrectly suppress legitimate re-poisoning by repeated attacks from the same source. Not Core-shared; applied identically to both Generals and GeneralsMD. TheSuperHackers @BugFix ZsoltFeher 07/20/2026 Deal real POISON damage on the periodic tick so POISON resistance applies, guarded by a reentrancy flag instead of DAMAGE_UNRESISTABLE so it no longer re-triggers itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit 48078fb)
…follow-up) A fable-model review of commit 48078fb pointed out that the comment on the DAMAGE_POISON assignment still described the abandoned "self-source check" approach (comparing damageInfo->in.m_sourceID against the object's own ID), which was already known to be unsound - the periodic tick's damage.in.m_sourceID is m_poisonSource (the original attacker, kept for XP credit), not the poisoned object's own ID, so that comparison could never match. The actually-committed fix uses the m_dealingPeriodicDamage reentrancy flag instead; the code was already correct, only this one comment was left describing the wrong mechanism. Reviewed-by: fable (code-reviewer) TheSuperHackers @BugFix ZsoltFeher 07/20/2026 Comment described the abandoned self-source-check approach instead of the actual m_dealingPeriodicDamage reentrancy guard that was committed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> (cherry picked from commit ea5ce8a)
|
| Filename | Overview |
|---|---|
| Generals/Code/GameEngine/Include/GameLogic/Module/PoisonedBehavior.h | Adds the periodic poison reentrancy guard member. |
| Generals/Code/GameEngine/Source/GameLogic/Object/Behavior/PoisonedBehavior.cpp | Initializes the guard and applies poison-typed periodic damage outside retail-compatible builds. |
| GeneralsMD/Code/GameEngine/Include/GameLogic/Module/PoisonedBehavior.h | Mirrors the guard member addition for Zero Hour. |
| GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Behavior/PoisonedBehavior.cpp | Mirrors the periodic poison damage and guard behavior for Zero Hour. |
Reviews (1): Last reviewed commit: "Fix stale comment in PoisonedBehavior::u..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix PoisonedBehavior damage-over-time ignoring POISON resistance (#1616)
PoisonedBehavior::update()'s periodic damage-over-time tick dealt
DAMAGE_UNRESISTABLE instead of DAMAGE_POISON, so a target's actual POISON
resistance (e.g. Chemical Suits) was never applied to it, even though the
effect is conceptually poison damage. The existing developer comment
explained this was to avoid PoisonedBehavior::onDamage() re-arming/
re-triggering itself off its own tick.
Changed the periodic tick to deal real DAMAGE_POISON (gated behind
#if !RETAIL_COMPATIBLE_CRC, since this changes actual damage dealt - a
CRC-relevant simulation outcome - so it stays dormant by default for
multiplayer/replay compatibility with retail 1.04/1.08).
Replaced the old damage-type workaround with an explicit reentrancy guard:
a new m_dealingPeriodicDamage member is set for the duration of the
synchronous getObject()->attemptDamage() call in update(), and onDamage()
ignores POISON damage while that flag is set. This could not be done by
comparing damageInfo->in.m_sourceID against getObject()->getID() (self),
since the periodic tick's damage.in.m_sourceID is m_poisonSource - the
original poisoner, kept for XP-credit purposes (see the existing Stubbjax
fix in startPoisonedEffects) - not the poisoned object itself; a
sourceID-based self-check would never match and would also incorrectly
suppress legitimate re-poisoning by repeated attacks from the same source.
Not Core-shared; applied identically to both Generals and GeneralsMD.
TheSuperHackers @BugFix ZsoltFeher 07/20/2026 Deal real POISON damage on the
periodic tick so POISON resistance applies, guarded by a reentrancy flag
instead of DAMAGE_UNRESISTABLE so it no longer re-triggers itself.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Fix stale comment in PoisonedBehavior::update() (#1616 follow-up)
A fable-model review of commit 48078fb pointed out that the comment on
the DAMAGE_POISON assignment still described the abandoned "self-source
check" approach (comparing damageInfo->in.m_sourceID against the object's
own ID), which was already known to be unsound - the periodic tick's
damage.in.m_sourceID is m_poisonSource (the original attacker, kept for XP
credit), not the poisoned object's own ID, so that comparison could never
match. The actually-committed fix uses the m_dealingPeriodicDamage
reentrancy flag instead; the code was already correct, only this one
comment was left describing the wrong mechanism.
Reviewed-by: fable (code-reviewer)
TheSuperHackers @BugFix ZsoltFeher 07/20/2026 Comment described the
abandoned self-source-check approach instead of the actual
m_dealingPeriodicDamage reentrancy guard that was committed.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com
Fixes #1616