Code checkup (007) & bugs/annoyances I noticed (in 006, haven't ran 007 yet).

1) The function MySort() is no longer being used and can be removed...

2) CheckSov()
You're testing the variable caster but this wasn't ever initialised anywhere in the rest of the code. From what I can tell this should have been the next variable declared (and initialised) a bit higher at the UnitDebuff() call.

3) Interesting to see you removed the 3nd ability in line... It's kinda annoying (and time consuming) to predic future events...

4) The range check currently is annoyingin that it checks melee range on all abilities.
I guess it depends what you really expect here... Do you expect a 'in melee range' indicator or a 'out of range for this ability'. It does the 'in melee range' now, which I find annoying if the current ability has a longer range than that.
Add an option for range checking... "Disabled", "Melee range" (and "Spell range").

5) GetBest()
I could be missing the point or the clever trick here, but... it would appear to me this should either be...
- xprio = 1 and xcd = <ArbitraryLargeValue> to make sure you'll be taking the 'if' branch to set the x-variables to the 1st priority item but then you can better do is the next way...
- xprio = pq[i].priority, xcd = pq[1].xcd, pq[1].xcd = max(0, pq[i]xcd] - 1.5) and start the for loop at 2 instead of 1.

Looking at it now. I'm getting the impression it will sometimes have xprio set to 1 where it should have it set to pq[1].priority

6) since the (2) items in the qd table are intended as copies of an item of the pq table...
wouldn't it be more optimal to have qd just be indexes into the pq table rather than actual copies ?

instead of the self:QD() call:
with appropriate changes in UpdateUi() et al.

7) CheckQueue()
v = pq[i]
you're creating a reference to the pq[i] item, but you still end up using pq[i] (as well as v) in the rest of the for loop.

8) CenterHorizontally()
db.x = (UIParent:GetWidth() / 2 - 110 * db.scale) / db.scale
that line (the 110) got me boggled for a while until I realised it's a hardcoded reference to the width of the frame (half of 220)
Hardcoded values are bad, mkay
-> db.x = ( (UIParent:GetWidth() - (self.frame:GetWidth() * db.scale) / 2 ) / db.scale ? (the self.frame:GetWidth() could be wrong though).

9) InitUI()
Is this needed for anything ? I can't recall seeing that minimalist bar in my last nights raid and clcret test...


which leads me to...
local clcret = LibStub("AceAddon-3.0"):NewAddon("clcret", "AceEvent-3.0", "AceConsole-3.0", "AceTimer-3.0")
Are all 3 of those libs needed ? I can't seem to see any usage of them (?)

1 - true, but I'm still changing a lot of stuff so I postponed code cleanup
2 - is fixed in one of the later alpha builds
4 - first time I implemented it was done per ability but I changed it because without cs displayed you will not have any information regarding your melee range, could add a toggle sure
5 - the code is good, just a lot of unnecesary from old builds, for example priority field is unnecesary since pq is sorted according to priority
6 - pq gets changed so I didn't want to link to it, maybe later after I'm sure important info isn't affected
7 - yeah cleanup is needed
8 - true
9 - is the background shown when you can manualy move the frame
10* - aceevent is needed for talent change, target change, combat status, aceconsole for console commands, ace timer for the delayed init

I tested this briefly the other evening. Seems pretty solid in performing its assigned task.

The frames not disappearing appears to occur if your target despawns or you move out of range without changing target. I was beating on test dummies, then hearthed away, it was still open until I selected a new non-combat target (at which point it vanished). I suspect his "on death" issue was the target despawning before release (bosses/adds as opposed to world mobs, hence your inability to replicate).

Divine Plea and Avenging Wrath on the left never showed on cooldown in clcret once used. I'd appreciate an option to not display these (if I use this in the future I'd just like the current use and next ability, the DP and AW just use screen real estate as I manage them manually elsewhere).

