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

Require both rarity and value #498

Closed
2 tasks done
TomBoj opened this issue Jul 3, 2024 · 6 comments · Fixed by #499
Closed
2 tasks done

Require both rarity and value #498

TomBoj opened this issue Jul 3, 2024 · 6 comments · Fixed by #499
Labels
enhancement Improvement to an already existing notifier Notifier: Loot

Comments

@TomBoj
Copy link

TomBoj commented Jul 3, 2024

Checklist

  • I've searched the issues and pull requests for similar looking suggestions.
  • I've checked the Unreleased section of the changelog for newly added features that sound like my suggestion.

Describe your Suggestion

It would be useful to be able to only send loot notifications if both the rarity and value thresholds are met.
This was a feature in the discord-rare-drop-notifier plugin
image

Apologies if this is already supported, I could not find a way to do it

Reasoning

It would save having to add a lot of items to the Denylist in the case of getting expensive but common loot. Similarly if you have the rarity override on, but get something rare and worthless

@TomBoj TomBoj added the enhancement Improvement to an already existing notifier label Jul 3, 2024
@iProdigy
Copy link
Member

iProdigy commented Jul 3, 2024

With this setting enabled, would it be reasonable to have to allowlist certain rare untradeable drops that may have a store value below the configured value threshold?

@TomBoj
Copy link
Author

TomBoj commented Jul 3, 2024

The current allowlist notifies on items regardless of value already right? So I would imagine it would work the same way with this.

The way I am imagining this would work is when the notifier checks the value is higher than the threshold, it would then just do an additional check against the rarity threshold before sending the message

@TomBoj
Copy link
Author

TomBoj commented Jul 3, 2024

Could it be as simple as something like this?

image

This is very rough and inefficient and I'm not familiar with this code, so I could be way off, just an idea

@TomBoj
Copy link
Author

TomBoj commented Jul 3, 2024

With this setting enabled, would it be reasonable to have to allowlist certain rare untradeable drops that may have a store value below the configured value threshold?

hmm, I guess this may just be a limitation of using this option. Unless there is some way to identify untradeables, and some other setting associated with that

@iProdigy
Copy link
Member

iProdigy commented Jul 4, 2024

Could it be as simple as something like this?

unfortunately it's a bit more complicated (as you can see in the PR)

 

hmm, I guess this may just be a limitation of using this option. Unless there is some way to identify untradeables, and some other setting associated with that

we theoretically could identify items that are not traded on the GE, but I want to avoid settings proliferation (and the code is complicated enough without that customization)

in general, there are likely more rare untradeable items with low store value that shouldn't be dinked (relative to those that should), so the allowlist approach should be fine

@TomBoj
Copy link
Author

TomBoj commented Jul 4, 2024

yeah sounds sensible, thanks for taking a look at this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an already existing notifier Notifier: Loot
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants