Skip to content
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.

Sign up for GitHub

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

Jump to bottom

make/test timeline has weird offset with log file #6048

Open
quisquous opened this issue Dec 18, 2023 · 2 comments
Open

make/test timeline has weird offset with log file #6048

quisquous opened this issue Dec 18, 2023 · 2 comments
Labels
bug

Comments

@quisquous
Copy link
Owner 

$ node --loader=ts-node/esm util/logtools/make_timeline.ts -f docs/logs/TheAbyssalFractureExtreme.log -lf 1
(node:24172) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This
 feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
### THE ABYSSAL FRACTURE (EXTREME)
# ZoneId: 491

hideall "--Reset--"
hideall "--sync--"

0.0 "--sync--" InCombat { inGameCombat: "1" } window 0,1
72.8 "--sync--" Ability { id: "8C49", source: "Zeromus" }
75.8 "--sync--" Ability { id: "8C49", source: "Zeromus" }
83.9 "Abyssal Nox" Ability { id: "8B3F", source: "Zeromus" }
92.9 "--sync--" Ability { id: "8B41", source: "Zeromus" }
92.9 "--sync--" #Ability { id: "8B40", source: "Zeromus" }
92.9 "--sync--" #Ability { id: "8B40", source: "Zeromus" }
92.9 "--sync--" Ability { id: "8D2B", source: "Zeromus" }
97.9 "--sync--" Ability { id: "8B41", source: "Zeromus" }
97.9 "--sync--" Ability { id: "8B40", source: "Zeromus" }
99.9 "Abyssal Echoes" Ability { id: "8B42", source: "Zeromus" }
# etc etc etc

TODO: for some reason, make_timeline.ts is confused here and thinks the first
real ability (Abyssal Nox 8B3F) occurs at time=83.9.
It's clear from the log that it should be t=11.1.

Similarly test timeline doesn't work without a large sync on in combat.

This could be identical to #5716
@quisquous quisquous added the bug label Dec 18, 2023
@quisquous
Copy link
Owner Author

See log added in #6051

@xiashtra
Copy link
Contributor

This looks similar to #5635

This was referenced Dec 23, 2023
Closed
Merged
JLGarber pushed a commit to OverlayPlugin/cactbot that referenced this issue Dec 24, 2023
@wexxlee
util: fix encounter start bug in make/test timeline scripts (#15)
eab4545 
(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
gh-pages build: util: fix encounter start bug in make/test timeline s…
b6301db 
…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
@wexxlee @SiliconExarch
util: fix encounter start bug in make/test timeline scripts (quisquou…
f437c84 
…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
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@quisquous @xiashtra