[Enh] Add Always-Be-Casting Alert

[aspiringnobody]

Adds an Always-Be-Casting alert that warns the player if they allow the GCD to expire without starting a new cast. Includes some attempts at checking to make sure the target hasn't died or changed, in which case we expect/don't care that there is a gap in the GCD. Also reorders the borders for the GCDBar to keep them always on top.

[aspiringnobody]

I've got one more commit to make to this, waiting on Dalamud to update to test. I noticed that if you're using ShowOnlyGCDRunning, DrawGCDWheel and DrawGCDBar don't get invoked, so the timeouts to reset the bar coloring after a clip or ABC alert don't execute (bar will be red many minutes later on first cast of next combat). This probably applies to ShowOutOfCombat as well.

I can think of a few ways to fix this:

1. Move the reset checks outside of DrawGCDWheel/DrawGCDBar (don't really care for this, either have to make the methods to reset public and call them from Draw() in PluginUI or put them in Update(IFramework) if leaving them private in GCDWheel. Both methods work but I'm not sure of the implications of moving them to something that always is executing, so I opted to avoid this.

2. Move the checks for ShowOnlyGCDRunning and ShowOutOfCombat inside DrawGCDWheel and DrawGCDBar and just return early, after the resets, when we don't want to draw the bar. (Again, unsure of the implications of doing that, chose to avoid).

3. Set the reset time below the GCD timeout ( 4s -> 1.9s ). This fixes the case where ShowOnlyGCDRunning doesn't reset -- but probably doesn't help with ShowOutOfCombat unless the timeout is short enough to complete before inCombat toggles false. That's probably not deterministic so would vary user-to-user based on ping. I chose this since I already implemented variable timeout in the configuration, it seemed the least "invasive." I'll leave it to you to decide if this fixes it "well enough" or if there's a better solution that someone with more experience can implement.

[aspiringnobody]

Dalamud is up, the edit outlined above tests okay. Has the side effect of changing the default behavior when ShowOutOfCombat(ON) and ShowOnlyGCDRunning(OFF) by lowering the default background color reset from 4s to 2s, but this is now adjustable with a GCD Timeout slider in the config for bar/wheel respectively.

[CaiClone]

Hey, thank you so much for the contribution, I just checked and it looks, works and feels great! I have added a couple comments on changes I would suggest if you want to take a look, but nothing blocking, once you feel this is ready to merge I'll merge it.

[aspiringnobody]

Hey, thank you so much for the contribution, I just checked and it looks, works and feels great! I have added a couple comments on changes I would suggest if you want to take a look, but nothing blocking, once you feel this is ready to merge I'll merge it.

Hey, bit of a noob here. Where do I go to see your comments? I'll have a go at addressing them tonight.

Thanks!

[CaiClone]

My bad, I hadn't sent the review yet. I'll be out of town until Monday and won't be able to create a new release until then so feel free to take your time.

[aspiringnobody]

My bad, I hadn't sent the review yet. I'll be out of town until Monday and won't be able to create a new release until then so feel free to take your time.

Okay, sounds good. Those should all be easy to address. There's also a bug I want to track down I noticed in the clipping logic wherein if you use the same ability with multiple charges twice (double weave), like bard's Bloodletter, it doesn't recognize that it caused a clip. I can wait and do that as its own PR after this is merged though (assuming I can find it).

[CaiClone]

Perfect! definitely appreciate putting that on its own PR.

[aspiringnobody]

@CaiClone I've done some pretty major refactoring -- I'm going to pause here until you've had a chance to review. I basically combined all of the GCDBar and GCDWheel settings as much as could reasonably done. Most likely, nobody will use both, and if anyone does it will likely be more convenient to have the colors and timers all be the same anyway. At least that's the thinking.

That allowed me to create a lot of helper methods since the settings were the same.

Have a look and see what you think -- I'll roll back if it's not to your taste. Hope you have a good weekend!

[aspiringnobody]

My bad, I hadn't sent the review yet. I'll be out of town until Monday and won't be able to create a new release until then so feel free to take your time.

I forgot to mention in the commit: breaking config change, you need to manually change GCDTimeout in the config json to an integer between 1-20 if you’re going to try to use a config that loaded my earlier version.

[aspiringnobody]

Edit: I need some feedback on what should happen with the alert when used by a healer. If you're casting a heal on a member of your party, should the alert trigger? Or if you're healing do you not care?

I'm happy with how this is now, I think I've addressed everything I meant to except checking on that bug mentioned before. I've left my debug display in for now, it gives some visual on what's going on behind the scenes with the idleTimer.

Anything you don't like or want redone differently, let me know and I'll change it back/modify it. Once you're happy with it I'll do a final commit to remove the debug stuff.

Hope this isn't too much. If it is let me know and I'll dial it back as you see fit.

[CaiClone]

Hello, I'm back!

First of all, thank you again for the PR and the huge work you are putting into this, really appreciate it! Here's a summary of my understanding of the changes for easier documentation:

• Adding the ABC Alert.
• Reordering the drawn order of the bar.
• Merging common Wheel and Bar options into GCDTracker and GCDDisplay and refactoring the config window to match.
• Add options such as HideAlertsOutOfCombat and HideDisplayIfTP.

The only change I see as blocking is the issue with GCDTimeout float to int. For now I suggest converting to ms if possible, but if that's not possible, we can handle it in other ways such as with the migration code or by creating a new entire variable.

Regarding the question about the healer displaying alerts, I recommend keeping it as is to avoid complicating the configuration further. Based on feedback, most users do not utilize the GCDTracker for casters, so we should avoid adding this option unless someone specifically requests it.

I've left several comments in the review, mostly minor. You've already done a lot, so if you're busy, I'm willing to merge it as is and handle the final edits myself.

[aspiringnobody]

Hello, I'm back!

First of all, thank you again for the PR and the huge work you are putting into this, really appreciate it! Here's a summary of my understanding of the changes for easier documentation:

* Adding the ABC Alert.

* Reordering the drawn order of the bar.

* Merging common Wheel and Bar options into GCDTracker and GCDDisplay and refactoring the config window to match.

* Add options such as HideAlertsOutOfCombat and HideDisplayIfTP.

The only change I see as blocking is the issue with GCDTimeout float to int. For now I suggest converting to ms if possible, but if that's not possible, we can handle it in other ways such as with the migration code or by creating a new entire variable.

Regarding the question about the healer displaying alerts, I recommend keeping it as is to avoid complicating the configuration further. Based on feedback, most users do not utilize the GCDTracker for casters, so we should avoid adding this option unless someone specifically requests it.

I've left several comments in the review, mostly minor. You've already done a lot, so if you're busy, I'm willing to merge it as is and handle the final edits myself.

The float/int should only apply to people who have compiled themselves which is almost certainly just you and I. I started to do a migration but realized anyone who knew enough to compile it would be able to edit the config on their own.

Moot point anyway depending on if we decide it's worth the extra cpu overhead to use real time vs. an approximate time.

[CaiClone]

Perfect, Merging, thank you for the help!