Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature Suggestion] Long Cast Support #15

Closed
aspiringnobody opened this issue Jul 25, 2024 · 16 comments
Closed

[Feature Suggestion] Long Cast Support #15

aspiringnobody opened this issue Jul 25, 2024 · 16 comments

Comments

@aspiringnobody
Copy link
Contributor

Now that we have tooling to detect hard-casts, it should be pretty trivial to add full cast-bar functionality (show spell name, time to finish, queue-lock, and slide-cast indicators).

If this seems like a feature you’d like to add comment below and I can start working on it. Otherwise I’m probably going to implement some quality of life improvements to hide the GCDTracker when actions like item use and hard-casts are in progress.

But the thought occurred to me that there’s no need to hide the GCDTracker if I hide the castbar instead!

@CaiClone
Copy link
Owner

Hmm I think having the name of the spell would be really nice to make the addon more usable on casters, but replacing the castbar entirely does sound like a hard job, my main concern is:

  • Ease of maintenance. I'm already in the process of deprecating the Combo Tracker due to requiring manual work almost every patch. So I'm worried that getting things such as the correct version of the of the spell might require hardcoding exceptions if that's the case I would prefer to avoid adding it.

Other than that it depends on exactly what we are talking about, how would you handle challenges such as:

  • Showing both GCD time and Time to finish spell, when cast time != GCD.
  • Will this display the name of queued spells or just the one you are currently casting? Future spells seems really hard to do, as they might change depending on buffs/phases and we cannot know in advance.
  • Right now we are only showing weaponskills and ogcds, but having it replace the castbar entirely would require also registering actions such as interacting/tp and displaying on non combat jobs, would it not?

If it's just a simple solution that does not require a lot of maintenance I'm good with it! obviously quality of life improvements also sound good!

@aspiringnobody
Copy link
Contributor Author

Hmm I think having the name of the spell would be really nice to make the addon more usable on casters, but replacing the castbar entirely does sound like a hard job, my main concern is:

* Ease of maintenance. I'm already in the process of deprecating the Combo Tracker due to requiring manual work almost every patch. So I'm worried that getting things such as the correct version of the of the spell might require hardcoding exceptions if that's the case I would prefer to avoid adding it.

Other than that it depends on exactly what we are talking about, how would you handle challenges such as:

* Showing both GCD time and Time to finish spell, when cast time != GCD.

* Will this display the name of queued spells or just the one you are currently casting? Future spells seems really hard to do, as they might change depending on buffs/phases and we cannot know in advance.

* Right now we are only showing weaponskills and ogcds, but having it replace the castbar entirely would require also registering actions such as interacting/tp and displaying on non combat jobs, would it not?

If it's just a simple solution that does not require a lot of maintenance I'm good with it! obviously quality of life improvements also sound good!

I'm thinking all instant cast abilities work exactly as they do now. No text display at all. For spells that are interruptable, we would switch to "castbar mode" where we swap the queuelock for a slidecast bar, and color the end of the bar differently for slidecast. We then draw the name of the ability and the time elapsed/remaining.

As far as I can tell, the ability names are accessible using Lumina, and DelvUI's code for this could probably be used as a template (they're GPL so no worries there). They're always super up-to-date so as long as our implementation resembles theirs the hard work is just keeping relatively current with what they do:

public class LastUsedCast {  
    public uint CastId { get; }  
    public ActionType ActionType { get; }  
    public bool Interruptible { get; }  
    public string ActionText { get; }  
  
    public LastUsedCast(uint castId, ActionType actionType, bool interruptible) {  
        CastId = castId;  
        ActionType = actionType;  
        Interruptible = interruptible;  
        ActionText = GetActionText(castId, actionType);  
    }  
  
    private string GetActionText(uint actionId, ActionType actionType) {  
        var actionSheet = Plugin.Data.GetExcelSheet<Lumina.Excel.GeneratedSheets.Action>();  
        var action = actionSheet?.GetRow(actionId);  
        return action?.Name.ToString() ?? "Unknown";  
    }  
}  

Give me a bit to work on it and we will see if it turns into a complicated mess or not.

@aspiringnobody
Copy link
Contributor Author

Getting the spell name from the data files was pretty easy:

SpellText

I'm realizing now that this might be difficult to make work properly without putting some constraints on the dimensions of the bar. I doubt anyone but me is really using the bar at this point since it's so new, but it is still worth mentioning. We will have to dynamically calculate the text position from the bar and under a certain size it won't make sense to do so.

@aspiringnobody
Copy link
Contributor Author

aspiringnobody commented Jul 26, 2024

Okay, I've got a basic castbar working now. DelvUI's Castbar on top, ours on the bottom:

longer_cast

ToDo:

  • improve detection logic for when to use GCDBar and when to use Castbar
  • figure out how to show visually when the next spell can be queued
  • implement text size and location customization
  • implement some minimum checks on barsize to hide text when it won't fit
  • implement non-ability "casts" -- interacting, teleporting, etc.
  • Decide what to do in the case CastTime < GCDTime. Part of me is inclined to run the bar for the full GCDTime, but that is complicated because we need to show the slidecast window over the top (easy), and then continue to draw that even though HelperMethods.IsCasting went false (less-easy). We would need to cache that state, but only when the cast was successful (hard). Other ideas include having a portion of the bar at the bottom show the castbar progress as a line and the rest be the gcd. I need to play with it and see what feels best in game.

Edit: When GCDTime >= CastTime, I think the play is to scale the castbar so that it exactly lines up with where the GCDTracker would be on its way to the TotalGCD. That way, when IsCasting goes false, the GCDTracker exactly syncs with the location of the castbar, and you seamlessly transition to the GCDTracker. I'll try this first and see how it plays, but conceptually this makes the most sense to me.

@aspiringnobody
Copy link
Contributor Author

aspiringnobody commented Jul 27, 2024

Edit: I'm working out of main now

https://github.com/aspiringnobody/GCDTracker

It's "rough"

Don't say I didn't warn you ;-)

-Edit: for whatever reason, the very first cast doesn't work. I'll sort it out later. Wanted to get you this proof-of-concept before I spent anymore time on it.- I think I fixed this now.

@CaiClone
Copy link
Owner

Visually really appealing! Nice!
Love both the design, and the part that tells you the next spell you have queued.
I think the solution of scaling the CastBar to not fill all the GCDBar looks good, and is clear as to what time you have to weave casts or until next cast.
The only thing I find a little confusing is that we end up having a bit too many vertical lines on the bar, and sometimes it's hard to distinguish which one is the queue lock/slidecast/Cast end/OGCD so it would be good if we find a way to make them more distinct. I could try changing the queue lock to be represented on the frame itself similar to what's on the GCDWheel.

And In general I feel like the options you need to set up on the Castbar are different than the ones you would use just for the GCDBar, such as ShowOutOfCombat, so maybe at the end we can look at it as a whole thing on it's own, and enabling it just changes a bunch of other objects to allow the castbar to work well, but it depends on which options we end up.

From my point of view the result looks great, performance is okay, and is not a big advantage over other players. If you haven't found any technical reason this can't be done I see no problem adding it to GCDTracker!

@aspiringnobody
Copy link
Contributor Author

I'm going to do some polish. Midori told me in the discord that AtkStringArrayData[20] contains the game's version of the spell text (where it's being displayed on the in-game castbar). Apparently it can be accessed by using Dalamud's memorymanager to read it as a null terminated string. This allows dispensing with all of the reading of the sheets and special one off cases forever.

It's way over my head though. He wrote the following pseudo code as an example:
AtkStage.Instance()->SomethingStringArrayData()[20][0]

If any of that means anything to you I could use help making that work. It's much better and more future proof than what DelvUI currently does.

I agree about the lines; they make a bit more sense when the slidecast area is red instead of transparent black, but there are a lot of colors going on already, so transparent black seemed like a safe default that would work with everything. I've got a couple of ideas I'll implement and we can see what feels the best.

@CaiClone
Copy link
Owner

Oh the AtkStage thing is pretty neat. You can try it with:

AtkStage = FFXIVClientStructs.FFXIV.Component.GUI.AtkStage.Instance();
byte** stringData = AtkStage->GetStringArrayData()[20][0].StringArray;

int length = 0;
byte* currentByte = stringData[0];
while (currentByte[length] != 0) length++;

byte[] data = new byte[length];
for (int i = 0; i < length; i++) data[i] = currentByte[i];

string result = Encoding.UTF8.GetString(data);
GCDTracker.Log.Warning($"Cast: {result}");

It does get the name of the exact thing on the castbar and without overhead. Although I don't think we can use that to know the next ability so it is a trade off.

@aspiringnobody
Copy link
Contributor Author

Oh the AtkStage thing is pretty neat. You can try it with:

AtkStage = FFXIVClientStructs.FFXIV.Component.GUI.AtkStage.Instance();
byte** stringData = AtkStage->GetStringArrayData()[20][0].StringArray;

int length = 0;
byte* currentByte = stringData[0];
while (currentByte[length] != 0) length++;

byte[] data = new byte[length];
for (int i = 0; i < length; i++) data[i] = currentByte[i];

string result = Encoding.UTF8.GetString(data);
GCDTracker.Log.Warning($"Cast: {result}");

It does get the name of the exact thing on the castbar and without overhead. Although I don't think we can use that to know the next ability so it is a trade off.

The next spell is easy enough to grab from sheets since it will always be on the ability sheet. And it’s easy enough to turn off that functionality if it ever breaks so it won’t block an update. Single line fix.

@aspiringnobody
Copy link
Contributor Author

aspiringnobody commented Jul 29, 2024

Alright, new build available. Probably better if you don't look under the hood, it's a crime-scene in there. This is strictly as a proof of concept for how to deal with the double-line problem. Still need to add an option that will be the default to prevent the queuelock from "sliding" when in GCDBar mode.

When it's done it will have the normal behavior for the Bar unless opted-in to the slide effect.

In essence, when both bars are present on a short cast, they get caught by the advancing spell and collapse into each other, so we leave none behind. I also added some little triangles to the bottom to differentiate them (point up is slidecast, point down is queuelock). This was harder than it seems.

I did a lot of casting floats to int to allow "pixel perfect" alignment -- or more accurately, to allow the numbers to interact in a predictable way. It would probably be better to rework everything to be in hard pixels -- but that would break everyone's config. So I decided to cast the border numbers to int so that there would be even "breakpoints" where the border would increment by 1 and allow for everything to align properly. This has the effect that the slider moves, but nothing happens until you cross a whole number -- BUT -- doesn't break everyone's config. It will work as is, with the caveat that we might be off by one pixel depending on where they were exactly on the slider. But it's entirely deterministic now -- you can get exactly one pixel borders, or two, etc.

This can be changed back, but would pretty much make this sliding/collapsing thing a no-go. To work we really need to know where stuff is to the pixel or you end up in edge-case hell.

Haven't made the change to the spell names yet - getting this done took forever tracking down off-by-ones and the like.

edit: you might have some gaps between the lines and the triangles depending on your bar width slider setting. I'll fix this later but wanted to make sure you liked it as it was before i spent anymore time on it. If you have misalignment or gaps it should suffice for now to click over a notch or two on the slider until you find something that lines up.

Untitled2
Untitled

@CaiClone
Copy link
Owner

Triangles are a good idea!
in my case the configuration worked well and I didn't have any gaps. And if we can calculate which values have gaps we can just shift them a bit in migration.

Ok, so from my understanding:
image

  • Lower triangle: SlideCastStart
  • Upper triangle: QueueLock
  • End of shaded are: CastEnd. (only if Slidecast Covers End of bar disabled)
    Personally I find disabling the "Slidecast covers end of bar" a lot more intuitive as not seeing where the cast actually ends feels weird to me, but I can see how that's redundant information that someone might not care about.
    I like the triangle solution a lot, to the point I would consider (once everything is decided) adding the upper triangle by default to the queue lock on the bar even on melees and long casts. To make it more consistent and easier to figure out which is which.

@aspiringnobody
Copy link
Contributor Author

Alright, I got tired of trying to fit a square peg into a round hole. The DrawBar method uses different math to calculate its top and bottom (namely, just the size.X&Y and the barRatios) than the drawRect and drawTriangle methods (now we are adding in the center position and trying to split the difference, which is hell when we are working in floats that have to appear as whole pixels).

So I got to thinking, if you can't beat them, join them: if everything is equally inaccurate, then everything will align. So I've started work on depreciating DrawBar and just doing everything with the filled rectangles. This requires us to calculate a bunch of vertices, but fortunately we had most of what we needed anyway. I've finished the castbar and the basic progressbar of the GCDTracker -- but I still need to code the drawing of the oGCD animation locks and oGCD start indicator.

But this build should hopefully fix our alignment issues (minus some +1 crap I might need to remove later from when I was trying to make it work the other way). If you could, test this on your PC and let me know how it scales and performs.

I also added a helpful pixel display in the bar config to show you the actual (effective) dimensions of the bar to make dialing in a specific size easier.

@aspiringnobody
Copy link
Contributor Author

aspiringnobody commented Jul 30, 2024

New Build:
I "finished" with the move from DrawBar to DrawRect. This hasn't been properly tested (e.g. I haven't zoomed in to make sure everything is pixel perfect yet). Also adds text centering, text size options, triangle size slider, and implements the mono font the rest of the plugin uses. I actually didn't like how the font looked on the bar, so I implemented a checkbox to disable that feature (I must be seeing the default ImGui font or DelvUI sets it to something and doesn't pop the old font back when it's done. Either way I left the checkbox for you to see what you think.

