gcsim
gcsim copied to clipboard
Rework Logging of Dynamic Buffs
Currently dynamic/conditional buffs (think 4 Blizzard Strayer) are handled as loose-leaf event subscriptions that directly modify snapshots prior to damage instances. This works ok for calculation and coding purposes, but it does not promote consistency and clarity to the end user. A few things that I've noticed:
- Dynamic buffs are often logged inconsistently, such as 4BS showing up the "Calc" log, while Lion's Roar only shows up in the "Weapon" log. Raiden A4 only shows up in the "Character" log.
- In contrast, snapshottable buffs are basically all handled through the "Mod" interface for each character and are easily visible in a consolidated view.
This may require some additional legwork, but I think we should have some similar mechanism as the "Mod" framework to handle dynamic buffs. In general how these buffs all work internally is that they take the snapshot being used to calculate damage and just manually modify the stats after checking some condition, but it should ideally go through some kind of middle layer.
so this should have been addressed in #105 but will need to go through one by one and do some clean up on the logging part of it. system should be ready though
@Tsym-or-Tysm thoughts on closing this? is the predamagemod implementation good enough to cover this?
@srliao The piece that we're still missing from this are flat damage buffs from Shenhe/Yun Jin/Cinnabar Spindle/Redhorn. I think we'd want to think of a way to track those properly in the system somehow?
@srliao The piece that we're still missing from this are flat damage buffs from Shenhe/Yun Jin/Cinnabar Spindle/Redhorn. I think we'd want to think of a way to track those properly in the system somehow?
hmmm any thoughts on specific? for example do we want it to show up in summary stats somehow? or does it just need to show up in debug in a more organized/consistent fashion?
I was thinking both are useful actually, but more generally there should be a system for tracking the buff that isn't just using the "OnAttackWillLand" hook I think. Perhaps we should change PreDamageMods so that it also applies to flat damage?
Not sure exactly, but IMO this is low down on the priority list since it works well enough for now.