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 Emulator: suppressSeconds cannot be used with regex, but it can be used with netRegex. #5530

Open
Souma-Sumire opened this issue Jun 9, 2023 · 9 comments

Comments

@Souma-Sumire
Copy link
Contributor

As mentioned in the title, I have some specific regex requirements, so I used regex. However, I found that it cannot be used with suppressSeconds, and it throws an error saying "Uncaught (in promise) Error: This code shouldn't be reached." I don't understand why this is happening.

Here are two simple demos, and the second trigger will cause the error.

Options.Triggers.push({
  zoneId: ZoneId.TheOmegaProtocolUltimate,
  id: "Souma_Test_TheOmegaProtocolUltimate",
  triggers: [
    {
      id: "TEST 20230610 1",
      type: "StartsUsing",
      netRegex: { id: "6503" }, // Glare III
      suppressSeconds: 999,
      run: () => console.warn(1),
    },

    {
      id: "TEST 20230610 2",
      type: "StartsUsing",
      regex: /^.{14} StartsCasting 14:1.{7}:[^:]+:6503:/,
      suppressSeconds: 999, // if no 'suppressSeconds', no Error.
      run: () => console.warn(2),
    },
    // Error: Uncaught (in promise) Error: This code shouldn't be reached
  ],
});
@Souma-Sumire
Copy link
Contributor Author

Souma-Sumire commented Jun 9, 2023

image

@quisquous
Copy link
Owner

Is this a raid emulator bug only?

Also, what are you trying to do that you need a regex?

@Souma-Sumire
Copy link
Contributor Author

Is this a raid emulator bug only?

Yes, it's an emulator-only bug.

Also, what are you trying to do that you need a regex?

(generated by translation software)

Compared to using the standard construction syntax provided by cactbot and then using the matches in the condition function for logical checks, I prefer quickly writing some regex. For example, directly specifying in the regular expression whether the first digit of an ID is 1 or 4 to quickly filter out unwanted individuals. Or quickly implementing certain functionalities, even though the regex may not be rigorous. I also have the freedom to choose capturing groups. Overall, it offers more flexibility.

This time, I just wanted to quickly match the logs where the LB resets when the gauge is full. So I directly wrote regex: /^.{14} LimitBreak 24:0000:3$/, instead of type: "LimitBreak", netRegex: { bars: "3", valueHex: "0000" },. It's just a matter of habit. Additionally, since I write JavaScript code directly in the user directory, I don't have any TypeScript prompts, so I don't know the netRegex property names corresponding to each type unless I go and check netlog_defs.ts, which is cumbersome. So, writing regex based on the logline directly allows me to achieve the functionality more quickly, even though I understand it may not be standard practice.

@Souma-Sumire
Copy link
Contributor Author

Souma-Sumire commented Jun 9, 2023

Although it's off-topic, I'd like to mention:

Is there a way for me to access data outside of the trigger's callback function? Currently, I'm utilizing a trigger that is guaranteed to be triggered once before the battle, along with a large suppressSeconds. Then I pass a reference to data in the run function for other functions to use. Otherwise, I would need to pass data explicitly every time I call a function, and each function would need to pass it to one another.

The second instance where I need access to data is when I provide some config options for users to control certain functionalities. However, these functionalities actually control some functions that I attach to Util for use in other files. Therefore, I also need this trigger that is guaranteed to be triggered to pass the value of data.triggerSetConfig.xxx. Currently, I have multiple triggers that are matched very frequently but restricted by suppressSeconds. This results in a lot of wasted performance.

If it's possible to obtain data in initData, it would be very convenient.

@quisquous
Copy link
Owner

Otherwise, I would need to pass data explicitly every time I call a function, and each function would need to pass it to one another.

This is what I would recommend doing. Pass data explicitly around to Util functions or anywhere else you need it.

Storing a global reference to data maybe works as the code is written right now as it is only reassigned in Reset, but it's possible that PopupText.data might get reassigned differently in the future if somebody ever changes it. Doing this might also currently break assumptions in raid emulator. It's safer to just pass data around than making assumptions about its lifetime and reusing it across functions.

@Souma-Sumire
Copy link
Contributor Author

Otherwise, I would need to pass data explicitly every time I call a function, and each function would need to pass it to one another.

