feat(loot): add config to require both rarity and value to notify

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Unreleased
 
+- Minor: Allow filtering of loot notifications if both rarity and value thresholds are not met. (#499)
+
 ## 1.10.2
 
 - Minor: Allow RuneLite notifier messages to trigger the Dink chat notifier. (#493)

src/main/java/dinkplugin/DinkPluginConfig.java

@@ -951,14 +951,27 @@ default int lootRarityThreshold() {
         return 0;
     }
 
+    @ConfigItem(
+        keyName = "lootRarityValueIntersection",
+        name = "Require both Rarity and Value",
+        description = "Whether items must exceed <i>both</i> the Min Value AND Rarity thresholds to be notified.<br/>" +
+            "Does not apply to drops where Dink lacks rarity data.<br/>" +
+            "Currently only impacts NPC drops",
+        position = 39,
+        section = lootSection
+    )
+    default boolean lootRarityValueIntersection() {
+        return false;
+    }
+
     @ConfigItem(
         keyName = "lootNotifMessage",
         name = "Notification Message",
         description = "The message to be sent through the webhook.<br/>" +
             "Use %USERNAME% to insert your username<br/>" +
             "Use %LOOT% to insert the loot<br/>" +
             "Use %SOURCE% to show the source of the loot",
-        position = 39,
+        position = 40,
         section = lootSection
     )
     default String lootNotifyMessage() {

src/main/java/dinkplugin/notifiers/LootNotifier.java

@@ -1,6 +1,5 @@
 package dinkplugin.notifiers;
 
-import com.google.common.math.DoubleMath;
 import dinkplugin.message.Embed;
 import dinkplugin.message.NotificationBody;
 import dinkplugin.message.NotificationType;
@@ -29,17 +28,12 @@
 import net.runelite.client.util.QuantityFormatter;
 import net.runelite.http.api.loottracker.LootRecordType;
 import org.apache.commons.lang3.StringUtils;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
-import java.util.Optional;
 import java.util.OptionalDouble;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.regex.Pattern;
@@ -158,42 +152,59 @@ private void handleNotify(Collection<ItemStack> items, String dropper, LootRecor
         long totalStackValue = 0;
         boolean sendMessage = false;
         SerializedItemStack max = null;
-        Collection<Integer> deniedIds = new HashSet<>(reduced.size());
+        RareItemStack rarest = null;
 
+        final double rarityThreshold = config.lootRarityThreshold() > 0 ? 1.0 / config.lootRarityThreshold() : Double.NaN;
         for (ItemStack item : reduced) {
             SerializedItemStack stack = ItemUtils.stackFromItem(itemManager, item.getId(), item.getQuantity());
             long totalPrice = stack.getTotalPrice();
-            boolean worthy = totalPrice >= minValue || matches(itemNameAllowlist, stack.getName());
-            if (!matches(itemNameDenylist, stack.getName())) {
-                if (worthy) {
-                    sendMessage = true;
-                    lootMessage.component(ItemUtils.templateStack(stack, true));
-                    if (icons) embeds.add(Embed.ofImage(ItemUtils.getItemImageUrl(item.getId())));
-                }
-                if (max == null || totalPrice > max.getTotalPrice()) {
-                    max = stack;
+
+            OptionalDouble rarity;
+            if (type == LootRecordType.NPC) {
+                rarity = rarityService.getRarity(dropper, item.getId(), item.getQuantity());
+            } else {
+                rarity = OptionalDouble.empty();
+            }
+
+            boolean shouldSend;
+            if (config.lootRarityValueIntersection() && rarity.isPresent()) {
+                shouldSend = totalPrice >= minValue && MathUtils.lessThanOrEqual(rarity.orElse(1), rarityThreshold);
+            } else {
+                shouldSend = totalPrice >= minValue || MathUtils.lessThanOrEqual(rarity.orElse(1), rarityThreshold);
+            }
+
+            shouldSend |= matches(itemNameAllowlist, stack.getName());
+
+            boolean denied = matches(itemNameDenylist, stack.getName());
+            if (denied) {
+                shouldSend = false;
+            }
+
+            if (shouldSend) {
+                sendMessage = true;
+                lootMessage.component(ItemUtils.templateStack(stack, true));
+                if (icons) embeds.add(Embed.ofImage(ItemUtils.getItemImageUrl(item.getId())));
+            }
+
+            if (max == null || totalPrice > max.getTotalPrice()) {
+                max = stack;
+            }
+
+            if (rarity.isPresent()) {
+                RareItemStack rareStack = RareItemStack.of(stack, rarity.getAsDouble());
+                serializedItems.add(rareStack);
+                if (!denied && (rarest == null || rareStack.getRarity() < rarest.getRarity())) {
+                    rarest = rareStack;
                 }
             } else {
-                deniedIds.add(item.getId());
+                serializedItems.add(stack);
             }
-            serializedItems.add(stack);
             totalStackValue += totalPrice;
         }
 
-        var rarityResult = getItemRarities(type, dropper, serializedItems, deniedIds);
-        Collection<SerializedItemStack> augmentedItems = rarityResult.getKey();
-        RareItemStack rarest = rarityResult.getValue().orElse(null);
-
         Evaluable lootMsg;
         if (!sendMessage) {
-            if (sufficientlyRare(rarest)) {
-                // allow notifications for rare drops, even if below configured min loot value
-                sendMessage = true;
-                lootMsg = Replacements.ofMultiple(" ",
-                    Replacements.ofText("Various items including:"),
-                    ItemUtils.templateStack(rarest, false)
-                );
-            } else if (totalStackValue >= minValue && max != null && "Loot Chest".equalsIgnoreCase(dropper)) {
+            if (totalStackValue >= minValue && max != null && "Loot Chest".equalsIgnoreCase(dropper)) {
                 // Special case: PK loot keys should trigger notification if total value exceeds configured minimum even
                 // if no single item itself would exceed the min value config - github.com/pajlads/DinkPlugin/issues/403
                 sendMessage = true;
@@ -233,57 +244,14 @@ private void handleNotify(Collection<ItemStack> items, String dropper, LootRecor
                 NotificationBody.builder()
                     .text(notifyMessage)
                     .embeds(embeds)
-                    .extra(new LootNotificationData(augmentedItems, dropper, type, kc, rarity))
+                    .extra(new LootNotificationData(serializedItems, dropper, type, kc, rarity))
                     .type(NotificationType.LOOT)
                     .thumbnailUrl(ItemUtils.getItemImageUrl(keyItem.getId()))
                     .build()
             );
         }
     }
 
-    /**
-     * Converts {@link SerializedItemStack} loot to {@link RareItemStack}
-     *
-     * @param type the type of loot source
-     * @param source the name of the loot source
-     * @param reduced the looted items (after {@link ItemUtils#reduceItemStack(Iterable)} was performed)
-     * @param rarestDeniedIds the item IDs that should be ignored when finding the rarest drop
-     * @return the dropped items augmented with rarity information (as available from the NPC drop table), and the single rarest item
-     */
-    @NotNull
-    private Map.Entry<Collection<SerializedItemStack>, Optional<RareItemStack>> getItemRarities(LootRecordType type, String source, Collection<SerializedItemStack> reduced, Collection<Integer> rarestDeniedIds) {
-        if (type != LootRecordType.NPC) {
-            return Map.entry(reduced, Optional.empty());
-        }
-
-        RareItemStack rarest = null;
-        List<SerializedItemStack> transformed = new ArrayList<>(reduced.size());
-        for (SerializedItemStack item : reduced) {
-            final int id = item.getId();
-            OptionalDouble rarity = rarityService.getRarity(source, id, item.getQuantity());
-            if (rarity.isPresent()) {
-                RareItemStack rareItem = RareItemStack.of(item, rarity.getAsDouble());
-                transformed.add(rareItem);
-
-                // check if this stack is the rarest so far
-                if ((rarest == null || rareItem.getRarity() < rarest.getRarity()) && !rarestDeniedIds.contains(id)) {
-                    rarest = rareItem;
-                }
-            } else {
-                transformed.add(item);
-            }
-        }
-        return Map.entry(transformed, Optional.ofNullable(rarest));
-    }
-
-    private boolean sufficientlyRare(@Nullable RareItemStack rarest) {
-        if (rarest == null) return false;
-        int configRareDenominator = config.lootRarityThreshold();
-        if (configRareDenominator <= 0) return false;
-        double rarityThreshold = 1.0 / configRareDenominator;
-        return DoubleMath.fuzzyCompare(rarest.getRarity(), rarityThreshold, MathUtils.EPSILON) <= 0;
-    }
-
     private static boolean matches(Collection<Pattern> regexps, String input) {
         for (Pattern regex : regexps) {
             if (regex.matcher(input).find())

src/main/java/dinkplugin/util/MathUtils.java

@@ -1,5 +1,6 @@
 package dinkplugin.util;
 
+import com.google.common.math.DoubleMath;
 import lombok.experimental.UtilityClass;
 
 import java.math.BigDecimal;
@@ -10,6 +11,10 @@ public class MathUtils {
     public static final double EPSILON = 0.00001;
     private static final int[] FACTORIALS;
 
+    public boolean lessThanOrEqual(double a, double b) {
+        return a < b || DoubleMath.fuzzyEquals(a, b, EPSILON);
+    }
+
     public String formatPercentage(double d, int sigFigs) {
         return BigDecimal.valueOf(d * 100)
             .round(new MathContext(sigFigs))

src/test/java/dinkplugin/notifiers/LootNotifierTest.java

@@ -137,7 +137,7 @@ void testNotifyNpcRarity() {
             NotificationBody.builder()
                 .text(
                     Template.builder()
-                        .template(String.format("%s has looted: Various items including: 1 x {{key}} from {{source}} for %s gp", PLAYER_NAME, value))
+                        .template(String.format("%s has looted: 1 x {{key}} (%s) from {{source}} for %s gp", PLAYER_NAME, value, value))
                         .replacement("{{key}}", Replacements.ofWiki("Larran's key"))
                         .replacement("{{source}}", Replacements.ofWiki(name))
                         .build()
@@ -780,6 +780,83 @@ void testNotifyAmascutExpert() {
         );
     }
 
+    @Test
+    void testNotifyRarityValueIntersectionValue() {
+        // update config mocks
+        when(config.minLootValue()).thenReturn(LARRAN_PRICE);
+        when(config.lootRarityThreshold()).thenReturn(100);
+        when(config.lootRarityValueIntersection()).thenReturn(true);
+
+        // prepare mocks
+        NPC npc = mock(NPC.class);
+        String name = "Ice spider";
+        when(npc.getName()).thenReturn(name);
+
+        // fire event
+        double rarity = 1.0 / 208;
+        NpcLootReceived event = new NpcLootReceived(npc, List.of(new ItemStack(ItemID.LARRANS_KEY, 1)));
+        plugin.onNpcLootReceived(event);
+
+        // verify notification message
+        String value = QuantityFormatter.quantityToStackSize(LARRAN_PRICE);
+        verify(messageHandler).createMessage(
+            PRIMARY_WEBHOOK_URL,
+            false,
+            NotificationBody.builder()
+                .text(
+                    Template.builder()
+                        .template(String.format("%s has looted: 1 x {{key}} (%s) from {{source}} for %s gp", PLAYER_NAME, value, value))
+                        .replacement("{{key}}", Replacements.ofWiki("Larran's key"))
+                        .replacement("{{source}}", Replacements.ofWiki(name))
+                        .build()
+                )
+                .extra(new LootNotificationData(List.of(new RareItemStack(ItemID.LARRANS_KEY, 1, LARRAN_PRICE, "Larran's key", rarity)), name, LootRecordType.NPC, 1, rarity))
+                .type(NotificationType.LOOT)
+                .thumbnailUrl(ItemUtils.getItemImageUrl(ItemID.LARRANS_KEY))
+                .build()
+        );
+    }
+
+    @Test
+    void testIgnoreRarityValueIntersectionRarityTooLow() {
+        // update config mocks
+        when(config.minLootValue()).thenReturn(LARRAN_PRICE - 1);
+        when(config.lootRarityThreshold()).thenReturn(1000);
+        when(config.lootRarityValueIntersection()).thenReturn(true);
+
+        // prepare mocks
+        NPC npc = mock(NPC.class);
+        String name = "Ice spider";
+        when(npc.getName()).thenReturn(name);
+
+        // fire event
+        NpcLootReceived event = new NpcLootReceived(npc, List.of(new ItemStack(ItemID.LARRANS_KEY, 1)));
+        plugin.onNpcLootReceived(event);
+
+        // verify notification message doesn't fire
+        verify(messageHandler, never()).createMessage(any(), anyBoolean(), any());
+    }
+
+    @Test
+    void testIgnoreRarityValueIntersectionValueTooLow() {
+        // update config mocks
+        when(config.minLootValue()).thenReturn(LARRAN_PRICE + 1);
+        when(config.lootRarityThreshold()).thenReturn(100);
+        when(config.lootRarityValueIntersection()).thenReturn(true);
+
+        // prepare mocks
+        NPC npc = mock(NPC.class);
+        String name = "Ice spider";
+        when(npc.getName()).thenReturn(name);
+
+        // fire event
+        NpcLootReceived event = new NpcLootReceived(npc, List.of(new ItemStack(ItemID.LARRANS_KEY, 1)));
+        plugin.onNpcLootReceived(event);
+
+        // verify notification message doesn't fire
+        verify(messageHandler, never()).createMessage(any(), anyBoolean(), any());
+    }
+
     @Test
     void testDisabled() {
         // disable notifier