raidboss: avoid multiple callouts during r2s cone tankbuster

[jacob-keller]

The cone tankbuster during r2s appears on both tanks. This results in
the callout for the tankbuster triggering twice, with an overlapping
sound effect and multiple messages on screen.

Setting suppressSeconds would resolve the multiple callouts, but
prevents correctly calling out "tank cleave on YOU".

All of the tank buster cleaves happen simultaneously. Add a collector
trigger to collect the targets of the tank buster. Add delaySeconds to
the tankbuster trigger to give time for the collector to execute.

Replace the check against the target with a check on whether the
collected targets includes the active player. This ensures that the
correct message is displayed, and prevents the overlapping of
simultaneous triggers.

I chose not to add a check on the size of the laser collect, as this
will at least allow it to make some noise if for some reason the log
didn't include all of the expected tank busters.

[jacob-keller]

This might be a little overkill, but having the simultaneous callouts is problematic because the sound clips overlap and generate constructive interference (i.e. extremely loud, especially for alert or alarm triggers).

I tried a couple of alternatives to keep using the Responses.tankCleave(), but couldn't figure out how to get it to work. I was hoping that suppressSeconds and delaySeconds would interact nicely, but that didn't seem to work.

[valarnin]

Just suppressSeconds alone seems to be sufficient to prevent the double callout here?

    {
      id: 'R2S Headmarker Cone Tankbuster',
      type: 'HeadMarker',
      netRegex: { id: headMarkerData.tankLaser, capture: true },
      suppressSeconds: 1,
      response: Responses.tankCleave(),
    },

[wexxlee]

More a suggestion/for future reference, but you can also just make this a string[] and push matches.target rather than the entire matches object (and no need for importing the type then). If not using anything other than the target field, it's not necessary to store the entire regex capture object on data.

[wexxlee]

I don't think this early exit will ever be triggered -- the first time the trigger is fired, the delaySeconds func will push the first match to data.tankLaserCollect, so data.tankLaserCollect.length will never be 0 by this point? Maybe instead:

Suggested change

	if (data.tankLaserCollect.length === 0)
	return;
	if (data.tankLaserCollect.length < 2)
	return;

[jacob-keller]

Let me check, I think it can because the tankLaserCollect is cleared in the run() function. I modeled this after the way that other single-trigger collect things work: the collect is done in delay seconds, then the alert function checks the length, while the run function clears the array. Is that not correct?

[wexxlee]

Let me check, I think it can because the tankLaserCollect is cleared in the run() function. I modeled this after the way that other single-trigger collect things work: the collect is done in delay seconds, then the alert function checks the length, while the run function clears the array. Is that not correct?

Key things to note here are that (1) this trigger will fire twice; (2) the functions will be executed sequentially, e.g., delaySeconds -> alertText -> run each time the trigger fires, unless there is a condition that prevents those functions from being executed. In other words, the run function will be executed after alertText both times the trigger is run.

As this is currently written, delaySeconds will push the headmarker target into data.tankLaserCollect, whose .length === 1 when alertText runs (so the early exit is never triggered), and run then clears the array. When the second headmarker line is received, the exact same process will repeat, with a 0.1 second delay.

(NB: We often have separate collector vs. output triggers. It's entirely possible to combine both in a single trigger like you've done, but it just need to take into account the above rules.)

[JLGarber]

(Opinion only, non-binding)

As I noted on the Beat 2 spread changes, unless there's a specific reason to do single-trigger collect-call-clear constructions, I do think we're better off collecting in a separate trigger, and then changing the call trigger to include a 0.1s delay, and an early return within the alertText if data.tankLaserCollect.length is not 2. Obviously this does have the downside of permanently including a 0.1s delay for the call trigger, but that's still going to give users More Than Five seconds to react, and that's above our standard four for AoEs.

[jacob-keller]

Sure I can look into that. I personally found same trigger easy to read but it does seem less common.

[wexxlee]

Sure I can look into that. I personally found same trigger easy to read but it does seem less common.

fwiw, I agree with @JLGarber -- I think it would be cleaner if collect vs. call are handled by separate triggers (with cleanup handled in the run block of the call trigger).

(And, as an aside/for future reference, having separate triggers is almost always easier to debug in raidemulator if there's a problem somewhere. Based on personal experience. 🤣 ).

[JLGarber]

Ah, that is a good point. While we don't precisely want to write for the raid emulator, I do think it's worthwhile to deliberately make it easier where it's basically free. (As another example where I do this almost for free, see

cactbot/ui/raidboss/data/03-hw/trial/sephirot-ex.ts

Line 111 in d375dce

// We make this conditional to avoid constant noise in the raid emulator.

. This construction avoids having this trigger activate every three seconds during the encounter, which is otherwise incredibly noisy in the emulator.)

[wexxlee]

Per note above about storing the entire regex capture group on data, this would then be .includes(data.me).

[wexxlee]

Just suppressSeconds alone seems to be sufficient to prevent the double callout here?

I think the issue may be that, if using suppressSeconds, only one headmarker target will get picked up and will get the 'Tank Cleave' alert, and the other 7 party members (including the tank whose cleave was suppressed) will get 'Avoid Tank Cleave`.

[valarnin]

I think the issue may be that, if using suppressSeconds, only one headmarker target will get picked up and will get the 'Tank Cleave' alert, and the other 7 party members (including the tank whose cleave was suppressed) will get 'Avoid Tank Cleave`.

Hmm, I didn't consider the other tank perspective. But I feel like this is still easier to solve by e.g. two triggers such that the first fires if target is self, and the second fires if the current player isn't a tank?

[jacob-keller]

