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.

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

raidboss: convert timelines to use netregexes #3547

Closed
quisquous opened this issue Oct 16, 2021 · 3 comments · Fixed by #6042
Closed

raidboss: convert timelines to use netregexes #3547

quisquous opened this issue Oct 16, 2021 · 3 comments · Fixed by #6042

Comments

@quisquous
Copy link
Owner

Currently, timelines are using bare regexes against parsed log lines. Given that the format of these will break in 6.0 and that literally everything else is using network log lines, we should switch these to using netregexes.

Proposing something like this, with the code (at least logically) being eval'd:

-11.5 "Screams Of The Damned" sync /:Suzaku:32D2:/ window 12,20
+11.5 "Screams Of The Damned" sync ability({ name: 'Suzaku', id: '32D2'}) window 12,20

It's probably worth breaking timeline backwards compatibility for this, as it seems unlikely that users will have extensive syncs in their own personal timelines (additional entries: yes, additional syncs: maybe no?). It is possible that users will have replaced entire timelines, which is probably the worst case scenario for breakage. We could consider being backwards compatible to parsed log line syncing as well if we had to, but it might be nice if cactbot only dealt in network log lines everywhere, for efficiency / consistency / network bandwidth when using cactbot remotely.

The blocker here to doing this is that make_timeline.py and test_timeline.py need to be converted to TypeScript first so that they can parse this new timeline format. Although it'd be simple to have make_timeline.py emit this, as NetRegexes.ts is TypeScript, there'd be no (easy) way for test_timeline.py to eval these expressions.

@MaikoTan
Copy link
Contributor

In my opinion, if it is worth breaking backward compatibility in order to use netregexes in a timeline, it is better to change the timeline file format itself instead of adding more syntax that is not easy to be matched by regexes. Technically a timeline file is a data repository that stores abilities that the boss used in a dungeon, for these data, we have data formats like JSON or YAML which are more suitable for interpreting them.

This is a YAML that I can imagine:

name: Ultima Weapon Ultimate
hide:
  - "--sync--"
  - "--Reset--"

entries:
  - t: 0.0
    name: Start
  - t: 9.0
    name: Slipstream
    sync:
      type: ability
      args:
        source: Garuda
        id: 2B53
    window: [10, 5]
  # Or also support a SHORT syntax like before?
  - '12 "Mistral Song"'

This also let us easier to add more syntax (feature) in a timeline.

@quisquous
Copy link
Owner Author

quisquous commented Oct 22, 2021

if it is worth breaking backward compatibility in order to use netregexes in a timeline, it is better to change the timeline file format itself instead of adding more syntax that is not easy to be matched by regexes.

I think I am a bit torn on this. Your suggestion makes writing a timeline parser easier, but I don't think it makes reading or writing a timeline easier. Splitting out everything into a multi-line yaml format would make 300 line timeline files become ~2000 line timeline files.

I would need to look closer at the details, but KDL would be a more interesting alternative to me, as it seems possible to keep a format that is very close to what we had before.

EDIT: mostly my feeling here is that writing a timeline parser is not what I want to make more efficient. The bulk of the time is spent on writing and reading timelines, and so making that as easy as possible is much more important.

@xpdota
Copy link
Contributor

xpdota commented Apr 14, 2022

My 2c is that one very nice thing you'd get by using something like either of those options, is that it would be possible to support things that aren't directly in the log line, like NPC IDs. It could also open the door to future customization via user-added fields.

Something like:

entry time=100 window=[2, 5] sync={type: ability, source: {npcID: 123}, id: 456} text="Something"

This would significantly reduce the need for multilingual timeline replacements, because most things could be done without any name whatever (realistically, that goes for Cactbot triggers as well - most of the multilingual versions are differentiated only in the NPC name).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants