-
Notifications
You must be signed in to change notification settings - Fork 729
[Mage] Clearcasting BLP + Arcane TWW2 Tier Effect #10303
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
base: thewarwithin
Are you sure you want to change the base?
[Mage] Clearcasting BLP + Arcane TWW2 Tier Effect #10303
Conversation
engine/class_modules/sc_mage.cpp
Outdated
@@ -1026,7 +1029,7 @@ struct mage_t final : public player_t | |||
void trigger_arcane_charge( int stacks = 1 ); | |||
bool trigger_brain_freeze( double chance, proc_t* source, timespan_t delay = 0.15_s ); | |||
bool trigger_crowd_control( const action_state_t* s, spell_mechanic type, timespan_t adjust = 0_ms ); | |||
bool trigger_clearcasting( double chance, timespan_t delay = 0.15_s ); | |||
bool trigger_clearcasting( double chance, timespan_t delay = 0.15_s, bool predictable = true, bool guaranteed_jackpot = false ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it's better to handle jackpots/tier effects inside of Clearcasting or ToTM; ToTM grants a guaranteed jackpot, either I deal with it within totm's execute, or here. My reasoning was that since jackpots are directly linked with regular Clearcasting procs, I might as well pair them up, but I'm not sure if this is the right call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the need to set up the RPPM for this as I mentioned in another comment, I don't think it makes sense to have the guaranteed triggers in here. I think it's better to just trigger it directly from TotM so that the trigger in here can always roll the RPPM. This way it's also a bit more consistent with how the other specs are handled. While this could cause a double trigger of the jackpot buffs in some cases, as far as I can tell that wouldn't matter, so we don't even need to test if it can happen in game.
Is there any case other than the Time Anomaly trigger where a true 100% chance proc would not be predictable here? It might be better to make it never_predictable = false
by default and then use if ( chance >= 1.0 && !never_predictable )
as the condition.
engine/class_modules/sc_mage.cpp
Outdated
@@ -2259,15 +2262,32 @@ struct mage_spell_t : public spell_t | |||
// This will prevent for example Arcane Missiles consuming its own Clearcasting proc. | |||
consume_cost_reductions(); | |||
|
|||
if ( p()->spec.clearcasting->ok() && triggers.clearcasting ) | |||
if ( triggers.clearcasting && p()->spec.clearcasting->ok() ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why I changed this. I suppose it just looked neater to me at the time? Sorry.
engine/class_modules/sc_mage.cpp
Outdated
trigger_clearcasting( 1.0, 0_ms ); | ||
if ( has_4pc ) | ||
buffs.aether_attunement->trigger(); | ||
if ( guaranteed || rng().roll( 0.13 ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused as to where it originally (before this) returned Jackpot's original proc rate -- I'm certain it's a 13% chance, but I can't find a mention of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jackpot effects are normally triggered through the driver aura that gets set up in mage_t::init_special_effects
. For Arcane, this aura is 2 RPPM, which when saturated after 3.5 seconds of no trigger attempts comes out to a proc chance of 2 RPPM * 3.5 seconds / 60 seconds = 11.67%
. On top of this, RPPM has its own BLP system, which is probably where the 13% average you're seeing comes from (under normal conditions, the BLP averages out to around 13.1% extra procs, which would increase the 11.67% to about 13.2% here).
To roll the RPPM after triggering Clearcasting, we're probably going to need to (1) not create that special effect for Arcane and (2) manually set up the RPPM object for Arcane (see mage_t::init_rng
for a couple of other RPPMs we use manually). The RPPM object would then be manually rolled before calling non-guaranteed triggers of the jackpot.
engine/class_modules/sc_mage.cpp
Outdated
} | ||
|
||
// Likely a bug: Arcane Orb procs from Orb Barrage uniquely decrements the BLP, effectively nullifying the incoming increment from Barrage. | ||
if ( name_str == "orb_barrage_arcane_orb" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if I should include the use of 'bugs' here. I have no idea if this is intentional by blizz -- if it is, I think it's kinda silly, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have data available somewhere regarding the tier set changes in this PR? The Clearcasting BLP changes make sense to me given that the average rate of procs from simulations matches what is seen in game, but I'd want to verify the tier set changes before this gets merged because those are a pretty significant behavioral change.
engine/class_modules/sc_mage.cpp
Outdated
trigger_clearcasting( 1.0, 0_ms ); | ||
if ( has_4pc ) | ||
buffs.aether_attunement->trigger(); | ||
if ( guaranteed || rng().roll( 0.13 ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jackpot effects are normally triggered through the driver aura that gets set up in mage_t::init_special_effects
. For Arcane, this aura is 2 RPPM, which when saturated after 3.5 seconds of no trigger attempts comes out to a proc chance of 2 RPPM * 3.5 seconds / 60 seconds = 11.67%
. On top of this, RPPM has its own BLP system, which is probably where the 13% average you're seeing comes from (under normal conditions, the BLP averages out to around 13.1% extra procs, which would increase the 11.67% to about 13.2% here).
To roll the RPPM after triggering Clearcasting, we're probably going to need to (1) not create that special effect for Arcane and (2) manually set up the RPPM object for Arcane (see mage_t::init_rng
for a couple of other RPPMs we use manually). The RPPM object would then be manually rolled before calling non-guaranteed triggers of the jackpot.
engine/class_modules/sc_mage.cpp
Outdated
} | ||
|
||
// Likely a bug: Arcane Orb procs from Orb Barrage uniquely decrements the BLP, effectively nullifying the incoming increment from Barrage. | ||
if ( name_str == "orb_barrage_arcane_orb" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should check whether the arcane_orb_t's type == ao_type::ORB_BARRAGE
instead of doing a string comparison.
engine/class_modules/sc_mage.cpp
Outdated
@@ -1026,7 +1029,7 @@ struct mage_t final : public player_t | |||
void trigger_arcane_charge( int stacks = 1 ); | |||
bool trigger_brain_freeze( double chance, proc_t* source, timespan_t delay = 0.15_s ); | |||
bool trigger_crowd_control( const action_state_t* s, spell_mechanic type, timespan_t adjust = 0_ms ); | |||
bool trigger_clearcasting( double chance, timespan_t delay = 0.15_s ); | |||
bool trigger_clearcasting( double chance, timespan_t delay = 0.15_s, bool predictable = true, bool guaranteed_jackpot = false ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the need to set up the RPPM for this as I mentioned in another comment, I don't think it makes sense to have the guaranteed triggers in here. I think it's better to just trigger it directly from TotM so that the trigger in here can always roll the RPPM. This way it's also a bit more consistent with how the other specs are handled. While this could cause a double trigger of the jackpot buffs in some cases, as far as I can tell that wouldn't matter, so we don't even need to test if it can happen in game.
Is there any case other than the Time Anomaly trigger where a true 100% chance proc would not be predictable here? It might be better to make it never_predictable = false
by default and then use if ( chance >= 1.0 && !never_predictable )
as the condition.
engine/class_modules/sc_mage.cpp
Outdated
p()->state.clearcasting_blp_count += 1; | ||
if ( !background && rng().roll( proc_chance ) || ( p()->state.clearcasting_blp_count >= cc_blp_threshold ) ) | ||
{ | ||
p()->trigger_clearcasting( 1.0, 100_ms, false ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 100_ms and not the default 0.15_s? I don't know which is more accurate, but I think it should be consistent in both places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100ms delay for CC gained via the BLP/random proc rate is what I consistently saw in logs. Occasionally, I'd see a bit of a delay greater than 100ms, but I believe it's mostly due to my autoclicker being unsynchronized with spell casts and its following GCD, so it'd desync every now and then. For stuff like TA/Splinterstorm/Soul/ToTM jackpot, it was 0ms, though.
Yeah, I'll go ahead and change it.
edit: I'll re-verify the delay closer, it's confusing me. Just a bit of time.
engine/class_modules/sc_mage.cpp
Outdated
p()->state.clearcasting_blp_count -= 1; | ||
else | ||
p()->state.clearcasting_blp_count += 1; | ||
if ( !background && rng().roll( proc_chance ) || ( p()->state.clearcasting_blp_count >= cc_blp_threshold ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think something like this would be cleaner here, especially when combined with the change I mentioned above to replace that predictable
argument, because unless I'm missing something, we could predict a proc is coming due to BLP.
if ( p()->state.clearcasting_blp_count >= cc_blp_threshold )
proc_chance = 1.0;
if ( p()->trigger_clearcasting( proc_chance ) )
p()->state.clearcasting_blp_count = 0;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we allow predicting the BLP (I'd assume for both this and intuition), might it be a good idea to make the bad luck protection visible as an actual buff, something like with AA's counter? I'd also assume it'd be nice to have it visible + tangible within the input.
I've a bunch of log dumps, but they're all kinda unorganized since I kept a TLDR within my local sim's input (because it saves a buncha time as I don't have to constantly tab in + out to compare with logs -- see attached just so I don't sound crazy. I've also a bunch of them saved within my wcl profile, if you'd like to look at literally everything, but its unbelievably unorganized, so); most prominently: all for tier: I don't think there's a BLP w/ tier (or at least, it's higher than 33 unsuccessful applications), and CC total applications x 0.13 is 1:1 with how it appears on logs -- I'm completely unfamiliar with the RPPM (thanks for the explanation by the way), and the way I landed on everything was pretty straight-forward. I've a bunch more logs relating to CC's blp if you're interested. Admittedly, I wasn't as thorough with tier effect as I was with CC -- I don't mind leaving tier be for now if this isn't sufficient. Aside from spells, I had checked to see if events (i.e splinterstorm/TA/soul) + background increments have a chance with tier, and it appeared to proc from any source. edit: https://www.warcraftlogs.com/reports/VGLX8TZtdj3DWHJm?fight=last&type=auras&ability=1216178 |
Thanks, I'll take a look at the data a bit later. I have a description of RPPM here, but it's all implemented into simc so we can just hook it up to the 2 RPPM in spell data without doing much work. The BLP system involved is not a sudden increase, but a gradual increase over time after going long enough without getting a proc. I'd be surprised if that's not what's in use here given the spell data and given that the 13% number you found is very close to the expected proc chance from 2 RPPM. |
Just writing an update regarding this: This log has TA untalented + IT talented + 4pc + alternates between Barrage and ToTM. Plugging in sims and ditching the roll chance of 13% in exchange for 2RPPM seems like it's accurate w/ the log above. Though, I don't know how to definitively check it uses 2rppm instead of a simple roll; having it one or the other doesn't really make a difference in terms of total applications -- which I suppose is expected -- but in turn it makes it difficult to really know what's actually present in-game. I suppose the mention of it using a 2rppm within the spell data dump is proof enough, right? Anyways, attached file is what the sim spat out while using an rppm of 2 + having spells alternate between totm and barr (just like in the log above). Should note: it's not matching spell casts 1:1 (midway through I changed the frequency of the auto-clicker from 4s to 1s, so its kinda annoying to replicate within input), so take it with a grain of salt. Reasoning for the ToTM + Barrage run was because I wanted to see if guaranteed jackpots reset the RPPM -- the attached file has 'guaranteeds' not resetting it, which appears to be more accurate. I'll run more checks soon -- particularly about the delay -- as I'm confused as to why it'd occasionally grant CC past 100ms. dunno. Although, give me a couple of days as I'll be out for today and tomorrow. Alright, byes. |
Thanks for the update.
While there are ways to process a lot of logged data to differentiate between 13% and 2 RPPM, I think the 2 RPPM being in spell data is sufficient here.
Don't worry too much about the exact delay; combat log timestamps are not as consistent as needed to easily measure things like that (they often get batched together and don't represent the true server time when something happened). What matters here is that any delay means that the Clearcasting proc won't immediately be available when the next action is selected by the APL and executed. Choose one of 100 ms vs 150 ms is just a matter of consistency in the code and should not affect the results in any meaningful way. |
Hey. Sorry for the long wait, I've been too long gone from home. Since we're making the BLP predictable, should I convert state clearcasting_blp_count + intuition_blp_count into visible buffs? I think it'd be pretty neat, and I'm sure it'll have some influence within the APL; I believe a WA could keep track of it in-game, but I mean I dunno if it's necessarily a good idea to make people (literally) dependant on a WA -- I suppose everybody kinda is already? I don't know. If I may swap it into a buff, do I hijack clearcasting/intuition's ID -> give it a name -> then manually set stack + duration, or is there a way to circumvent the spell data argument whenever making a buff? Are there any examples of this being done within the past or something? |
Rather than buffs, tracking them through Mage-specific expressions like clearcasting_blp and intuition_blp is probably best. It skips the overhead of a buff and the only real downside is not being able to see the current stacks in sample sequence. They could even be something like clearcasting_blp_remains and intuition_blp_remains, where they could count down from the maximum instead of up from 0. See |
Hey again. I think I'm (hopefully) done-done. Thanks for patiently guiding me through a lot of new module stuff; I probably wouldn't have ever known about RPPM or expressions if you hadn't pointed me there -- (seriously) thank you so very much. Let me know if there's any more issues. I still feel the need to check-up on two things, though: the ms delay between gaining Intuition and when it can amplify the damage of Barrage (currently 30ms -- just_gained_intuition), and secondly -- out of paranoia -- whether or not background effects can randomly proc either Clearcasting or Intuition (I checked this some time ago, but I'm VERY worried about having messed something up -- saw some video on discord; I have no idea if its just an inaccurate or misleading weakaura, or if I'm just dumb); PTR is currently down so I'll deal with this whenever it comes back up. Alright, once again, thank you. Byes. |
hey again, again. sorry for the super late commit: newest one is a completely separate thing relating to leydrinker (ae echo + er can randomly proc it, and there's no 150ms delay whenever it's refreshed or applied regardless of source). it's quick and fairly straight-forward, so I hope it's no trouble. sorry sorry. |
Another update just so I'm not leaving stuff in the dark: I'm currently re-testing out background effects again. I'll request re-review whenever I'm finished because data gathering'll take an excruciatingly long while. Aside from that, I assume due to spell-batching, I can't really pinpoint precisely whenever intuition's damage effect'll take place for anything below ~100ms; the only thing I could reasonably test (I think) is ToTM, where if you gain intuition from totm -> (literally) immediately click barrage, Intuition's damage is accounted for in the calculation (So I had written for totm to be exempt from 'just_gained_intuition'). Again, I feel like I can't really test any other scenarios because everything that is off the GCD (aside from totm) are background effects, which would require me to somehow find a way to register the moment a proc takes place -> click barrage 50ms after. Either way, if I ensure that background effects are not accounted for within the .react-ion of a buff application (wont register that it's up within the apl), does this matter? Alternatively, I could just ensure that things that proc intuition after its spell cast (surge and blast -- everything else has enough time to nullify this via the GCD) are the only scenarios where that intuition damage effect wont take place. However, I suppose this is again leaning on whether or not this occurs with background effects; I don't know if I'm under-thinking or over-thinking this through because this is literally all about 30ms, and wouldn't really be relevant if the APL uses .react. |
…t roll the proc chance for cc
Hey. I believe I'm finished. For future reference, attached (notes.txt) has an overview of how things behave, or if you wanna use it to proof read through the changes. edit: forgot to say thank you, my bad. Thank you! bye. |
sorry again, (another) late commit. this doesn't change the behaviour, just contains a small expression that'll (probably, hopefully) be useful whenever writing the APL: primarily for waits, and being able to find different, queueable spells to use during that brief moment. |
Hey. I probably should've made two separate branches, but the tier effect change is fairly straight-forward (they're kinda linked with one another), so I hope it's no trouble -- can fix it if you'd like.