Just suppressSeconds alone seems to be sufficient to prevent the double callout here?

    {
      id: 'R2S Headmarker Cone Tankbuster',
      type: 'HeadMarker',
      netRegex: { id: headMarkerData.tankLaser, capture: true },
      suppressSeconds: 1,
      response: Responses.tankCleave(),
    },

At least so far in my testing, the problem here is that if you are the tank, and the first log line is for the other tank, the message doesn't say "tank cleave on you", which I find to be bit annoying. I guess its not that big of a deal but I wanted to fix that.

[jacob-keller]

I think the issue may be that, if using suppressSeconds, only one headmarker target will get picked up and will get the 'Tank Cleave' alert, and the other 7 party members (including the tank whose cleave was suppressed) will get 'Avoid Tank Cleave`.

Hmm, I didn't consider the other tank perspective. But I feel like this is still easier to solve by e.g. two triggers such that the first fires if target is self, and the second fires if the current player isn't a tank?

This could work. I guess the only real situation this would break in are cases we don't care about? Like unsync with more than 1 tank, or if a non-tank gets the buster due to mismanaged aggro?

[valarnin]

Something like this is what I was thinking of, but I'm not in the right place right now to make sure it makes sense logically:

    {
      id: 'R2S Headmarker Cone Tankbuster Self',
      type: 'HeadMarker',
      netRegex: { id: headMarkerData.tankLaser, capture: true },
      condition: Conditions.targetIsYou(),
      response: Responses.tankCleave(),
    },
    {
      id: 'R2S Headmarker Cone Tankbuster Non-Tank',
      type: 'HeadMarker',
      netRegex: { id: headMarkerData.tankLaser, capture: true },
      condition: (data, matches) => Conditions.targetIsNotYou()(data, matches) && data.role !== 'tank',
      response: Responses.tankCleave(),
    },

I think this also covers non-tank getting the marker?

[jacob-keller]

Something like this is what I was thinking of, but I'm not in the right place right now to make sure it makes sense logically:

    {
      id: 'R2S Headmarker Cone Tankbuster Self',
      type: 'HeadMarker',
      netRegex: { id: headMarkerData.tankLaser, capture: true },
      condition: Conditions.targetIsYou(),
      response: Responses.tankCleave(),
    },
    {
      id: 'R2S Headmarker Cone Tankbuster Non-Tank',
      type: 'HeadMarker',
      netRegex: { id: headMarkerData.tankLaser, capture: true },
      condition: (data, matches) => Conditions.targetIsNotYou()(data, matches) && data.role !== 'tank',
      response: Responses.tankCleave(),
    },

I think this also covers non-tank getting the marker?

I think the "targetIsNotYou" isn't necessary, because the targetIsYou would work normally to tell that player they got it. In the case that a tank and dps get it, this will end up having the tank who didn't get hit not have a message. I don't think thats a big deal, and two triggers is probably simpler than collecting the log lines. You also need suppress seconds on the 2nd one to avoid duplicate trigger, and you probably would still get a double trigger with avoid + tank cleave if a non-tank gets hit.

I'm looking at the implementation wex suggested as well to see if it looks simple enough too.

[jacob-keller]

This works in the emulator, and saves a lot less data now than it was by using strings. I personally prefer this because it seems more robust than the edge cases of some of the other triggers. I also had a PR which uses a similar approach for handling the multiple spreads +towers during beat two based on the same kind of logic.

[valarnin]

Consider the following situations and their resolutions:

1. MT has one, OT has the other
   a. MT and OT need to point cleaves away, party needs to avoid
   b. MT should get "tank cone on you"
   c. OT should get "tank cone on you"
   d. Rest of party should get "avoid tank cones"
2. MT has one, DPS has the other
   a. MT and DPS player need to point cleaves away, party needs to avoid
   b. MT should get "tank cone on you"
   c. DPS with cone should get "tank cone on you"
   d. OT doesn't have cone, there's no point in telling them to point cone away, they should get "avoid tank cones"
   e. Rest of party should get "avoid tank cones"

[jacob-keller]

Ya, that's the goal of my setup with collecting the targets. I'm not sure you can reliably get that with just two triggers along with:

3. avoid producing multiple callouts

Doubling a callout is bad because the sound overlaps and ends up sounding much louder than normal.

I think multiple triggers without collecting has edge cases where it gets weird. You can handle most of them by suppressSeconds + data.role !== tank, but then if a DPS gets it on accident they'll get one trigger for "tank cone on you" and one for "avoid tank cleave"

[xiashtra]

Consider the following situations and their resolutions:

1. MT has one, OT has the other

a. MT and OT need to point cleaves away, party needs to avoid

b. MT should get "tank cone on you"

c. OT should get "tank cone on you"

d. Rest of party should get "avoid tank cones"

2. MT has one, DPS has the other

a. MT and DPS player need to point cleaves away, party needs to avoid

b. MT should get "tank cone on you"

c. DPS with cone should get "tank cone on you"

d. OT doesn't have cone, there's no point in telling them to point cone away, they should get "avoid tank cones"

e. Rest of party should get "avoid tank cones"

Note that dps getting the buster cones will be a valid situation for BLU eventually (and likely BST at some point as well).

[valarnin]

I think I've voiced my thoughts here sufficiently. If there's not a good way to handle this without collector-ing, that's fine.

I'm taking a day away from actually opening the game or thinking too hard about it, because putting 180 hours into FFXIV over the past two weeks, while also putting 40 hours into my actual full-time job this week, has left me a bit fried.

So assuming this PR makes sense to others, that's enough for me 🙂

[jacob-keller]

Rebased and split to use multiple triggers now!