TODO:

  • Migrate Text to ATKStage
  • Add Queuelock Option -> Enable Slide (to mimic current behavior by default)
  • Add Options for Text Outline, Maybe Add Outline to Alerts
  • Probably add a Dropdown to Pick Font
  • Add Support for Odd Bar Sizes Done, not properly tested. Probably some off by one errors to find
  • Add Option to Disable One or Both Triangles done.
  • Force Slidecast & Queuelock Vertical Bars to be Drawn Even if Border Size == 0 done.
  • More ???
  • Bug Test

Edit: There are some bugs to squash, but I added another mode for the "slidecast doesn't extend to the end" of the bar setting. There should now be two lines, each with half the triangle, as "bookends" to the slidecast portion of the bar. Whether you want that mode or the slidecast goes to the end of the bar mode depends on class. RDM for example only has spells that are exactly 500ms less than the GCD, so they always end right at the 0.8f GCD marker. So having the bar end early doesn't make any difference, the crossover is the same. But for other jobs, like White Mage or the example you linked above, it makes more sense because they have spells that end way before the 0.8f GCD mark. I think it's important to figure out settings that will allow for both, since some jobs will really want one or the other.

I like this new (split) setting, but I need to work out all of the interactions between the queuelock and the slidecast since they use two different mechanisms.

I think I'm going to redesign this whole scheme, wherein the queuelock and the slidecast don't ever move -- they get drawn in the places they belong at the start of the cast and stay there. The progress bar will then "pick up" the triangles as it passes by, and when progressbar >= queuelock or slidecast those will turn off entirely. This will make it a lot easier to manage the interactions between all the settings, and allow us to finalize a list of settings that works for everyone without being mega-complex. I probably won't have time to do any more until Thursday, but this will give you some time to decide if you like the whole split thing or not.

@aspiringnobody
Copy link
Contributor Author

aspiringnobody commented Aug 1, 2024

Updated again, the split triangles are working better now, and less general jank. I wasn't able to redesign as outlined above because when the progress bar passes over the slidecast/queuelock it would cover over the leading triangle (if present). But I reworked the system for triggering the various bits so that it's hopefully a bit more maintainable. It's a bit spaghetti but it is what it is.

I've got some unused code and comments to clean up yet, but getting toward the home stretch. Next thing on the todo is clean up some config things. there are some options there that aren't implemented in the new code yet. (like you can't turn off the QueueLock at present) I'm going to start working out of main now to get this ready for a PR.

https://github.com/aspiringnobody/GCDTracker

@aspiringnobody
Copy link
Contributor Author

aspiringnobody commented Aug 2, 2024

Oh, one other thing, I need your opinion on something code related. There was way way too much going on in the DrawCastBar method, and I needed to break it up into smaller chunks to make it even remotely understandable/maintainable. But this presented a problem: I would have to declare like 75 variables at the class level or have a web of methods being called with 20-30 parameters passed to them.

So I settled on making some classes for the data, figuring that would be the cleanest solution (and might help if we decide to move some of this code out of GCDWheel). I’m not sure if that’s a totally appropriate thing to do though, can you have a look at what I’ve done and give me some feedback please?

PS I thought about calculating the vertices in a loop since there are only five real “types” of vertices (three for the triangles and two for the rectangles) but decided it would be easier to maintain if it was all copy/pasted. I think the vertex calculations are unlikely to change and we are also unlikely to add additional vertices in the future — meaning IMO there is little benefit to saving a few lines of code at the expense of making it less readable other than the small performance improvement we might get by creating an array of vertices that need calculated that frame and only processing what is actually needed. But I don’t think there are enough total to justify that level of abstraction. Let me know what you think.

@CaiClone
Copy link
Owner

Closing this as it ended up being merged in #17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants