oopsy: add translation for oopsy NetRegexes

[Jaehyuk-Lee]

Fixes #764

timelineReplace field

import { TimelineReplacement } from '../ui/raidboss/timeline_parser'; // from raidboss

type SimpleOopsyTriggerSet<Data extends OopsyData> = {
  zoneId: ZoneIdType | ZoneIdType[];
  zoneLabel?: LocaleText;
  triggers?: OopsyTrigger<Data>[];
  timelineReplace?: TimelineReplacement[]; // NEW
} & OopsyMistakeMapFields;

example

ui\oopsyraidsy\data\06-ew\ultimate\the_omega_protocol.ts

const triggerSet: OopsyTriggerSet<Data> = {
  zoneId: ZoneId.TheOmegaProtocolUltimate,
  timelineReplace: [ // this
    {
      'locale': 'de',
      'replaceSync': {
        'Omega': 'Omega',
      },
    },
// ...
    {
      'locale': 'ko',
      'replaceSync': {
        'Omega': '오메가',
      },
    },
  ],
// ...
}

Important

Refactoring Plan: TimelineReplacement

I used TimelineReplacement from raidboss, but I believe this type should now be defined in a common place, in types\trigger.d.ts I think.

I'm also concerned about the name of the type. Oopsy does not have the timeline. I'm still for keeping the original name, TimelineReplacement though. It'll be quite hard to find a good name to cover both raidboss and oopsy.

I am planning to refactor this in this PR. Feel free to make any comments about this plan.

[Jaehyuk-Lee]

This code, checking if regex is an Array, was written a long time ago when we didn't have the NetRegex translation system.

cf. git blame of damage_tracker.ts. This was written 5 years ago.

example from old days

  netRegex: NetRegexes.startsUsing({ source: 'The Manipulator', id: '13E7', capture: false }),
  netRegexDe: NetRegexes.startsUsing({ source: 'Manipulator', id: '13E7', capture: false }),
  netRegexFr: NetRegexes.startsUsing({ source: 'Manipulateur', id: '13E7', capture: false }),
  netRegexJa: NetRegexes.startsUsing({ source: 'マニピュレーター', id: '13E7', capture: false }),
  netRegexCn: NetRegexes.startsUsing({ source: '操纵者', id: '13E7', capture: false }),
  netRegexKo: NetRegexes.startsUsing({ source: '조종자', id: '13E7', capture: false }),

So, I'm removing the if condition, and just use regex (localRegex from my commits). - see 4ed345d

Important

Please leave comments if this change breaks cactbot.

[Jaehyuk-Lee]

Localized netRegexes were still being used in oopsy. I've replaced them with timelineReplace. - see
601287e

[valarnin]

This is, unfortunately, a breaking change as currently written because technically users could still have user js files using this syntax:

  {
    id: 'Some Custom Trigger',
    netRegex: [/^21\|[^|]+\|10001234/, /^22\|[^|]+\|10001234/],
  }

Yet another reason to do some major cleanup before next expansion, with regards to #907.

I don't really have time right now to sit down and think through the logic required to allow this without breaking potential user triggers, but maybe something like this?

    const netRegexAny = Array.isArray(regex) ? Regexes.anyOf(regex) : regex;
    const translated = translateRegex(netRegexAny, parserLang, timelineReplace);

[Jaehyuk-Lee]

@valarnin
🤦‍♂️ Right, user js.
I can just revert the 4ed345d commit.

[Akurosia]

Question regarding translations here:
would this also work for translating the rsv string in oopsy?

[Jaehyuk-Lee]

@Akurosia

would this also work for translating the rsv string in oopsy?

I'm not sure what you're asking about, can you ask me more specific or give me an example to check if it works? I'm not understanding well about how cactbot is handling RSV.

...

Edit: Ah, well... I edited this whole comment.

I have to read more code in raidboss_config.ts and damage_tracker.ts (oopsy)
I'll ping you later again with a new comment.

[jacob-keller]

