LegoFigure11 / RaidCrawler

Raid Viewer for Pokémon Scarlet and Violet
GNU General Public License v3.0
168 stars 59 forks source link

If multiple filters meet passing criteria, webhooks send multiple messages #99

Closed CardBreaker closed 1 year ago

CardBreaker commented 1 year ago

If I have multiple filters that meet the criteria on the same raid, multiple messages will be sent to the webhook. For example, I can have 1 filter that generally flags all shinies and another filter that's specifically looking for a shiny Numel. If both conditions are met, I get 2 messages sent to Discord for the same raid but with different stop conditions.

In my particular case, I have a bunch of individual filters for shiny pokemon I still need as well as a general filter to catch any shiny. I will typically run my specific filters overnight and simply toggle the general all shinies filter during the day since it would be tedious to turn off all my specific filters.

I think it would make more sense to batch up any passing filter conditions for a raid and then send the webhook message with the list of matching filters in a single message.

jnovack commented 1 year ago

Unlikely to be implemented in that fashion. Right now, we're iterating through the raid set and if a match occurs, it'll blindly fire and move on.

Buffering would require saving the information and using it at a later stage. It would need a rewrite of the matching logic to (as an example, compound a string) and then the SendNotification() function to not accept the filter as an object and just accept text.

Granted, it's a trivial rewrite, but two notifications is not the end of the world; and if it is, we welcome your PR.

architdate commented 1 year ago

One way to handle this would be to iterate over the active raids instead of active filters and then check which filters match for that raid and then send the notification accordingly. I'll keep this open, but ill tag it as an enhancement on low priority for now

jnovack commented 1 year ago

I'll keep this open, but ill tag it as an enhancement on low priority for now

Heh, got inspired?

architdate commented 1 year ago

I'll keep this open, but ill tag it as an enhancement on low priority for now

Heh, got inspired?

Was just bored lol