Closed vitharr137 closed 3 months ago
alright, this is a very old issue and it is interesting in some points I'm sorry I never took the time to acknowledge/delve into it, but I have just tested & updated it to use it in https://github.com/MKhayle/XIVComboExpanded/commit/9373f8fa32fd5cef8d710197e93690efa4479afa
Monk has been changed significantly since you published this and I am less aiming at optimization than other forks, but this is still very nice to have overall if I need it at some point
Thanks!
Glad it helped! I am not great at github protocol and hope this isn't some wrong way to notify, but here's something a bit more up to date than the one from last year, it includes a check for Haste Adjusted Cast Time (CastTime) and resource Cost, as well as another check in the form of TotalChargeRecoveryRemaining which has been useful for redmage and blackmage
@kaedys idk if you're still interested in this over what you did there, but just in case
One other place to look at is the IsOnCooldown() and IsOffCooldown() for a sanity check just to see if those are still doing what is intended on line 400+ in CustomCombo.cs. I think what was discussed in the paladin section fixes it, but just for certainty
So, looks like IsOnCooldown
and IsOffCooldown
both just call and return IsCooldown
, Off
inverting the result first. So both of those are also fixed by the change I made.
I'm comparing the above snippet to the existing code, though, and I'll see how easily it can be integrated without (hopefully) breaking anything else >.<
Ok @vitharr137, so, I'm super confused by some of the implementations here, both in the original and the updated, and I think it's causing issues for some of the combos.
RemainingCharges
trying to implicitly calculate the charges, rather than just using the cur
return from Service.ComboCache.GetMaxCharges(this.ActionID);
?TotalBaseCooldown
multiply by cur
? Wouldn't that return zero for the TotalBaseCooldown if all charges on the ability are currently used? Why isn't it multiplying against max
instead?TotalBaseCooldown
subsequently divide by cur
, which as far as I can tell is 100% identical to just using this.BaseCooldown
.var total = this.cooldownTotal / max * cur;
in CooldownTotal
meant to be this.cooldownTotal / (max * cur)
or (this.cooldownTotal / max) * cur
, which is equivalent to this.cooldownTotal * (cur / max)
? As written, the latter is what is used, due to C# operator order precedence.
cooldownTotal
the cooldown per charge or the cooldown per charge multiplied by the number of charges? ie. for, say Drill (2 charges, 20s recharge), would this be 20s or 40s?ChargeRecoveryTime
is supposed to do. From the code, it appears to return the current remaining cooldown if at least 1 charge is remaining, and the cooldown remaining plus the time to get a charge if it currently has 0 charges. What use is this meant to serve?
TotalChargeRecoveryRemaining
?Edit: Ok, digging into it, it looks like the first return of GetMaxCharges
is not the current number of charges, but the maximum at the current level (very silly naming, imo). So that obviates at least some of the above.
Edit2: Ok, further observations/questions after re-reading it with the above.
IsCooldown
returns this.CooldownElapsed < this.TotalBaseCooldown
(if CooldownElapsed
is non-0), but CooldownElapsed
returns 0 if this.cooldownElapsed >= this.BaseCooldown
, so shouldn't it be impossible for the IsCooldown
conditional to return false? Seems like duplicated logic.
IsCooldown
returns false if the ability has charges and is not currently at 0 charges, but is below maximum charges and thus currently recharging. I think this might actually be what's breaking the Machinist logic.cooldownTotal
and TotalBaseCooldown
. Is it just that the latter is adjusted by spell speed, if relevant (which, afaik, only applies to a handful of abilities at the most, notably a summoner's Demi summons)?I do want to preface this with it was coding by trial and error, and I am not a well educated coder in general by any stretch so there is plenty of room for mistakes that I've made along the way, but so far these have been my observations. Most of this is cobbled together from other forks, or directly dragged out of the structs themselves and put into code here in the best way I could (which is not well):
FFXIV's reverse engineering chains are split between some of Dalamud's intepretations, and the FFXIVClientStructs themselves, and several references to Excel sheets made from the game engine itself. The lines from 14 to 23 in CooldownData.cs are the ones I was having the most problems with in general, especially cooldownTotal, which does seem to return 0 in many more instances than would be logical, and since a majority of already written combos were using those returns, most of CooldownTotal was rather hackedly put together to match logic that was already being used by other forks, such as the very weird bit with return 0 if 0
(this.cooldownTotal / max) * cur
is meant to be the result, as in, for and example like Bloodletter for BRD, which has at high levels 3 charges with 15 second recharge times each, a 45second total cooldown, but at lower levels, only 2 charges, for a max of 30, but each function should return a "CooldownTotal" for the charge of 15 seconds each
cooldownTotal itself seems to return a timeline-style full value, taking the example of Bloodletter from above, it would be 45 seconds returned for 3 charges of Bloodletter, 15 seconds each. I haven't verified if newer implementations have better calculated today, but at the time, I was struggling with the calculation returning 45 for example, when the level I was being synced to was supposed to have 30 only (because I was lacking the trait for 3 total)
It looks like you may be here right now, so I'll post this part of the explaining and continue editing further :)
ChargeRecoveryTime
was made to see if there was enough time to use a free charge available before other cooldowns such as 2 minutes would occur, or if a maintenance buff/debuff (in this case I was tracking huton) would last long enough for the charge to be used. The desired logic would be: 1) check if there's a charge left after spending one, meaning I could use that and still have a free one for maintenance., or 2) if there would be no charges left at the end, then tack on a full charge's worth of recharge onto the current one. It's meant as a look-ahead calculation to see if I needed to spend one now to refresh huton (when it existed) so that I could have both charges ready for the next burst phase
TotalChargeRecoveryRemaining
is an extension of that 2 charge focus into more than that, specifically for Bloodletter in this case, and may be a more formal representation of the two. This is a check to see when everything will finish cooling down, and is haste adjusted. To put it in simpler terms, here is a breakdown of the separation I made form the original cooldownTotal and my new one:
Using BaseCooldown, find the real cooldown of a single charge of any given ActionID (15 for Bloodletter)
Using BaseCooldown and GetMaxCharges, find the full amount of Cooldown Seconds for any ActionID (45 for Bloodletter) as TotalBaseCooldown
Reintroduce cooldownElapsed as TotalCooldownElapsed to preserve the old usage in many forks, and a new variable I could use in later calculations
Using TotalBaseCooldown (45) and TotalCooldownElapsed (cooldownElapsed returns 40 in the case of the first of 3 Bloodletter charges used 10 second ago (45 - 15 + 10), 0 if all charges are ready, and 1 if there are no remaining charges after using the last one 1 second ago (45 - 45 + 1), if that all makes sense), find the TotalCooldownRemaining, which is the haste adjusted value of 5 in the case of cooldownElapsed returning 40
Introduce a new CooldownRemaining format, using modulo to find the per charge cooldown from TotalCooldownRemaining, TotalBaseCooldown, and the current level's max charges (cur used as the GetMaxCharges variable, returning 3 for a Bard at the appropriate level for 3 Bloodletter charges). Since TotalCooldownRemaining returns a value from 0 to 44.9... modulo will return cyclic values from 0 to 15, properly tracking the per charge cooldown. As an example, if the last charge of Bloodletter was used 3 seconds ago, this would be the equation tracked: (45 - 3) % (45 / 3) = 12
seconds until a charge of Bloodletter is ready
TotalChargeCooldownRemaining
at this point may be redundant, but I was too lost in the sauce to tell at this point, and I wanted something to specifically track the difference between a single charge returning off cooldown, and all charges returning off cooldown. As in, if there are no charges of Bloodletter remaining, this value should be somewhere between 45 and >30 seconds
We've now arrived at TotalChargeRecoveryRemaining
, which simply asks if you use a charge, how long until that charge also recovers (mostly for aligning to burst windows) and this is all haste adjusted rather than flat values previously provided, meaning it can respond dynamically for haste applied by buffs in those rare occasions it does
IsCooldown
is dealing with the difference between my CooldownElapsed
and the previously coded cooldownElapsed
. Mine returns 0 if there are any charges ready, but the original only returns 0 only if the full stack is ready, and neither properly track the time since the last actionID, only different varieties of comparison against the full cooldown value. My IsCooldown indicates that the action is unavailable to press at all, and returns true in the window of cooldownElapsed = 0 - 14.99~ in the case of Bloodletter (cooldownElapsed not being >= 15, so !=0, and cooldownElapsed < 45 (15 * 3))
Your last comment is the main reason for the difference, yes. Haste adjusted values were extremely vital to Monk prior to Dawntrail in order to determine which combo to fall into and how to prepare for burst windows, and allow for smoother calculation overall that fall more in line with game state rather than flat cooldowns on a number of other niche cases like haste applying somewhere, or cast times changing, or simply things like Gnashing Fang for GNB that are cooldown based moves using skill speed like standard weaponskills
Please let me know if there are more questions, I tried to take time to be exact for my definitions but there has been some time since I looked over the code like this so I am likely not explaining well enough
In case it doesn't ping properly, the more complete description for your questions has been edited in
Ok, I'm going to need to look at this in more depth tomorrow when my brain is against functioning properly, and see if I can figured out how to adapt CalcBestAction
for the changes.
Conversation on this will continue in #305
Hello,
I've been working on optimizing some of the ways the cooldown data works in the addon, and while I'm not confident enough in it yet to open a real pull, I do want to let you know about the advancements we've discovered in some of the other forks of this project to see if you'd like to use any of them! The main reason currently that I avoid a pull is it seems to mess with other aspects of code that rely on the durations of cooldowns being set to 0 manually in CooldownData.cs
These are used in the CooldownData.cs section, and require:
using FFXIVClientStructs.FFXIV.Client.Game
The major one is it's been discovered how to get the haste/speed adjusted cooldown of a given actionID:
That line alone has allowed functions such as tracking uptime for combo based rotations like Twin Snakes or Demolish, by using code like such:
The rotate double can now be used to check if the Twin Snakes or Demolish duration remaining is less than it, providing an easy switch for combos to adjust to
Another set of useful data has been the easy calculation of time until a certain actionID is able to be used after the next usage:
These can assist for timing/reserving charge based actionIDs for certain buff windows, allowing checks to see if the full charge recovery time would occur prior to a given cooldown.
I've uploaded the txt version of the modified CooldownData.cs which contains some commented lines for the functionality, hope you can find some useful additions!
CooldownData.txt