RE naming, we could use something like "AbilityNameReplacement" or "TranslationReplacements" or something along those lines

[Jaehyuk-Lee]

@Akurosia

1. Trigger Initialization: ProcessTrigger()

When Oopsy loads trigger files (e.g. ui\oopsyraidsy\data\06-ew\ultimate\the_omega_protocol.ts), ProcessTrigger() is called for each trigger.

the_omega_protocol.ts
-> ProcessTrigger()
-> Translates netRegex into the valid parserLanguage regex (← This is the change introduced by this PR)
-> Pushes to the internal triggers array
-> Waits for incoming logs

2. Runtime Processing: OnNetLog()

When a network log line arrives from ACT/OverlayPlugin, OnNetLog() iterates through the active triggers.

// Process triggers after abilities, so that death reasons for abilities that do damage get
// listed after the damage from that ability.
for (const trigger of this.triggers) {
  // internal triggers array contains the already-translated `localRegex`
  const matches = trigger.localRegex.exec(line);
  if (matches)
    this.OnTrigger(trigger, matches, this.lastTimestamp);
}

• this.triggers contains the triggers with localRegex (which were translated in Step 1).
• If matches finds a hit, the trigger executes.

Conclusion

If RSV data were not already decoded before reaching OnNetLog, even the standard English triggers would fail to match (as they expect text names, not RSV IDs).
The fact that English triggers work confirms that OnNetLog receives decoded text.

Therefore, this PR only needs to ensure that the netRegex string is correctly translated to match the client's language, which is now handled in ProcessTrigger(). Since this implementation utilizes the existing translation functions already proven and used extensively in raidboss, the likelihood of issues arising from this change is extremely low.

[Jaehyuk-Lee]

Tests

logs testing

It works when I tested on http://localhost:8080/ui/oopsyraidsy/oopsy_viewer.html

after:

Real-time testing

It's also working with the game running.

[Souma-Sumire]

Just a heads-up. If you want to rename fields, such breaking changes might not be merged until the release of version 8.0.

[jacob-keller]

Just a heads-up. If you want to rename fields, such breaking changes might not be merged until the release of version 8.0..

I'd say is to keep the same for now (but maybe move it to types.d) and make an issue to suggest the breaking change rename as prep for 8.0

[Jaehyuk-Lee]

@Souma-Sumire @jacob-keller Oops, sorry 😓 I've reset commits and removed the renamings. I'll add a comment on #907

[valarnin]

This is, unfortunately, a breaking change as currently written because technically users could still have user js files using this syntax:

  {
    id: 'Some Custom Trigger',
    netRegex: [/^21\|[^|]+\|10001234/, /^22\|[^|]+\|10001234/],
  }

Yet another reason to do some major cleanup before next expansion, with regards to #907.

I don't really have time right now to sit down and think through the logic required to allow this without breaking potential user triggers, but maybe something like this?

    const netRegexAny = Array.isArray(regex) ? Regexes.anyOf(regex) : regex;
    const translated = translateRegex(netRegexAny, parserLang, timelineReplace);

[Jaehyuk-Lee]

@valarnin I'm sending you a reminder that I've reverted the breaking change by 9350458. Sorry if you just didn't have time to review. 😅

[Jaehyuk-Lee]

60186d6: removed localized netRegex types. This won't break user js. I've tested ui/oopsyraidsy/data/00-misc/test.ts on Korean ffxiv. It's working fine.

TODO: oopsy needs 'find missing translation' function now, like raidboss.

[valarnin]

This looks good to me now. Sorry for the late re-review, for some reason github didn't notify me of the revert or your comment pinging me.

If you don't plan to start working on the find missing translations functionality, please open an issue to follow up on it later.

Additionally, please comment in the breaking changes issue that we should drop support for the lang-keyed netRegex properties for 8.0, and revisit the change that was reverted.

[Jaehyuk-Lee]

I'll make another follow-up PR or an issue later. You can merge this PR now.