Prelim Unified Pulse/Flyout Alert Unification

[aspiringnobody]

Okay, so this is the floating triangles and pulses, with the beginnings of unified alert management. I haven't moved the floating triangles or the colored backgrounds into the new class. I haven't really looked at the Floating Triangles yet, but IMO it doesn't really make a lot of sense to move the colored backgrounds into the AlertManager (it's just not a very complex function).

I'm going to wait now for review. There are bound to be bugs and other nasty stuff in here. Probably in over my head if I'm honest. For sure there will be some conditional checks that can be removed (I was trying to be overly cautious not to get double-alerts, so there are some things that are checked two different ways and I'll need to clean that up).

[aspiringnobody]

@CaiClone I'm leaving Wednesday for Vacation. I'll be in the UK until 9/19. I haven't decided if I'm going to take my laptop with me, so you're probably on your own with this from here. If you're able to review today/tonight I might have some free time tomorrow to fix up a couple of things.

[aspiringnobody]

@CaiClone I tried something completely different as a middle ground. Something I could get done quickly that makes the Notifier more palatable until we can do the major refactor.

Basically, the alerts always exist in AlertManager now, and just get turned on by whatever is calling the alert, and then turned off by the EventHandler when it is done with them. This has the added benefit of not requiring us to check/block events from being queued -- if it's already on and enabled, you can't turn it "more" on. It's already on. This lets us eliminate all of the bools to track state and time -- they're just in the alert itself and we pass the whole alert.

Update now just calls the methods that update bar/wheel properties and invoke the FlyOutAlerts -- no queue management required.

I had some problems with the AlertManager -- that's gonna need some cleaning up. As a hack I just created every permutation of Type/Cause/Source on load to work around. For whatever reason, if I tried to create them as they were needed, the logic for the Bar Pulses would eat whichever one was called first (Queuelock or Slidelock). I didn't have time to figure out what I was doing wrong, but if you have time and want to fix it, I had planned on creating the Alerts whenever we peekAlert(). That way we only make the 8 or so we need instead of 80. They do need to exist right from the first time they are peekAlerted though -- We need the alert.Active to be false so that the bar will failover to using default values. Otherwise we get a blank bar until the first time an event happens.

It's not perfect but is certainly better than it was (at least I feel like it's better). Feel free to change whatever you like. For sure one of us needs to figure out how to dynamically create the Alerts to live in the AlertManager -- dealing with every single permutation is a performance hit for sure. I'd also like to rework the ActivateAlert method to not have to copy the entire Queue -- for whatever reason when I tried to update alert.Active in the IEnumerable I got from PeekAlert it didn't stick properly. I had to dequeue and requeue to get it to stay true. I didn't have time to track that down either.

In any case, I think if you fix those two things it is performant enough to wait for a major rewrite to get it to align with what you outlined earlier. If you want to skip working on this code and go straight to that I understand. But I thought I'd at least give it a go making it "better" with the couple of hours I had.

[aspiringnobody]

@CaiClone

Okay, I've taken care of the above post. We are back to dynamically adding alerts as they are needed. Once an alert type is added to the AlertManager it will stay until the plugin is reloaded. The alerts are then activated by GCDHelper as needed and deactivated by GCDEventHandler when finished. There is around a 5-10ms framework update the very first time the bar is actually displayed. Other than that we are averaging 50us when the bar/wheel is being displayed (on my machine, at least). I think that's "good enough" as far as I'm concerned.

I left my debug code in (commented out) so you can observe the behavior of the AlertManager. It just spits out whatever is in the AlertManager every second to the log.

You can check out this other branch as well. I removed as many uses of Linq as I could. I see maybe a minor improvement in performance. Probably (certainly?) not enough to bother with the extra code complexity. But it's there if you want to check for yourself.

https://github.com/aspiringnobody/GCDTracker/tree/Unified_Alerts_No_Linq

[aspiringnobody]

I did a bit to work today on making the alert classes abstract -- It's not going to be that hard. I sort of figured out with the changes there is less "need" to do this now -- I took out most of what didn't apply universally (like LocationX and LocationY that only applied to the FlyOutAlerts. We are down to:

        public EventType Type { get; } = type;  
        public EventCause Reason { get; } = reason;  
        public EventSource Source { get; } = source;  
        public float LastClipDelta { get; set; } = 0f;  
        public bool Active { get; set; } = false;  
        public bool Started { get; set; } = false;  
        public DateTime StartTime { get; set; } = DateTime.MinValue;  

The way I see it we should end up with three levels of abstract classes. The base alert should probably consist of:

        public EventType
        public EventCause
        public EventSource
        public Active

That will then filter down to two sub-classes, call them PulseAlerts and FlyOutAlerts

PulseAlerts inherits the base and adds:

        public DateTime StartTime { get; set; } = DateTime.MinValue;
        private const float TransitionDuration = 300f;
        private const float FirstStageEnd = 100f;
        private const float SecondStageEnd = 200f;

FlyOutAlerts inherits the base and adds (optionally we can split alertText up and move to the child-classes below):

        public bool Started { get; set; } = false;
        alertText = new[] { "CLIP", "0.0", "0.00", "A-B-C" };
        public readonly Easing alertAnimEnabled;
        public readonly Easing alertAnimPos;
            alertAnimEnabled = new OutCubic(new(0, 0, 0, 2, 1000)) {
                Point1 = new(0.25f, 0),
                Point2 = new(1f, 0)
            };
            alertAnimPos = new OutCubic(new(0, 0, 0, 1, 500)) {
                Point1 = new(0, 0),
                Point2 = new(0, -20)
            };

Finally, we have our four actual alert types (five if you want to split bar color and bar size, up to you).

For BarPulse we inherit the above and add our output values:

        public int PulseWidth { get; private set; }
        public int PulseHeight { get; private set; }
        public Vector4 ProgressPulseColor { get; private set; }

For WheelPulse we do the same:

        public float WheelScale { get; private set; }

Then, for FlyOut/ABC we don't actually need anything new, but, since we are going to make Clip its own class we should probably do the same for ABC.

Finally for FlyOut/Clip we do:

        public float LastClipDelta { get; set; } = 0f;

Then it's just a matter of moving the logic elements from GCDEventNotifier into the updates for their respective classes and calling them from either where GCDEventNotifier.Update is invoked now (for ease of implementation) or if we want we can find somewhere more elegant and do it there. GCDEventNotifier should go away entirely.

The logic to manage the alert itself (AlertManager) can either go into the base alert class or be moved as applicable to the update of each child class. Depending on how much code duplication it requires I think I'd prefer each alert child-class to manage its own state, but, whatever is easiest. All we really need is GetOrCreateAlert(), PeekAlert(), and ActivateAlert(). Probably also need whatever initialization logic is necessary to set up the singleton instance as well.

If you want to merge now and handle the abstraction on your own (or do it some other way) that's fine with me. If you're not in a rush and you like what I've described I wouldn't mind doing it when I get home from vacation as a learning exercise. I think the code as is is performant enough to put out for testing (so the guy who asked for it can use it). Of course when we break out the config for the notifications to their own draw instance we will have to do a migration. Maybe better for you to just finish up everything now. Up to you.

Edit: and a child-class for the floating triangles, whenever we get around to moving that here.

[aspiringnobody]

I don't think I'm going to have time to get to the abstract alert refactoring for a few weeks. I haven't even been able to find time to login to the game for that matter! Feel free to do on your own if you like. I don't want to hold you back just because I'm too busy at work to do anything at the moment.

Thanks, Evan