Not necessarily germane and potentially beyond the scope of the addon, but any plans to add a digital countdown to remaining cooldown? I use a mod to add numeric cooldown remaining to my buttons (I find the swirl imprecise and like to know to the tenth of a second what's going on), but of course this mod doesn't appear on clcret (as they're not, and cannot be, actual buttons).

I'll look into it but you should give OmniCC a try. I like the numbers too but I didn't pay much attention to it because OmniCC works on them.

Update (v8):
- replaced DP and AW buttons with 10 configurable ones
- more layout options (position is relative to the black box behind main skill when you unlock the frame)

With 006, dual swapping to holy spec from ret, inside TotC normal (no idea if it matters)


Date: 2009-08-15 02:28:51
ID: 1
Error occured in: Global
Count: 1
Message: ..\AddOns\clcret\core.lua line 498:
attempt to perform arithmetic on field 'cdStart' (a nil value)
Debug:
[C]: ?
clcret\core.lua:498: CheckQueue()
clcret\core.lua:384:
clcret\core.lua:380

Hopefully that talent change bug is fixed in 009.

In addition to the problem that the frame stays visible sometimes when it shouldn't (happend for me when I left the instance while I had an enemy in target) I noticed the following bugs with the "valid target" setting:
* The frame shows if the target is dead (when looting).
* The frame doesn't show if the target changes from friendly to enemy (e.g. in Trial of the Champion).

Tested with last version (10 release) and seemed fine in ToC. Also changed so it disables the frame in vehicles.

Looting seems to be ok now, couldn't test the other cases yet.
But with 010 I saw two new cases (pretty sure they didn't occur with older versions):
* When changing talent spec, the frame shows even if I have no target.
* When getting off a vehicle (e.g. the Argentum horses), the frame shows even if I have no target.

Meh I evidently suck at testing, first one was there, mount related one just exacerbated it.
Should be fixed in new version.

Last beta should allow to track equiped item cooldowns along buffs/skills.

I have to say, I love the changes you made with the latest version of the addon. The new look and SoV DoT tracking is a nice touch. However, I tried to edit one of the aura buttons to track Bloodlust but was unable to get it to work. Does it only track buffs/debuffs which we cast?

Yes it only checks buffs/debuffs cast by the player. Added a an option in latest alpha to disable the player check.

:p

This is an issue I have with mods... I come home from work fairly close to raiding time. I still need to prepare and actually eat dinner. So I don't typically have a lot of time between coming home and geting invited for the raid.

Experience has taught me that changing mods just minutes before the raid starts can be disastrous. So I usually run with mods that I know that work or where I know what issues there are (and how to deal with them) rather than get stuck with all sorts of weird things going on when engaging the boss.

That's why I was still running with 006 even though there was (and I was aware of it) a 008 already.

Last beta (Retribution FCFS helper (clcret) - Addons - Curse) has an option to track your sov dot on multiple targets (up to 5) and display them as bars. Should be disabled by default so check the options.
If you are interested in that kind of feature please let me know if you find any bugs or you have ideas how to improve it.

Played around with a version on the weekend. Again, quite nice and thank you for putting in all the effort for the Ret paladin community.

More suggestions, but they're personal preference so ignore if they don't match your ideas of how the mod should function.

First, when you removed the 2nd queue spot (current, next, 2nd in queue) it made a gap between current and next ability (current, blank space, next). I'd love to have the next ability right beside the currently available. The more you scale the wider the gap becomes.

Second, I love how you can set priority and even throw Divine Plea into things (not to mention your % or absolute mana thresholds for things). Would it be possible to have Divine Plea in priority but only show it if you have a full 1.5 second GCD to invoke? Knowing my mana is low I can choose to DP at any time, delaying an attack, but having mana fairly good and having DP suggested when an attack is .2 seconds from cooldown just pokes me in my perfectionist spot (that's 1.3 seconds of lost DPS!). Probably another checkmark box on the screen where you can adjust mana thresholds.

1) /agree
A slider where you can set the amount of time until a DPS ability comes off cd (0.1 to 1.5) would be handy.
Even better would be if you could set the spare time on various amounts of mana left. If I have 80% mana, I want to DP only if it's a full 1.5sec gap. But if I have 60% mana, I wanna hit DP when it's a 1sec gap. But I'll interrupt myself already here and say it's probably going to far.



2) Part of that is why I suggested being ability to change the rotation on the fly via a slashcommand. When I'm on low mana, I can swap to a rotation that's more mana conserving (no Cons, DP earlier).
The problem is you can't see which of the rotations is active. It's workable, but could be better with actual multiple rotations in the mod and a set of buttons to select between them. Again, I can make do without
I currently have:
- normal (raid)
- normal (solo)
- low mana (raid)


3) Being able to add HoR to the rotation would be nice. The obvious conditional would be that the target is not targetting you already in which case it'll do nothing except waste a GCD. Maybe a safety feature to exclude it when your target is a boss (or elite) would be handy also Caution: Use at your own risk on various types of trash
Would make things a bit easier on the freya trash packs. (if you remember to swap to the rotation without HoR for the 2 lashers+elemental and ancient protector).

Not necessary. They nerfed this "Free Ret DPS" (thankfully). If the target does not change and begin targeting you, no extra damage. I.e. if it doesn't taunt, it doesn't damage. Untauntable targets now take no damage from Hand of Reckoning. HoR is useless unless you really want to taunt something.

I'll take a look for the button movement, thanks. It could be an update since the version I ran (Friday or Saturday).

Try this alpha (WoW AddOns - Retribution FCFS helper (clcret) - r47 - CurseForge.com) for that DP change. The option is last in the behaviour tab, probably need to scroll. Tested it a bit with a value of 1.5 and seemed fine. A value of 0 is the old behavior.

Regarding HoR, when you want to use it, add it as a skill to the ui and use it whenever is not on cooldown, doesn't have a gcd anyway.

Thank you for your hard work burghy, I'm amazed at the swiftness of your updates. I'm loving how this one is turning out. There are however a few things which would make this addon even better in my opinion:

• An option to make it visible only on bosses, SHIT style
• Options to make the SoV tracking use buttons instead of bars, and disabling the names of the afflicted targets
• An option to reduce the opacity of SoV tracking bars/buttons which isn't your current target

Of course this is only my opinion, but I hope that you'll take it into consideration.

Quick test with the R47 version.
My previously installed and tested version was 006.

I really ... (actually let me change that)...

I REALLY dislike the fact that the ability icons are now getting clipped. I spent quite some time making custom icons and a large part of the border/fringe artwork is now gone. The icons are getting clipped well beyond their default beveled border and thus at least another 2 pixel border (of the 64x64 icons) from the actual full icon. They're getting clipped and on top of that there's an enforced border of at least '1' (ok I can alfachannel it).

Please please revert back to drawing the icons in their normal way or allow to do so. I don't know what version exactly broke this, but this (for me) is the kind of change that'll make me throw this mod in the bin.


Too much options to even try and configure this in time for raid (even tho I'm early for a change), so going back to 006 for now... (I removed 'shit' btw )

Still works on Freya trash packs. Many mobs have a constantly resetting agg table (Detonating Lashers) or even attack randomly without actually being taunt-immune; HoR works just fine for them.