seems to not be working for the new 4sec crusader strike.

I'm not a Wow addon specialist, far from it, I've never written one before. So my knowledge with the Wow APi is limited. I'm also not fluent in Lua, I can read/understand (usually) a piece of lua presented to me, but i'm not going to be spewing lines of lua code without constantly looking things up... I am however a professional software developper and have been for many years, so I do at least know how to write something slightly more complex than a "hello world" application :p

I went over the SHIT.lua source to try and figure out an issue I was having and ended up going way beyond that. Here's some stuff I noticed that may (or may not) help make this an even better mod than it already is.

1) the PLAYER_ALIVE event is being registered to the eventFrame, but all this event ever does is unregister itself. So unless this is a placeholder for a planned future feature. it can be removed entirely.

2) The PLAYER_REGEN_DISABLED and the COMBAT_LOG_EVENT_UNFILTERED handler do exactly the same. So could both be delegated to a single helper function.
the COMBAT_LOG_EVENT_UNFILTERED event seems a weird choice to me here (but then I don't know the wow api that well) but it's my understanding this is fired each and every time a line gets added to the combatlog, isn't this a bit "extreme" ? I'd imagine updates don't need to happen anywhere near that frequent (?).

2b) the PLAYER_REGEN_DISABLED function (and thus the combat event one) can be written smaller and more optimal (less overhead). Note that unless an error (or a purposed attempt at sabotage by manually changing the savedvariable file) is made, SHITdb.showon cannot have any other value than 1, 2 or 3. This assumption means we can rewrite the entire function to:

3) The same logic as 2b can be applied to the PLAYER_TARGET_CHANGED() event handler.
4) SHIT: DecideSpells() is now always being called in a situation where prior testing has been performed to determing that indeed something needs to be displayed.
Yet, in the start of DecideSpells() function this same set of tests (player has Crusader strike, testing the ShowOn conditions etc) is being done again to have an 'early out' type test.
The tests seem to be not needed.

The one exception is in the showon=1 case where some extra stuff happens. If this is needed the start of DecideSpells() can be changed to just:
My guess is it isn't needed. DecideSpells() is now always being called when UnitName("target") has not returned nil. I'd guess/expect that when you have a targetname you will always also have a targetGUID (?).

5) SHIT: DecideSpells()
Is using a hardcoded 1.5sec (GCD) towards calculating the spell to use after the current one.
This is correct except for Consecration, Exorcism, Divine Plea and Sacred Shield as those are on hasted GCD.
Is there a way to calculate actual GCD ? maybe you could use the CastTime for FOL. since that's a 1.5sec spell and has the same haste benefit as the GCD on spells.
I'm not sure if our GCD's on the melee abilities get hasted from Heroism, but that would also mean the "next spell" might be off with the actual spell that will be suggested as new current one after casting the current one.

The next spell also doesn't account for the change in mana for casting the current ability. So next spell could be suggested to be Consecration, but after casting your current ability not have enough mana left to meet the Consecration mana requirement. Or other way around, if you just casted judgement, the mana returned could mean you suddenly have enough mana for Consecration where you previously had not.

Next spell also assumes the consecration glyph (hardcoding the cooldown in nextspell to 10 seconds), and also assumes 2/2 Imp Judgement setting judgement at 8sec cd, while we probably "all" will have this, there was the debate about whether or not you'd ever judge faster than every 9 seconds, so only 1/2 imp judgements would be needed.

A minor issue, but... don't trust too much on the "next" ability to actually going to be the next one being suggested

6) SHIT:NextSpell()
The long chain of if-tests to test each possible ability in each possible priority seems like a prime candidate for a better alternative.

for a start the pr1, pr2, pr3, pr4... can be changed to an array...
-> pr[1], pr[2], pr[3], pr[4]...
you can then already cut the chain of if's down to a for-loop testing each priority (by index) for the various ability numbers.
That'll cut down in lua code size, but I'm not sure how that change would affect performance.

I've been using NeedToKnow to track the dot on targets. Shows the duration of the dot left and also the number of stacks. Handy for many classes/mechanics within the game.

Any dot timer would do a similar job, though.

Interesting... Especially seeing I'm not really into mod programming (yet, have been planning to for a while). I can see definate parallels between this and 'shit' code wise in setting things up, but clcret is much cleaner in design. Lua is amazingly flexible it would seem...

I'm currently running a slightly modified version of 'shit' and was already planning to either start a new project or continue on it if Thaeryn wasn't going to continue supporting it. I didn't want to just 'steal' it. I kinda wanted to talk to Thaeryn about that but haven't seen him on here for... near a month now (?).


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).

For various reasons predicting the 2nd spell in line after the current one isn't going to be 100% accurate, but you can get it to a decent degree of reliability. Not accounting for the GCD and remaining cooldown on the current spell however will make the 2nd and 3rd spell so totally random it's essentially unusable.

Now I'm faced with the trilemma to work with Thaeryn (if he is still going to support it), making my own or working with you