Ret FCFS Rotation Helper - Page 13

Neraya · 08/13/09, 4:31 AM

Originally Posted by vorda

Could you maybe write a few lines where exactly it's different from SHIT?

- not configurable (yet)
- doesn't account for low mana settings (not doing cons below certain mana)
- no setting for suggesting recast of SS (don't cast SS faster than)
- no setting for suggesting recast of DP (don't cast if you're above a certain amount of mana)
- 2nd and 3rd in line spell (rather than just 2nd), but (see above) currently seriously flawed
- Doesn't have a funny/witty acronym (although shit really is over the top, it gets filtered out on a lot of moderated forums).

burghy · 08/13/09, 5:00 AM

Originally Posted by Neraya

Code wise: I had already come up with the 'sorting' by time/priority as a much better way to decide the next spell myself, but as it is coded now in clcrret it's flawed for the 2nd (and 3rd) spell in line since it will prioritise spells already off CD over higher priority spells that will come off cd within the next GCD (and the GCD after that for 3rd).

Not exactly sure what you mean. FCFS "says" what spell comes first off cooldown is used, even if a better priority spell will come of cd before next gcd ends. Also, GetSpellCooldown returns the GCD value if cooldown is lower than GCD. That means the sorting is done based on priority for spells with cooldown lower than gcd and cooldown duration in rest.

Could you maybe write a few lines where exactly it's different from SHIT? Is it coded more efficiently?

Can't be many differences outside the code except for the changes needed in 3.2, like showing exorcism only if art of war buff is up (which I can't see handled in the latest version of SHIT I have).

Originally Posted by Neraya

- not configurable (yet)

Pretty sure I made the ej post after I updated the build with the config options.

cremor · 08/13/09, 5:14 AM

Originally Posted by Neraya

Actually clcret DOES use OnUpdate (see clcretOnUpdate), which from what i can tell is the proper way to actually do something like this. It's probably how the original program 'shit' was copied from did things as well, but this got changed by moving most of the work to the COMBAT_LOG_EVENT_UNFILTERED (CLEU) event which is strange as I'd imaging that being called A LOT more frequent than OnUpdate. It's also the cause of an annoying bug in 'shit' in that if you target something and are out of range and out of combat, then run to it, it won't update to signalling in range. That's because the COMBAT_LOG_EVENT_UNFILTERED event handler doesn't get registered until combat starts.

It's currently hardcoded to not update more than 10 times a second, which imo is too restrictive. It should be lower by default, and should be configurable to toggle it off. (or configurable throttling)

Oh yea, missed that OnUpdate.
But since it uses correct throttling, it should be ok (I just realized that such an Addon isn't possible without an OnUpdate

).
A configurable throttling value/timeout would be really nice, but I wouldn't offer the option to disable throttling. That would result in the same awful performance like SHIT currently has.

Neraya · 08/13/09, 8:00 AM

Originally Posted by burghy

Not exactly sure what you mean. FCFS "says" what spell comes first off cooldown is used, even if a better priority spell will come of cd before next gcd ends. Also, GetSpellCooldown returns the GCD value if cooldown is lower than GCD. That means the sorting is done based on priority for spells with cooldown lower than gcd and cooldown duration in rest.

Start out.
Everything is on CD (ignore HoW in this, it's not available until mob is <20%).
0.0s: Hit CS,
1.5s: Hit Judgement.
1.5s: The situation for Current, 2nd and 3rd ability should now be: DS, CS, Cons but this isn't what the mod proposes, it suggests DS, Cons, Exo instead.
The sorting solution sorts the current abilities based on time left on CD (2.5sec for CS, 8Sec for judgement), priority after.
3.0s: Suggestion for ability to hit is still DS, with Cons & Exo next. (should be DS, CS, Cons). CS has 1sec CD left, Judgement has 6.5sec left, everything else has 0s CD.
3.0s: Hit DS. Next suggestion is Cons, Exo, CS (where it should be CS, Cons, Exo)
4.0s: CS CD is over, and is now being suggested as 1st. with Cons and Exo after. it's right now, until you hit CS, then it'll be wrong again. When the cooldown of an ability is over, things change suddenly, rather than the cooldown being accounted for.

-> Entering CheckQueue() at 3.0 seconds in the fight...
q[1].cd (cs) = 1.0
q[2].cd (judge) = 6.5
q[?].cd (all others) = 0

You sort first by CD, then by priority
Ability 3, 4 and 5 are suggested next since 1 and 2 are on cd.
This is correct for 1st ability (3=DS) but for the 2nd ability, you need to account for the fact that this will happen 1.5 seconds later in time, Ability 3 will happen 3 seconds later in time (not accounting any hasting, lag etc). That time difference is important. It changes the order since during that time spells will come off cd

You not only need to account for GCD, but also for any CD left on whatever ability is going to be used next.

Can't be many differences outside the code except for the changes needed in 3.2, like showing exorcism only if art of war buff is up (which I can't see handled in the latest version of SHIT I have).

