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... |
- 2 comments
- 2 comments
- Reply
- #2
UltisFoo Sep 24, 2011 at 21:46 UTC - 0 likesBTW, 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.
- Reply
- #1
UltisFoo Sep 24, 2011 at 17:48 UTC - 0 likesUploaded new patch with fixed EclipseDirection code.