This is what I would recommend doing. Pass data explicitly around to Util functions or anywhere else you need it.

Storing a global reference to data maybe works as the code is written right now as it is only reassigned in Reset, but it's possible that PopupText.data might get reassigned differently in the future if somebody ever changes it. Doing this might also currently break assumptions in raid emulator. It's safer to just pass data around than making assumptions about its lifetime and reusing it across functions.

I understand. Thank you.

@Souma-Sumire Souma-Sumire changed the title Raidboss: suppressSeconds cannot be used with regex, but it can be used with netRegex. Raidboss Emulator: suppressSeconds cannot be used with regex, but it can be used with netRegex. Jun 9, 2023
@quisquous
Copy link
Owner

Sorry to continue this offtopic conversation, but I had a couple of other thoughts.

Do you have use cases that you find yourself doing a lot that would be helpful? For example, if you have a lot of ui options and need to check if those options are on, I could think that we could add a condition field to trigger sets (that could check role, or name, or job, or triggerset config options) and it would apply to the entire trigger set rather than having to do that on each and every trigger.

My other suggestion also is to write in TypeScript if you can. This is up to your own preference in terms of what is comfortable to program in, but TypeScript and npm run test will catch a lot of errors. If I were writing a lot of cactbot user JavaScript code, I would instead do this:

  • add new TypeScript files directly in ui/raidboss/data/souma/ or whatever
  • you have to add these files to raidboss_manifest.txt for now, but soon build: Build manifest automatically #5480 will make that not necessary
  • write code in TypeScript
  • run npm run test to verify things are working
  • convert to JavaScript automatically using something like process_triggers_folder.ts

Right now, things like p12s.ts are automatically turned into p12s.js by this script and should be suitable for adding to a cactbot user folder. Using an automated script like this would let you use TypeScript if you wanted.

@Souma-Sumire
Copy link
Contributor Author

Souma-Sumire commented Jun 9, 2023

It's great to continue the discussion! Regarding the built-in fields, the new config configuration method is already very effective, but there are still some optimizations that I think can be made:

  1. Global override for modifying outputs, such as default texts for [north/east/south/west/dirNESW/NE/SE/SW/NW], instead of specifying them individually for each trigger's output. (Please ignore the incorrect practice of writing Chinese strings in the EN attribute, as this is only for distribution on the CN server, and EN is a required option.)

  2. Currently, I have a JS file, ID2Job.js, which hijacks Options.PlayerNicks to replace player IDs with their corresponding job names. This is very popular on the CN server. For this, I have a one-time trigger to retrieve data.party (previously, I registered the partyChanged event to get the party, but it seems that the trigger from overlayPlugin is not very stable, so I switched to using Proxy). It would be great if this functionality could be built-in.

Thank you for providing a detailed tutorial on how to use TypeScript. I will give it a try (previously, I didn't understand the correct way).

@Souma-Sumire
Copy link
Contributor Author

Souma-Sumire commented Jun 9, 2023

It's great to continue the discussion! Regarding the built-in fields, the new config configuration method is already very effective, but there are still some optimizations that I think can be made:

  1. Global override for modifying outputs, such as default texts for [north/east/south/west/dirNESW/NE/SE/SW/NW], instead of specifying them individually for each trigger's output. (Please ignore the incorrect practice of writing Chinese strings in the EN attribute, as this is only for distribution on the CN server, and EN is a required option.)
  2. Currently, I have a JS file, ID2Job.js, which hijacks Options.PlayerNicks to replace player IDs with their corresponding job names. This is very popular on the CN server. For this, I have a one-time trigger to retrieve data.party (previously, I registered the partyChanged event to get the party, but it seems that the trigger from overlayPlugin is not very stable, so I switched to using Proxy). It would be great if this functionality could be built-in.

Thank you for providing a detailed tutorial on how to use TypeScript. I will give it a try (previously, I didn't understand the correct way).

Oh, you mean conditions. I think I misunderstood you. The translation wasn't very clear. But if it's about conditions, then there's no problem. I can check data.triggerSetConfig in the conditions. Currently, there is no need to adjust trigger sets with different triggers but the same conditions in large numbers.

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

No branches or pull requests

2 participants