core/player: re-evaluate corruption penalties without re-applying them - #81
Draft
svenbledt wants to merge 2 commits into
Draft
core/player: re-evaluate corruption penalties without re-applying them#81svenbledt wants to merge 2 commits into
svenbledt wants to merge 2 commits into
Conversation
Player::UpdateCorruption had three problems, all of which left the CorruptionEffects.db2 threshold penalties out of step with the player. It re-applied live auras. The loop ended in an unconditional CastSpell(this, aura, true), so every corruption rating change re-cast penalties that were already present, restarting their durations and resetting their proc state. It now casts only what is missing and lets an aura that is already applied recalculate its own amounts in place. It was never re-evaluated on an area change. Each CorruptionEffects row carries a PlayerConditionID, and the sync ran only from UpdateRating(CR_CORRUPTION | CR_CORRUPTION_RESISTANCE) - a gear or rating-aura change. A player who walked into an area where the condition stopped matching kept the penalty indefinitely, and one who walked out never regained it. UpdateArea now schedules a sync. It was never re-evaluated on resurrect. RemoveAllAurasOnDeath strips the penalties and neither a rating change nor an area transition follows, so reviving where you fell left the player uncorrupted until they happened to cross an area border. That is why a corpse run appeared to fix it and .revive did not. Syncing at each mutation site is wrong in the other direction: a bulk item rebuild strips every item's corruption before re-applying it, so a per-mutation sync walks the total down to zero and tears down each tier's aura on the way. The sync is now coalesced behind a dirty flag and runs once per Player::Update tick. The two item-level rebuilds that did not bracket themselves with SetCanModifyStats now do, and UpdateItemLevelAreaBasedScaling settles its derived stats afterwards - without that the max health it scales the player to was one rebuild stale. GetEffectiveCorruption is split out of UpdateCorruption so callers that need the value do not repeat the rating subtraction. Verified in-game: penalties apply and remove as corruption crosses each threshold, survive a gear swap, and return on resurrect.
Corruption penalties are driven entirely by CorruptionEffects.db2, and when one fails to apply there is no way to see which of the three gates rejected it - the threshold, the PlayerConditionID, or the row being flagged disabled. The only alternative is equipping corrupted gear and inferring the answer from what happens. .debug corruption prints the selected player's effective corruption alongside the two ratings it derives from, then every CorruptionEffects row with its threshold, aura, condition, the verdict on that player and whether the aura is actually applied. A server whose client data has no CorruptionEffects rows at all reports that explicitly, since it is otherwise indistinguishable from a core fault. RBAC-gated behind RBAC_PERM_COMMAND_DEBUG like the rest of the table.
18 tasks
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.
Changes Proposed:
This PR proposes changes to:
.debug corruption.Player::UpdateCorruptionkeeps theCorruptionEffects.db2threshold penalties in step with theplayer's effective corruption. It had three defects, each of which left the penalties out of step.
It re-applied live auras. The loop ended in an unconditional
CastSpell(this, aura, true), soevery corruption rating change re-cast penalties that were already present — restarting their
durations and resetting their proc state. It now casts only what is missing and lets an already
applied aura recalculate its own amounts in place.
It was never re-evaluated on an area change. Each
CorruptionEffectsrow carries aPlayerConditionID, but the sync ran only fromUpdateRating(CR_CORRUPTION | CR_CORRUPTION_RESISTANCE)— a gear or rating-aura change. A player who walked into an area where thecondition stopped matching kept the penalty indefinitely; one who walked out never regained it.
UpdateAreanow schedules a sync.It was never re-evaluated on resurrect.
RemoveAllAurasOnDeathstrips the penalties, and neithera rating change nor an area transition necessarily follows, so reviving where you fell left the
player uncorrupted until they happened to cross an area border. That is why a corpse run appeared to
fix it and
.revivedid not.Why the sync is coalesced rather than run at each mutation site
Syncing at every mutation site is wrong in the other direction. A bulk item rebuild strips every
item's corruption before re-applying it, so a per-mutation sync walks the total down to zero and
tears down each tier's aura on the way back up. The sync is instead coalesced behind a dirty flag and
runs once per
Player::Updatetick.The two item-level rebuilds that did not bracket themselves with
SetCanModifyStatsnow do, andUpdateItemLevelAreaBasedScalingsettles its derived stats afterwards — without that, the max healthit scales the player to was one rebuild stale.
GetEffectiveCorruptionis split out ofUpdateCorruptionso callers that need the value do notrepeat the rating subtraction.
.debug corruptionWhen a penalty fails to apply there are three gates that could have rejected it — the threshold, the
PlayerConditionID, or the row being flagged disabled — and no way to tell which. The alternative isequipping corrupted gear and inferring the answer from what happens.
The command prints the selected player's effective corruption alongside the two ratings it derives
from, then every
CorruptionEffectsrow with its threshold, aura, condition, the verdict on thatplayer, and whether the aura is actually applied. A server whose client data has no
CorruptionEffectsrows at all reports that explicitly, since it is otherwise indistinguishable froma core fault. RBAC-gated behind
RBAC_PERM_COMMAND_DEBUGlike the rest of the table.AI-assisted Pull Requests
re-evaluation paths and drafting. Every line has been reviewed and is defensible by the author.
Issues Addressed:
SOURCE:
The changes have been validated through:
The behaviour being matched is described by the client data itself:
CorruptionEffects.db2carriesMinCorruption, anAuraand aPlayerConditionIDper row, which is what the sync is made tohonour. Retail's own behaviour — penalties appearing and disappearing as corruption crosses a
threshold, and being suppressed in certain areas — is public knowledge from 8.3.
Tests Performed:
Built clean on Windows / VS 2022 x64 RelWithDebInfo, and verified in-game: penalties apply and remove
as corruption crosses each threshold, survive a gear swap, and return on resurrect.
Not covered by that test: the area-condition path. The re-evaluation on
UpdateAreawas reasonedfrom the
PlayerConditionIDcolumn and has not been exercised against an area where a corruptioncondition actually changes state.
How to Test the Changes:
.debug corruptionon yourself with no corrupted gear — every row should report below-threshold.and the penalty aura should be on you.
mainthe penalty's duration restarts;on this branch it does not.
.revivein place. Onmainthe penalties stay gone until you cross an area border; onthis branch they return on the next tick.
Player::Updateand two item-level rebuild paths. Confirm max health andderived stats are correct after an item-level scaling change (a scaled instance, a timewalking
zone).
Known Issues and TODO List:
Player::Updatetick when dirty. That is one extraCorruptionEffectswalk per affected player per tick; it has not been profiled under load.This is one of six PRs splitting a previously oversized branch apart. It is independent of the
other five and can merge on its own; the corruption feature PR stacks on top of it.