Skip to content

Fix default DamageEvent listener priority - #460

Merged
Frotty merged 1 commit into
masterfrom
codex/fix-damage-event-default-priority
Aug 5, 2026
Merged

Fix default DamageEvent listener priority#460
Frotty merged 1 commit into
masterfrom
codex/fix-damage-event-default-priority

Conversation

@Frotty

@Frotty Frotty commented Aug 5, 2026

Copy link
Copy Markdown
Member

What changed

  • Make the no-priority DamageEvent.addListener(listener) overload register at priority 0.
  • Apply the same contract to addUnreducedListener(listener).
  • Add in-game regressions for a default modifier installed after an explicit priority-100 observer.

Root cause

Commit 8cbe5b9 correctly changed same-priority traversal to FIFO, but the no-priority overload still used the current maximum registered priority. If a final observer registered at priority 100 during initialization, a gameplay modifier installed later without an explicit priority silently became priority 100 as well. FIFO then ran the earlier observer first, so it read the raw amount even though the modifier ran afterward.

This is a live-gameplay pipeline issue, not only a test failure: damage observers such as statistics, shields, and chained effects can consume the unmodified amount when dynamically registered modifiers are unintentionally promoted to their priority. The engine still receives the modifier's final amount after traversal, but downstream listener state can be wrong.

The FIFO/tail-pointer and nested cursor work remains intact.

Verification

  • grill typecheck --quiet
  • grill test --quiet
  • Warcraft III in-game regression:
    • 8cbe5b9: reduced 100.0, unreduced 100.0; 21/23 checks
    • 8cbe5b9~1: reduced 40.0, unreduced 40.0
    • fixed branch: reduced 40.0, unreduced 40.0; 23/23 checks

@Frotty
Frotty marked this pull request as ready for review August 5, 2026 10:22
@Frotty
Frotty merged commit 6c84a67 into master Aug 5, 2026
2 checks passed
@Frotty
Frotty deleted the codex/fix-damage-event-default-priority branch August 5, 2026 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant