shalzuth / LostArkLogger

Lost Ark DPS Meter
GNU General Public License v3.0
170 stars 107 forks source link

Delay End of Encounter to address issue #55 #84

Closed darkamgine closed 2 years ago

darkamgine commented 2 years ago

As I understand it, when damage packets arrive after the end of raid, it causes the undesired behavior of splitting the encounter into two runs, with the second one consisting solely of the final damage.

This is my attempt to patch this issue by delaying the closing of the encounter window by 5 seconds (currently hardcoded, but seems to be a good starting point from my limited testing) so that late arriving damage packets can be fitted into the current window.

Note that I'm not a C# programmer, so not sure if this is the right way to handle such a behavior. Just found some example of Stackoverflow to work off of.

Also, the AppendLog(2); line seems to cause issues with compiling for me, not sure why.

shalzuth commented 2 years ago

While this fixes the issue, delaying the log isn't the correct way I don't think. AppendLog is added in a new patch (you might need to rebase).

darkamgine commented 2 years ago

Can you elaborate on why you feel this solution isn't the correct way? There shouldn't be any encounters that would go back to back with overlaps less than 5 seconds, nor would I expect damage to be delayed further than 5 seconds after the kill.

It's fairly common for event processors like Kafka to set windows with a late watermark of a few seconds or minutes to allow late arriving events to still fit within the original window. And given the simplicity of this workaround, it would be trivial enough to revert this later if you can come up with a better technical solution while resolving this very annoying issue in the meantime.

shalzuth commented 2 years ago

I've added this in the latest branch, but i use tasks instead.