Ovale Spell Priority

98 - [PATCH] Rewrite Eclipse logic, which is totally broken

Currently Ovale attempts to predict Eclipse energy like BalancePowerTracker, but unfortunately the code includes several critical bugs, which makes the functionality totally unusable (and in fact also breaks using the non-predicted Eclipse, which is not available).

Specifically, here is a list of bugs:
1. Ovale removes spells from its list on SPELL_CAST_FAILED.
This is correct when it happens due to cancelling the spell, but not when the spell failed in the first place.
The reason is that the spell was never queued, so what is removed is another copy that instead should be left alone (such as a Wrath during its travel time).

2. The Eclipse energy is reset on each spell, due to the line "self.state.eclipse = self.state.nextEclipse"; in AddSpellToStack (note that nextEclipse is actually never updated and is instead the "real" eclipse).
This results in incorrect computations when a Wrath is travelling and another is being cast

3. Ovale predicts energy gains even for the off-nuke

4. The SPELL_ENERGIZE, SPELL_DAMAGE and UNIT_AURA events for Eclipse seem to potentially come in several different orders and timings, and this might briefly confuse Ovale

Predicting Eclipse energy seems easy at first, but it is actually very hard to get right, so my patch simply makes Ovale use LibBalancePowerTracker for all spells which have been actually been cast, which includes well-tested code to do this, and avoids having to worry ourselves about those issues, and any possible changes in future WoW patches.

For "two action" mode, internal update code is required, and the patch includes a more sophisticated version of what Ovale currently does: since handling events is not required for this, this is much simpler than handling spells that have actually been cast.

A new flag is added to AddSpellToStack to skip the Eclipse logic for spells in spellList, since LibBalancePowerTracker already did it.

The Eclipse energy values are hardcoded, because they need to match the LibBalancePowerTracker data, or there will be very hard to detect bugs happening only in two action mode.

In addition, a new EclipseDirection condition is added, which allows to reliably and easily access the Eclipse direction.

The default Druid script is updated to use the EclipseDirection condition and no longer specify Eclipse energies, which Ovale now knows about automatically.

For distribution, you will also have to add a version of LibBalancePowerTracker to Ovale and update the TOC file.

Name Description Size MD5
ovale-eclipse.patch Eclipse patch 10.7 KiB be310a1e1a4a...
ovale-eclipse.patch Eclipse patch v2 10.7 KiB 443c871fe94c...
User When Change
UltisFoo Sep 24, 2011 at 17:48 UTC

Added attachment ovale-eclipse.patch

UltisFoo Sep 24, 2011 at 16:35 UTC

Added attachment ovale-eclipse.patch

UltisFoo Sep 24, 2011 at 16:35 UTC Create

You must login to post a comment. Don't have an account? Register to get one!

  • 2 comments
  • Avatar of UltisFoo UltisFoo Sep 24, 2011 at 21:46 UTC - 0 likes

    BTW, I just noticed that some additional logic will be needed to support t12 4pc, although it only really matters in "two actions" mode as LBPT will hopefully handle it otherwise.

    I don't have it right now though, so I'll probably not write it right away; the old code also didn't support it, so it's not a regression.

  • Avatar of UltisFoo UltisFoo Sep 24, 2011 at 17:48 UTC - 0 likes

    Uploaded new patch with fixed EclipseDirection code.

  • 2 comments

Facts

Last updated
Sep 24, 2011
Reported
Sep 24, 2011
Status
New - Issue has not had initial review yet.
Type
Patch - Source code patch for review
Priority
Medium - Normal priority.
Votes
0

Reported by

Possible assignees