I tweaked this in my own 'shit' build, as well as handling the "range check" checkbox.

Pretty sure I made the ej post after I updated the build with the config options.

I downloaded the 001 version to see the changes between 001 and 003 and based the quote off the 001 version. My bad.

Neraya · 08/13/09, 8:17 AM

Originally Posted by cremor

Oh yea, missed that OnUpdate.
But since it uses correct throttling, it should be ok (I just realized that such an Addon isn't possible without an OnUpdate

).
A configurable throttling value/timeout would be really nice, but I wouldn't offer the option to disable throttling. That would result in the same awful performance like SHIT currently has.

Ok, nub mode kicking in (I'm not a wow mod programmer remember)... The issue now is that the OnUpdate() is enabled but doesn't do anything in certain situations (you're holy/prot)

Another issue is that it uses the CLEU handler while in combat to update the display. that's several hundred times per second.

Unless my understand wrong.... OnUpdate gets called once every time the display is painted, so it gets called as many times as your screen refresh rate (ctrl+R)... so somewhere between 10 to maybe 100 times per second. If your PC sucks it won't update often, if you have an imba PC, you're not gonna notice. Or your FPS will drop from 250 to 248. OMG 2 frames lost when it doesn't even matter if it happens faster than the actual refresh rate of your monitor. A throttle, yes, maybe. somewhere in the area where it becomes inperceptible for the human eye (which has traditionally been decided to be around 30fps), any further throttling will mean the cooldown animation will appear jerky and means you'll see a change later than ideal. Throttling at 10fps is imo a bad idea, it means you potentially miss seeing the right 'next' spell up to 0.1 sec later than needed. If you have actually 10fps, if will on average be 0.15sec late.

cremor · 08/13/09, 10:21 AM

Yes, OnUpdate is called for each frame. The higher your FPS, the more calls.

In the current version of SHIT, the OnUpdate is completely unneeded. Even if you are in Ret spec, the OnUpdate handler just calls some API functions and does some checks - and then returns without doing anything. It will never call DecideSpells() because SHITdb.showon is always either 1, 2 or 3.
Short: Just unneeded code that can be removed.

I removed it in my local copy and reduced idle CPU time to 2% of unmodified code (from 1000ms over 5 min to just 20ms). Of course in combat the reduction isn't that big as a relative value. But it stays the same absolute reduction (if the frame rate stays the same).

Now to clcret:
Since the OnUpdate is really used for updating the display there, of course a lower value results in faster updates.
And of course your point that the FPS impact even without throttling is only minimal is correct. I agree with you that a minimal value of 30 FPS (0.03 sec) would be the best solution.

Neraya · 08/13/09, 11:24 AM

Originally Posted by cremor

I removed it in my local copy and reduced idle CPU time to 2% of unmodified code (from 1000ms over 5 min to just 20ms). Of course in combat the reduction isn't that big as a relative value. But it stays the same absolute reduction (if the frame rate stays the same).

So... SHIT calles OnUpdate without throttling. And this takes 1000ms over a period of 300.000ms. I agree that it isn't doing anything in SHIT so removing it is a good idea. But c'mon... 1sec of CPU time for a 5min duration... while basically idling in dalaran. So you're going to throttle it so it takes 1 second every 15minutes ? Personally, I'd prefer a smoother and faster updating mod instead...

The bigger issue is the hooking to CLEU when it should be doing all it's doing in OnUpdate instead.

burghy · 08/13/09, 11:24 AM

Update:
- option to select the number of updates per second
- option to check mana for consecration and divine plea
- improved a bit the prediction according to what neraya suggested (it's far from perfect and I'll look into it some more)

A throttle, yes, maybe. somewhere in the area where it becomes inperceptible for the human eye (which has traditionally been decided to be around 30fps), any further throttling will mean the cooldown animation will appear jerky and means you'll see a change later than ideal.

Most cooldowns in addons are displayed using Blizzard's own functions so the animation is not affected if the addon updates slower.

Mox · 08/13/09, 12:09 PM

Originally Posted by Neraya

So... SHIT calles OnUpdate without throttling. And this takes 1000ms over a period of 300.000ms. I agree that it isn't doing anything in SHIT so removing it is a good idea.

Exactly which bit of the code is it that you remove? Want to try this change for myself to see if theres any FPS difference.

Neraya · 08/13/09, 12:16 PM

Originally Posted by burghy

Most cooldowns in addons are displayed using Blizzard's own functions so the animation is not affected if the addon updates slower.

You're saying the button.cooldown:SetCooldown() effectively establishes an animation that will perform the "rotating cd" bit all on it's own even if you didn't process any more OnUpdate()'s ? I didn't find much info on it, I kinda assumed it merely drew a particular percetage complete and required you to update if explicitely...

My bigger concern with throttling however is that it'll delay updating the "hit this button now for max dps", by potentially a full throttling delay (on top of framerate delay, slowframerate is bad for dps). If you are throttling, then at the minimum I'd expect it to update the "hit now" button as soon as it changes. That's probably too complex to do, so disabling throttling entirely would be my preference.

'shit' now does it in CLEU so lots lots more than OnUpdate would do. It's not having a noticable effect on framerate on my PC whether I disable or enable it. However, adding a throttling of 0.1 to shit (as it is in version 003 of clcret) and testing on a dummy following it blindly does have an effect on dps (not by much, but still...). If the throttling delay can be set to 'zero' then I'll be happy :p

I'll give the new version a shake after dinner.

As for desired features...
1) Changing the priority list via a slashcommand. This would allow you to swap from a particular rotation to another depending on circumstances (low mana (no cons, priority to judge), trash (more prio on aoe), pvp, pve, ...) with a macro. Being able to save multiple rotation settings in the mod itself with a clickable button (glowing to show which one is active) would be even better but slashcommands seem simple and should do the job well enough, the only disadvantage would be not being able to see which one's currently active.
2) Displaying SOV stacks, and time left on the DoT.
3) Displaying AOW proc
4) Displaying time left on SS (even when not cast on self), and moving it up in the priority to make sure it'll be in the rotation to keep it up back to back (with the removal of the self hurt from blood, I'm typically using SS on the offtank), macro'd to focus target... I just sometimes forget to press it at 30sec :p yes, even if it means delaying dps. Sometimes keeping the tanks alive matters more.

