OverlayPlugin / cactbot

FFXIV TypeScript Raiding Overlay
Apache License 2.0
85 stars 34 forks source link

raidboss: avoid multiple callouts during r2s cone tankbuster #339

Closed jacob-keller closed 3 weeks ago

jacob-keller commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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(),
    },

image

wexxlee commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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(),
    },

image

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

image 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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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 commented 3 weeks ago

Rebased and split to use multiple triggers now!