Skip to content

[Warlock] Class adjustments for more faithful behavior of Mug's Moxie Jug trinket procs #10298

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

Open
wants to merge 1 commit into
base: thewarwithin
Choose a base branch
from

Conversation

millanzarreta
Copy link
Contributor

@millanzarreta millanzarreta commented May 30, 2025

The Mug's Moxie Jug trinket presents some issues in Simc. This trinket has two drivers: the main driver, which is responsible for processing the buff activation (15sec ICD), and the secondary driver, which increases the stacks while the buff is active (750ms ICD). Both effects can be triggered by a multitude of player actions: some healing effects, some damage effects, energize effects, some gain/drop/refresh auras, and even some triggers from periodic hidden auras/spells that don't appear in the combatlog or the logs.

Simc doesn't include the implementation of many of these effects, which causes some issues with the trinket. For the main driver, this isn't a big problem, as it's RPPM-based. In Simc it has been established for main driver that it can be triggered by many casts, damage effects, and healing effects. This should be sufficient for it to trigger periodically based on RPPM, so no major changes are needed. The problem is the secondary driver that increases stacks while the buff is present, which is not RPPM-based and always increases the buff by one stack with any of its activations (is 100% chance, so it is activating all the time with all these effects/spells, as long as they occur after its 750ms ICD). In Simc, many of these effects are not implemented, so the buff doesn't increase stacks as quickly as it does ingame for many specs, and that's the problem.

Commit 46eea16 has attempted to make the secondary driver increase stacks more quickly, including most damage and healing casts and effects. This isn't entirely accurate, but it makes its effectiveness more similar to ingame.

This pevious discarted PR provided more information on the issue and proposed an alternative option for a user to set up an alternative way for the trinket to periodically gain stacks based on a timer. However, it was discarded because it was a workaround that didn't address the root of the problem in the warlock module. Although this issue is more general and may affect other classes, it's best to implement it faithfully in the class modules, and for other classes seem that the difference between sims and ingame results doesn't seem as significant as it is for the warlock (specially the demonology).

Commit 53ce36e was then implemented to try to improve this. Although it is still a large workaround hack, it has been tuned to produce results that are more faithful to the observed ingame results.


This new PR aims to provide a faithful implementation of trinket procs for all three warlock specs as suggested. To achieve this, most of the modifications are made to the warlock module, although some specific adjustments to the trinket's warlock settings are also necessary. This should result in a much more faithful implementation of this trinket, and also a better tuning of the warlock module to work properly also with other trinkets/effects/procs.

All warlock spells have been tested ingame to document which ones trigger the trinket's secondary driver and which ones don't. This is then used to determine whether the current Simc implementation behaved correctly. An Excel spreadsheet detailing the results is attached below, along with their behavior in the current Simc implementation and the new implementation after this PR:

wl_trinket_triggers.xlsx

Below we will discuss some of the most relevant things about the Warlock's spells and the trinket's secondary driver procs:

  • The gear leech doesn't proc the trinket secondary driver.
  • The healing from the Fel Synergy talent also doesn't proc.
  • The damage dealt to the pet with Soul Link doesn't proc either.
  • It was believed that Soul Leech was continuously triggering the trinket with each aura refresh whenever damage is dealt, but this is not true. It has been tested for both the timed and permanent versions of the aura that the trinket procs can only happen when the warlock or pet gains the buff because they didn't have it, but not when the aura is refreshed. This means that the impact the aura was believed to have on the trinket is actually much smaller than previously thought. This spell isn't implemented in Simc, but as stated, it's not as significant.
  • Some other classes/specs have periodic unknown hidden auras that continuously trigger the trinket. In the case of the warlock, there is at least one periodic hidden effect, but it only triggers the main driver (which doesn't matter much because it's PRRM-based) and not the secondary driver.
  • Most utility spells trigger the trinket procs, although these aren't implemented in Simc.
  • Some buffs applied by the player (either to themselves or to other units like their pets) trigger the trinket procs, although not all.
  • All the current warlock spells that directly apply a debuff to the target (by the player) allow procs.
  • Actions, spells, buffs/debuffs and damage events from the warlock pets do not trigger the trinket procs. The only exceptions are when pet actions apply debuffs that are actually applied by the player (the logs show that the aura is applied by the player), such as the following:
  • Summoning pets doesn't always procs the trinket, it depends on the pet. Summoning main pets does, as well as Dreadstalkers, Tyrant, and Wild Imps from Demonology. However, other pets such as the Infernals, Vilefiend, or the three demons from the Diabolist Hero tree do not.
  • When the Soul Harvester hero tree talent is selected (for both Affliction and Demonology), this causes any damage the player deals directly (pets don't count) to trigger the trinket procs. This includes things that don't normally procs, such as DoT damage ticks, melee hits, or trinket-damaging effects.
  • Most energize effects (gaining soul shards) trigger the trinket procs, and do so even if the player is soul shard capped. However, some do not, such as the periodic regeneration of Infernals or Overfiends.
    • A special case is the Felguard's Soul Strike with the Fel Invocation talent. Normally, the trinket procs aren't triggered for this energize spell, but with the Soul Harvester tree selected it does, but only if the effect effectively gains a Soul Shard (if the player isn't soul shard capped).
  • Normally DoT damage doesn't trigger the trinket secondary driver, although as previously mentioned, this isn't the case for Soul Harvester. There are also other exceptions to this:
    • For the Destruction Warlock, each tick of Immolate and Wither procs the trinket. It's difficult to know if this is due to each tick generating a Soul Shard or some other related background effect.
    • For the Affliction Warlock, the Corruption DoT ticks always trigger procs because this spell only exists for Soul Harvester.
      • Wither replaces it for Hellcaller. Wither DoT ticks for Affliction only allow the trinket to proc in one of these two circumstances:
        • With the Siphon Life talent for all ticks
        • Without the Siphon Life talent, when the tick is on a target below 20% health, as this triggers the collapse of the stacks. This occurs for all ticks in that HP condition, even if the collapse has already ended and only one stack remains.
  • Casting Rain of Fire does trigger the trinket procs, but the damage ticks don't. However, if you have the Pyrogenics talent selected, each RoF tick will renew the application of this debuff and this does trigger the trinket procs.
  • Finally, it's important to note that in many cases it's very difficult to know if the trinket proc is triggered by a cast, the application of a buff, the application of a debuff, an energize effect, etc., since many of these effects occur simultaneously. In the attached spreadsheet, effects that aren't checked are marked with X, as they occur alongside others that already trigger the trinket. In Simc, in most of these cases, the most convenient option for implementation is chosen, which is usually the associated CAST spell event.
  • Special mention should be made of the Infernal Command aura, which is a remnant of an old demonology warlock talent that no longer exists. However, the aura still exists ingame, although its value is 0 and therefore the damage increase is 0%. This aura is relevant because its application triggers the trinket, and it has strange behavior: this aura is periodically applied/removed for each wild imp approximately every 5.25 seconds from its spawn. Since this aura had no effect, it was not implemented in Simc, and its implementation is also added in this PR to allow these procs.

Regarding more specific implementation details in this PR, the following are worth highlighting:

  • Normally, the second driver does not trigger DoT damage, but as previously mentioned, there are exceptions. This is managed with a custom callback configuration in the Warlock's trinket settings.
  • For the most relevant pet summons that are not direct actions in the current simc implementation, direct summon spell actions have been added, and now these actions are called instead of doing directly a .spawn().
  • By default in Simc, buffs/debuffs and energize effects do not trigger trinkets, and to do so, they must be wrapped in an action. This can be done in several ways. The way we chose was to use a dummy helper action (buff_apply_action) for applying relevant buffs/debuffs and another dummy helper action (energize_action) for the relevant energize effects. This has been done so that it's only necessary to replace the current line that triggers the buff/energize with a very similar one that wraps the buff/energize trigger into the action. Other alternatives were, for example, creating a single global dummy action and executing it after each buff/energize in a new line, which would somewhat "decouple" the effect from the action and be more error-prone; or creating a new separate action for each buff/energize effect that required it, which would be very verbose and add confusing duplicate actions to the spells. I guess another valid alternative would be to create our own warlock_buff_t derived from buff_t where some additional parameter indicates whether it has an associated action. So, if you think any of these or some other approach is preferable to the one implemented, please feel free to comment.

The changes have been thoroughly tested and debugged, and they appear to be working correctly, producing the results shown in the attached spreadsheet. This PR includes a considerable number of changes, so additional reviews are always welcome. Let me know if you have any thoughts, comments, or suggestions.

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

Successfully merging this pull request may close these issues.

1 participant