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

util: Add basic network log seal support in timeline tools #5947

Merged
merged 1 commit into from
Nov 30, 2023
Merged

util: Add basic network log seal support in timeline tools #5947

merged 1 commit into from
Nov 30, 2023

Conversation

JLGarber
Copy link
Collaborator

This wasn't as simple as I had hoped, but it does seem to work as it is. There's still a bit of fragility in make_timeline with this implementation, since it still relies on the seal name being present in game log form. We could change this to make "name from game log" optional in the assembleTimelineStrings() function, but that would involve some complex conditions to allow for use of either the network seal or the game log seal.

Alternatively, we could simply not care about game log seals at all, except to check "if there's a game log seal here, use the name it contains". Maybe something like this for the end of assembleTimelineStrings()?

  if (fight !== undefined && fight.sealId !== undefined) {
    if (fight.sealName !== undefined) {
      const sealMessage = SFuncs.toProperCase(fight.sealName);
      const sealComment = `# ${sealMessage} will be sealed off`;
      assembled.push(sealComment);
    }
    const netLogSeal = `0.0 "--sync--" sync / 29:[^:]*:7DC:[^:]*:${fight.sealId}/ window 0,1`;
    assembled.push(netLogSeal);
  } else {
    assembled.push('0.0 "--sync--" sync / 104:[^:]*:1($|:)/ window 0,1');
  }

Ultimately it would be nice to have the seal names dumped to a resource file as we do with ZoneId and ZoneName, and then just look them up here, but I'm not feeling up to CSV stuff this weekend.

@cactbotbot
Copy link
Collaborator

cactbotbot commented Nov 24, 2023

@JLGarber Thanks for your contribution! 🌵🚀

@github-actions github-actions bot added the util label Nov 24, 2023
@JLGarber JLGarber changed the title util: Add basic support for network log seals in timeline tools util: Add basic network log seal support in timeline tools Nov 24, 2023
@valarnin
Copy link
Collaborator

Why not just pull the names from PlaceName.csv like my convert script does?

@JLGarber
Copy link
Collaborator Author

I didn't read the script because I wasn't going to be using it That would be sensible, yes. I think we might still wish to support game log seals just so that we aren't dependent on XIVAPI to update, but you're right, for any other content that is by far the better way to handle it. I'll see what I can do.

@quisquous
Copy link
Owner

I think this is working as-is and so I'm going to merge it and start adjusting existing timelines over to this format.

Looking up PlaceName is a good idea, but can be done as a follow-up.

@quisquous quisquous merged commit fa8a97d into quisquous:main Nov 30, 2023
5 of 6 checks passed
github-actions bot pushed a commit that referenced this pull request Nov 30, 2023
…tools (#5947)

This wasn't as simple as I had hoped, but it does seem to work as it is.
There's still a bit of fragility in `make_timeline` with this
implementation, since it still relies on the seal name being present in
game log form. We could change this to make "name from game log"
optional in the `assembleTimelineStrings()` function, but that would
involve some complex conditions to allow for use of either the network
seal or the game log seal. fa8a97d
@JLGarber JLGarber deleted the make-timeline-seal-changes branch December 1, 2023 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants