OverlayPlugin / cactbot

FFXIV TypeScript Raiding Overlay
Apache License 2.0
85 stars 34 forks source link

Cactbot Timers in Castrum Lacos Litore #96

Closed ZenithAether closed 5 months ago

ZenithAether commented 5 months ago

Description

Cactbot Timers not stopping

Additional Details / Steps to Reproduce

In Castrum Lacos Lictore after the 3rd Boss and the Final Boss the Timers arent stopping and still ongoing after leaving the Castrum or after finished the 3rd Boss Fight.

Cactbot Module

Raidboss (alerts & timelines)

Configuration Info

Plugin Name          Enabled Version  Path
---------------------------------------------------------------------------------------------------------------------------------------
FFXIV_ACT_Plugin.dll True    2.6.9.9  C:\Users\AppData\Roaming\Advanced Combat Tracker\Plugins\FFXIV_ACT_Plugin.dll
OverlayPlugin.dll    True    0.19.27  C:\Users\AppData\Roaming\Advanced Combat Tracker\Plugins\OverlayPlugin\OverlayPlugin.dll
CactbotOverlay.dll   True    0.31.3.0 C:\Users\AppData\Roaming\Advanced Combat Tracker\Plugins\cactbot\cactbot\CactbotOverlay.dll
Triggernometry.dll   False   1.1.7.4  C:\Users\AppData\Roaming\Advanced Combat Tracker\Plugins\Triggernometry.dll

Overlay Name URL
---------------------------------------------------------------------------------------------------------------------------------
Raidbossy    file:///C:/Users/AppData/Roaming/Advanced%20Combat%20Tracker/Plugins/cactbot/cactbot/ui/raidboss/raidboss.html
DPS Meter    https://hibiyasleep.github.io/kagerou/overlay/
Hunt Radar   file:///C:/Users/AppData/Roaming/Advanced%20Combat%20Tracker/Plugins/cactbot/cactbot/ui/radar/radar.html
Fishing      file:///C:/Users/AppData/Roaming/Advanced%20Combat%20Tracker/Plugins/cactbot/cactbot/ui/fisher/fisher.html
Eureka       file:///C:/Users/AppData/Roaming/Advanced%20Combat%20Tracker/Plugins/cactbot/cactbot/ui/eureka/eureka.html

Various Settings                                Value
------------------------------------------------------------------------------------------------------------------------------------
Plugin Language                                 German
Machina Region                                  Global
Game Version                                    2024.02.05.0000.0000
Screen Mode                                     Borderless Windowed
Inject and use Deucalion for network data       True
Hide Chat Log (for privacy)                     False
Use WinPCap-compatible library for network data False
Disable high-performance network filter         False
Disable Combine Pets with Owner                 False
Disable Damage Shield estimates                 False
(DEBUG) Enable Debug Options                    False
(DEBUG) Log all Network Packets                 False
(DEBUG) Also Show 'Real' DoT Ticks              False
(DEBUG) Simulate Individual DoT Crits           False
(DEBUG) Graph Potency, not Damage               False
(DEBUG) Enable Benchmark Tab                    False
Cactbot User Dir                                C:\Users\AppData\Roaming\Advanced Combat Tracker\Plugins\cactbot\cactbot\user\

Log & Screenshots

No response

Confirmation

wexxlee commented 5 months ago

Per discussion in ACT discord: no log file attached because the log splitter apparently doesn't recognize CLL or Bozja CEs as exportable encounters, and the web version of the splitter doesn't allow for an entire log to be anonymized and exported. I'm going to PR an option to the web splitter to allow for full anonymize/export (mirroring functionality of existing CLI tool) so we can troubleshoot/analyze logs where the splitter isn't correctly recognizing the start/end of certain encounters.

ZenithAether commented 5 months ago

What I also noticed is that as soon as the timers were bugged once, they no longer appear at all in the next CLL, but the information messages with "Right" or "left" do.

wexxlee commented 5 months ago

@ZenithAether Apologies for the delay. The log splitter has now been updated to allow the entire log file to be anonymized/exported without being split: http://overlayplugin.github.io/cactbot/util/logtools/splitter.html

If you could do that and upload your anonymized log file that includes the CLL run, we'd be able to take a closer look at this. Thanks for your patience!

ZenithAether commented 5 months ago

Hello here is now the logfile. I had to zip it because the Original file had 197 MB. processed.zip

Also the splitter bought up these Error or Warns: warn: expected id field at index 6:21|2024-03-14T16:42:24.6340000+01:00|10FF0119|Seseda Seda|798D|Moonbeat|7FF72F||0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|||||||||||25429|25429|10000|10000|||-199.77|848.57|5.14|2.74|00050248|0|1| warn: expected id field at index 6:21|2024-03-14T16:48:35.6690000+01:00|10FF0119|Seseda Seda|798D|Moonbeat|7FF72F||0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|||||||||||25429|25429|10000|10000|||-196.94|843.62|5.12|2.68|00051080|0|1| warn: expected id field at index 6:21|2024-03-14T16:48:48.2070000+01:00|10FF0119|Seseda Seda|798D|Moonbeat|7FF72F||0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|0|||||||||||25429|25429|10000|10000|||-197.12|849.70|5.16|2.27|000510C6|0|1| error: uncaught player id 10749710, idx: 5:37|2024-03-14T21:12:54.8160000+01:00|4000056F|Bozja-Phantom|00009A5F|10749710||||||212.54|-349.78|-97.00|-1.78|

wexxlee commented 5 months ago

@JLGarber, I've got a theory about this, but this is a part of the code you know much better than I do, so would appreciate your thoughts.

In the Bozja timeline, the resets use a window 100000 command: https://github.com/OverlayPlugin/cactbot/blob/83e4de243c44acdc540f1aecdc685efc8def4750/ui/raidboss/data/05-shb/eureka/bozjan_southern_front.txt#L10-L12

I haven't run into a window command that only has a single param before (as opposed to the more familiar [start,end] format), but looking through timeline_parser.ts, I think the windowCommand regex would match this value as the end and leave start undefined (at least, it did when I did a mockup test): https://github.com/OverlayPlugin/cactbot/blob/83e4de243c44acdc540f1aecdc685efc8def4750/ui/raidboss/timeline_parser.ts#L158-L159

In that case, with start being undefined, it looks like the parser treats 100000 as a single window (in other words, it adds half of the window as a lookbehind and half as a lookahead). Since the time (seconds) for these entries is 0, it would set a start of -50000, and an end of 50000:

https://github.com/OverlayPlugin/cactbot/blob/83e4de243c44acdc540f1aecdc685efc8def4750/ui/raidboss/timeline_parser.ts#L609-L614

Which makes perfect sense.... except that the CLL timeline for the final boss (Dawon/Lyon) has entries at a time of 60000+: https://github.com/OverlayPlugin/cactbot/blob/83e4de243c44acdc540f1aecdc685efc8def4750/ui/raidboss/data/05-shb/eureka/bozjan_southern_front.txt#L357-L359

If I'm right about the way the unbounded window works, then I think it wouldn't match the ActorConrol at the end of CLL (which is definitely in the log) because it would occur after the +50000 sync window. (Edit: I think this would also explain why the timeline continues to run after CLL, and also why the timeline never starts again on a subsequent CLL in the same instance.)

I think that would be pretty easy to fix, by just changing window 100000 to be window 200000.

Does that sound right to you?

JLGarber commented 5 months ago

That sounds correct, although I think in this case it would be better to have an explicit window 100000,100000 just to keep things clear. I wonder whether it might be worth adding a test for whether all explicit window parameters have two arguments, just to avoid this ever happening elsewhere.

wexxlee commented 5 months ago

I wonder whether it might be worth adding a test for whether all explicit window parameters have two arguments, just to avoid this ever happening elsewhere.

Although the behavior isn't documented, it does seem by-design given the code, and there are examples of single-param windows in TimelineGuide: https://github.com/OverlayPlugin/cactbot/blob/83e4de243c44acdc540f1aecdc685efc8def4750/docs/TimelineGuide.md?plain=1#L423-L435

That, plus the possibility people may be doing it in user files - rather than break it or warn about it, maybe we could just add a note to the TimelineGuide documenting what the behavior is when only one param is specified? (FWIW, I took a quick look, and while it does exist elsewhere in the bozja/eureka stuff, the sync window amply covers the entire timeline).

JLGarber commented 5 months ago

Oh, I absolutely agree we shouldn't modify the behavior itself. I'm mostly thinking here that we might want to require any usages within the repository to define both start and end of a window, as a matter of style. (I sort of see it as analogous to how the raidboss module will accept bare regexes in triggers, since users might do that for whatever reason, but we do not allow it for repository triggers.)

wexxlee commented 5 months ago

Oh, I absolutely agree we shouldn't modify the behavior itself. I'm mostly thinking here that we might want to require any usages within the repository to define both start and end of a window, as a matter of style. (I sort of see it as analogous to how the raidboss module will accept bare regexes in triggers, since users might do that for whatever reason, but we do not allow it for repository triggers.)

Yeah, makes sense. I'll drop a PR to fix this one for now (and add some documentation to TimelineGuide), and then do a follow-up PR to clean up any stray single-param references and add a linting test.

ZenithAether commented 5 months ago

I recently ran the Dalriada for comparison. Everything seems to fit with the timers and there is no overlap like between Adrammelech and Lyon in CLL.