Add reactive Anti-Magic Shell and Malkorok (DPS) encounter - #1513
Merged
Conversation
Adds core.ImminentMagicAbilityProvider and wires Death Knight's Anti-Magic Shell to autocast reactively against any encounter target that implements it, falling back to the existing AvgAMSHit heuristic otherwise. Malkorok (AMS) is the first real WCL-data-driven boss AI using this interface. Also updates Frost (Masterfrost, 2H Obliterate) and Unholy P5 presets with current gear/APL and recalculated EP weights.
The masterfrost gear/APL/EP-weight and unholy P5 preset updates in cc7667d changed sim output but the expected .results files were never regenerated, leaving make test failing on master.
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
1337LutZ
reviewed
Jul 29, 2026
…ability data Removes the ImminentMagicAbilityProvider Go interface and the reactive-cast plumbing it added to Anti-Magic Shell's autocast heuristic -- the same behavior is already expressible in the APL via the existing bossSpellIsCasting/bossSpellTimeToReady condition nodes, as Blood/tanks already do for Iron Juggernaut. AMS's autocast reverts to the plain AvgAMSHit > 0 heuristic for all encounters. Also corrects Malkorok's ability modeling against WCL report CzZAMTXx1nW9Pygm fight 27: - Ancient Miasma and Imploding Energy are flat damage (44550 / 585000) with no variance, not the rolled ranges previously used. - Imploding Energy's CD range is Phase-1-only (19.01-30.79s); the previous range was computed across the whole pull and inflated by the ~34s intermission gap. - Essence of Y'Shaarj's CD range (2.96-26.78s) is now computed from spawn-to-spawn intervals rather than raw hit-to-hit gaps, which were polluted by accidental multi-target soaks of a single spawn. - Renamed "Malkorok (AMS)" to "Malkorok (DPS)" since it isn't tank-specific. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds "<Rotation> (Malkorok)" variants for each Frost/Unholy rotation that reactively cast Anti-Magic Shell when Essence of Y'Shaarj is within 5s of coming off cooldown (matching AMS's own shield duration), so the shell is up when the soak lands and guarantees the Runic Power gain -- with no fallback cast otherwise, since holding is always safe here: Essence's worst-case interval (26.78s) is comfortably inside AMS's 45s cooldown. These live in separate preset files rather than the shared default rotations: a manual Cast Spell action referencing a boss-specific spell ID unconditionally removes that spell from the autocast-cooldown pool at parse time (see apl_action.go's removeFromMajorCooldowns), and the condition itself silently collapses to "always true" against any boss that doesn't have that spell (newValueOr/newValueAnd filter out nil sub-values, and a nil action.condition is treated as unconditional) -- baking this into the general-purpose rotations used against every other encounter would have made Frost/Unholy autocast AMS unconditionally everywhere else. Also regenerates the checked-in assets/database/db.json's encounters section from the current core.PresetEncounters (Iron Juggernaut and Malkorok were registered in Go but never present in the committed preset DB, which the UI reads instead of talking to a live server -- Iron Juggernaut's dedicated Blood presets have the same class of gap already, just less noticeable since Blood has no shared default rotation for it to break). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- makefile: build the local dev binary as wowsimmop.exe on Windows (an OS-conditional BIN_EXT variable, empty everywhere else). air launches it through a path that needs a recognized extension; a bare "wowsimmop" makes Windows fall back to its "select an app to open this file" prompt instead of just running it. - vite.config.mts: set appType 'mpa'. This project is a genuine multi-page app (one index.html per spec), but without this Vite's dev server defaults to SPA mode and serves the root index.html as a fallback for every navigation request -- including ones that match a real nested index.html -- so every spec page silently rendered the home page instead of its own. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1337LutZ
reviewed
Jul 30, 2026
1337LutZ
reviewed
Jul 30, 2026
Address PR review feedback: drop resolved spell-school TODOs and cut redundant/verbose explanation from the Ancient Miasma, Imploding Energy, and Essence of Y'Shaarj comments. Also gitignore the local cmd/tmp_bis and cmd/tmp_verify tools (kept on disk for reuse, not meant for history).
1337LutZ
reviewed
Jul 30, 2026
Removed entries for temporary BiS/verification tools from .gitignore.
hillerstorm
reviewed
Jul 30, 2026
hillerstorm
left a comment
There was a problem hiding this comment.
Took a deep pass over this, inline comments below. One meta note: the PR description is outdated — it references core.ImminentMagicAbilityProvider, which doesn't exist anywhere in the diff (the reactive mechanism is APL-driven via bossSpellTimeToReady), and it says both Frost P5 presets were migrated to the Malkorok encounter, but only Masterfrost was. Worth updating the description to match what the PR actually does.
hillerstorm
reviewed
Jul 30, 2026
- Embed the -malkorok APL variant (with the reactive AMS cast) in all three P5 build presets instead of the plain rotation; migrate 2h Obliterate's build to the real Malkorok encounter and drop avgAmsHit now that a real boss AI deals the damage. - Zero avgAmsHit/avgAmsSuccessRate in Frost/Unholy DefaultOptions so they don't double-model AMS intake or starve the reactive AMS line when combined with a Malkorok preset. - Drop the dead ShouldActivate closure in anti_magic_shell.go. - Correct Malkorok's NPC ID (846 -> 71454, the real creature ID) in the Go preset, db.json, db.bin, and all three build JSONs; regenerate db.json's encounters block from core.PresetEncounters instead of hand-editing it, and patch db.bin's Encounters field directly since the client-data pipeline isn't available here. - Cast every ready Malkorok ability per tick instead of an if/else priority chain, and recalibrate Ancient Miasma's CD roll for the boss-GCD tick's rounding bias, so realized cast rates match the observed WCL data instead of being suppressed by contention/rounding. - Add Target.RandomizeGCDTiming and call it from MalkorokAI.Reset so cast timing gets re-randomized every iteration instead of anchoring to the same phase each time. - Warn when a Malkorok-tuned rotation preset is loaded against a non-Malkorok encounter, where the reactive AMS condition silently degrades to an unconditional cast-on-cooldown. - Revert vite.config.mts's appType 'mpa': its SPA-fallback diagnosis was wrong (real nested index.html files were never being overridden), and it broke fresh-clone devmode and /mop/raid/. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Embed the -malkorok APL variant (with the reactive AMS cast) in all three P5 build presets instead of the plain rotation; migrate 2h Obliterate's build to the real Malkorok encounter and drop avgAmsHit now that a real boss AI deals the damage. - Zero avgAmsHit/avgAmsSuccessRate in Frost/Unholy DefaultOptions so they don't double-model AMS intake or starve the reactive AMS line when combined with a Malkorok preset. - Drop the dead ShouldActivate closure in anti_magic_shell.go. - Correct Malkorok's NPC ID (846 -> 71454, the real creature ID) in the Go preset, db.json, db.bin, and all three build JSONs; regenerate db.json's encounters block from core.PresetEncounters instead of hand-editing it, and patch db.bin's Encounters field directly since the client-data pipeline isn't available here. - Cast every ready Malkorok ability per tick instead of an if/else priority chain, and recalibrate Ancient Miasma's CD roll for the boss-GCD tick's rounding bias, so realized cast rates match the observed WCL data instead of being suppressed by contention/rounding. - Add Target.RandomizeGCDTiming and call it from MalkorokAI.Reset so cast timing gets re-randomized every iteration instead of anchoring to the same phase each time. - Warn when a Malkorok-tuned rotation preset is loaded against a non-Malkorok encounter, where the reactive AMS condition silently degrades to an unconditional cast-on-cooldown. - Revert vite.config.mts's appType 'mpa': its SPA-fallback diagnosis was wrong (real nested index.html files were never being overridden), and it broke fresh-clone devmode and /mop/raid/.
devmode had no prerequisite generating the per-spec index.html files, so a fresh clone or a clean rebuild left Vite's default SPA fallback silently serving the homepage for every spec route (and /mop/raid/) until a full production `make` had run at least once.
Mirrors the player-side spell_is_known check but target-scoped, so a rotation can gate on whether the boss actually has a given spell registered instead of a missing condition value silently defaulting to "always ready". The reactive-AMS Frost/Unholy presets were the motivating case: bossSpellTimeToReady(Essence of Y'Shaarj) alone resolves to nil against any non-Malkorok encounter, and a nil APL condition reads as unconditionally true, turning the reactive Anti-Magic Shell cast into an unconditional cast-on-cooldown rather than skipping the action. With AND(bossSpellIsKnown(...), bossSpellTimeToReady(...) <= 5s) gating the cast, the Malkorok-tuned rotations degrade gracefully everywhere else, so they no longer need a separate "(Malkorok)" variant or the load-time toast warning about encounter mismatch. The plain non-Malkorok presets are removed and the Malkorok ones renamed to take their place; DK frost/unholy golden test results are updated to reflect AMS now being active by default.
…rok-ams # Conflicts: # ui/death_knight/frost/presets.ts # ui/death_knight/unholy/presets.ts
The boss_spell_known locale key was added to en/fr translation.json in 5991fb3 but the schema was never updated, so Test Locales CI rejected both files as having an additional property. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1337LutZ
reviewed
Aug 2, 2026
1337LutZ
reviewed
Aug 2, 2026
BIN_EXT was a hand-rolled ifeq on $(OS). go env GOEXE reports the same .exe on Windows and empty everywhere else, and stays correct under WSL or if GOOS is ever overridden. The makefile already shells out to go env for GOROOT, so this adds no new parse-time dependency. The .gitignore entry for wowsimmop.exe was redundant -- /wowsimmop* on line 17 already covers it.
Resolves conflicts against upstream's DBC spell proc mask fix (wowsims#1517), which regenerated db.bin and shifted DPS across every spec's golden results. All four conflicts were generated artifacts, not logic: - assets/database/db.bin: took upstream's regenerated binary, then re-applied the Encounters field from core.PresetEncounters the same way gen_db does. The DBC pipeline can't run locally without assets/db_inputs/dbc, so this restores Malkorok as the 11th encounter without discarding upstream's proc mask corrections. - DK frost/unholy .results: regenerated against the merged tree. Blood was untouched by this PR and regenerates byte-identical to upstream, confirming the proc mask fix is in effect and the db.bin re-marshal left item data intact. Frost/unholy match our pre-merge numbers except on the proc and enchant entries upstream moved.
…cache CD.Reduce() shortens Anti-Magic Shell's cooldown outside the normal TryActivate() path, but the majorCooldownManager's cached minReady bailout in getFirstReadyMCD was never refreshed to match. This let autocastOtherCooldowns keep short-circuiting past AMS until its pre-reduction ready time, silently eating the discount the glyph is supposed to grant and reducing AMS's effective cast count over a fight.
hillerstorm
requested changes
Aug 5, 2026
Wrapping the Anti-Magic Shell cast line in an AllOf(BossSpellKnown, TimeToReady<=5s) condition removed AMS from autocast consideration on every non-Malkorok fight, since an explicit cast line disqualifies a spell from autocast regardless of whether its condition passes. Merge in a Not(BossSpellKnown) branch so the old always-cast behavior is preserved when the Malkorok debuff isn't present. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolved conflicts: - assets/database/db.bin: couldn't be textually 3-way merged (binary). Rebuilt it from the cleanly-merged assets/database/db.json instead of re-running the full DBC extraction pipeline (which needs a real wowsims.db from DB2ToSqlite, not available here) -- Encounters was restored from the Go encounter definitions (core.PresetEncounters) and GlyphIds was read back out of the merged JSON. - sim/death_knight/frost/TestFrostMasterfrost.results, sim/death_knight/frost/TestFrostTwoHand.results, sim/death_knight/unholy/TestUnholy.results: regenerated by re-running the affected test suites, since both branches shifted DK numbers. Also regenerated the protobuf Go bindings (sim/core/proto/*.pb.go, not tracked in git) from the merged proto/*.proto to unblock the build. Full `go test --tags=with_db ./sim/...` passes after the merge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
hillerstorm
previously approved these changes
Aug 5, 2026
hillerstorm
self-requested a review
August 5, 2026 13:40
hillerstorm
approved these changes
Aug 6, 2026
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.
Adds Malkorok (DPS) encounter that simulates P1 of the Malkorok SoO encounter. Can wire up AMS in the APL to specific boss abilities to simulate real soaking mechanics.
Adds BossSpellIsKnown to gate these conditions in the APL to their specific encounters, without breaking compatibility for other encounters.
Also updates Frost (Masterfrost, 2H Obliterate) and Unholy P5 presets with current gear/APL and recalculated EP weights.