5) Showing the CD on AW after using bubble

Neraya · 08/13/09, 12:19 PM

Originally Posted by Mox

Exactly which bit of the code is it that you remove? Want to try this change for myself to see if theres any FPS difference.

Locate the SetScript call for "OnUpdate" and comment those 3 lines out...

-- displayFrame:SetScript("OnUpdate", function(this, elapsed)
--     SHIT:OnUpdate(elapsed)
-- end)

Neraya · 08/13/09, 1:24 PM

moar feature requests for clcret :p

1) no throttle option
2) it's impossible to set the minimum mana to an explicit value. the smallest step is also kinda high (partly because the maximum is set to 10K which is a lot higher than even my mana in a raid setting with all buffs).
3) Same for Divine Plea
4) a "don't suggest SS unless it's cooldown remaining is below Xsec" option.

burghy · 08/13/09, 2:29 PM

Regarding mana options, you can type the exact number in the boxes under the bars.

Neraya · 08/13/09, 3:00 PM

Bugs:
If you select the "on valid target" show mode, the frame stays visible when you die (even if you no longer have a target)

Neraya · 08/13/09, 3:49 PM

Originally Posted by burghy

Regarding mana options, you can type the exact number in the boxes under the bars.

never really occured to me those slide bars have editable numbers O.O (/me goes hide in the nub corner)