make_timeline/test_timeline drift #5635
Labels
Comments
This was referenced Dec 23, 2023
JLGarber
pushed a commit
to OverlayPlugin/cactbot
that referenced
this issue
Dec 24, 2023
(sorry for the git bork/closed PR - one more time...) This PR should address the encounter sync drift referenced in quisquous#5635 and quisquous#6048, as well as the missing zone-seal sync referenced in quisquous#5716. There were a couple of separate but related issues that I found: 1. `encounter_tools` had not (yet) been updated to use `InCombat` lines to start encounters, even though `make_timeline` is inserting an `InCombat` sync at the start of a new timeline. 2. For fights that do not have zone seals, `encounter_tools` would fall back on using `playerAttackingMob` or `mobAttackingPlayer` regex. While this mostly still worked (with minor drift issues), it was also counting faerie healing actions as the start of the fight. I confirmed this was the case with the log in issue 6048. I don't have logs for the original report from issue 5635, but I suspect a similar cause there, as I was unable to repro in e6n when on non-pet classes. 3. I think there was a minor logic bug in `encounter_tools` re: pushing the fight-starting log line into `logLines`. When encountering certain log lines that should trigger a new fight encounter, `onStartFight()` would reinitialize `this.currentFight` and set `.startTime`; but when `storeStartLine()` was subsequently called, it would not push the starting log line into `.logLines` because the fight already had a start time set. This was causing make/test_timeline to sometimes not have access to (and not be able to sync on) the log line that started the encounter. I'm wary about unintentional breakage, given the various different events that should (or should not) start a timeline. cc: @xiashtra and @JLGarber, would appreciate an extra set of eyes.
github-actions bot
pushed a commit
to OverlayPlugin/cactbot
that referenced
this issue
Dec 24, 2023
…cripts (#15) (sorry for the git bork/closed PR - one more time...) This PR should address the encounter sync drift referenced in quisquous#5635 and quisquous#6048, as well as the missing zone-seal sync referenced in quisquous#5716. There were a couple of separate but related issues that I found: 1. `encounter_tools` had not (yet) been updated to use `InCombat` lines to start encounters, even though `make_timeline` is inserting an `InCombat` sync at the start of a new timeline. 2. For fights that do not have zone seals, `encounter_tools` would fall back on using `playerAttackingMob` or `mobAttackingPlayer` regex. While this mostly still worked (with minor drift issues), it was also counting faerie healing actions as the start of the fight. I confirmed this was the case with the log in issue 6048. I don't have logs for the original report from issue 5635, but I suspect a similar cause there, as I was unable to repro in e6n when on non-pet classes. 3. I think there was a minor logic bug in `encounter_tools` re: pushing the fight-starting log line into `logLines`. When encountering certain log lines that should trigger a new fight encounter, `onStartFight()` would reinitialize `this.currentFight` and set `.startTime`; but when `storeStartLine()` was subsequently called, it would not push the starting log line into `.logLines` because the fight already had a start time set. This was causing make/test_timeline to sometimes not have access to (and not be able to sync on) the log line that started the encounter. I'm wary about unintentional breakage, given the various different events that should (or should not) start a timeline. cc: @xiashtra and @JLGarber, would appreciate an extra set of eyes. eab4545
SiliconExarch
pushed a commit
to SiliconExarch/cactbot
that referenced
this issue
Jan 5, 2024
…s#15) (sorry for the git bork/closed PR - one more time...) This PR should address the encounter sync drift referenced in quisquous#5635 and quisquous#6048, as well as the missing zone-seal sync referenced in quisquous#5716. There were a couple of separate but related issues that I found: 1. `encounter_tools` had not (yet) been updated to use `InCombat` lines to start encounters, even though `make_timeline` is inserting an `InCombat` sync at the start of a new timeline. 2. For fights that do not have zone seals, `encounter_tools` would fall back on using `playerAttackingMob` or `mobAttackingPlayer` regex. While this mostly still worked (with minor drift issues), it was also counting faerie healing actions as the start of the fight. I confirmed this was the case with the log in issue 6048. I don't have logs for the original report from issue 5635, but I suspect a similar cause there, as I was unable to repro in e6n when on non-pet classes. 3. I think there was a minor logic bug in `encounter_tools` re: pushing the fight-starting log line into `logLines`. When encountering certain log lines that should trigger a new fight encounter, `onStartFight()` would reinitialize `this.currentFight` and set `.startTime`; but when `storeStartLine()` was subsequently called, it would not push the starting log line into `.logLines` because the fight already had a start time set. This was causing make/test_timeline to sometimes not have access to (and not be able to sync on) the log line that started the encounter. I'm wary about unintentional breakage, given the various different events that should (or should not) start a timeline. cc: @xiashtra and @JLGarber, would appreciate an extra set of eyes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Breaking this out into a new issue by request (per conversation in #5592 ).
From @xiashtra regarding drift in the e6n timeline:
The text was updated successfully, but these errors were encountered: