Repository: pajlads/DinkPlugin
Pull request #314 (2023-09-11 09:01:00)

feat(pet): distinguish lost pets via collection log messages
Follow-up from https://github.com/pajlads/DinkPlugin/pull/303#issuecomment-1712821862

`duplicate` means *currently* already owned

Previously:

* `would have been followed` => duplicate => previouslyOwned is also true
* `being followed` => not duplicate => unclear whether previouslyOwned (because pet could have died)

New scenarios that are handled:

* `being followed` + collection log notification => not duplicate and not previouslyOwned
* `being followed` + no collection log notification (while collection log chat setting is enabled) => not duplicate, but has been previouslyOwned



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 438d2a3..4edff9d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## Unreleased
 
+- Minor: Indicate in pet notification metadata when a pet was previously owned but lost. (#314)
 - Minor: Support wildcards in loot item name filters. (#312)
 
 ## 1.6.3
diff --git a/src/main/java/dinkplugin/notifiers/PetNotifier.java b/src/main/java/dinkplugin/notifiers/PetNotifier.java
index 38bbe5a..343b159 100644
--- a/src/main/java/dinkplugin/notifiers/PetNotifier.java
+++ b/src/main/java/dinkplugin/notifiers/PetNotifier.java
@@ -11,6 +11,8 @@ import dinkplugin.util.ItemUtils;
 import dinkplugin.util.Utils;
 import lombok.AccessLevel;
 import lombok.Setter;
+import lombok.Value;
+import net.runelite.api.Varbits;
 import net.runelite.api.annotations.Varbit;
 import org.apache.commons.lang3.StringUtils;
 import org.jetbrains.annotations.VisibleForTesting;
@@ -75,6 +77,8 @@ public class PetNotifier extends BaseNotifier {
 
     private volatile boolean backpack = false;
 
+    private volatile boolean collection = false;
+
     @Override
     public boolean isEnabled() {
         return config.notifyPet() && super.isEnabled();
@@ -94,10 +98,15 @@ public class PetNotifier extends BaseNotifier {
                     this.duplicate = chatMessage.contains("would have been");
                     this.backpack = chatMessage.contains(" backpack");
                 }
-            } else if (PRIMED_NAME.equals(petName)) {
+            } else if (PRIMED_NAME.equals(petName) || !collection) {
                 parseItemFromGameMessage(chatMessage)
-                    .filter(item -> item.startsWith("Pet ") || PET_NAMES.contains(Utils.ucFirst(item)))
-                    .ifPresent(this::setPetName);
+                    .filter(item -> item.getItemName().startsWith("Pet ") || PET_NAMES.contains(Utils.ucFirst(item.getItemName())))
+                    .ifPresent(parseResult -> {
+                        setPetName(parseResult.getItemName());
+                        if (parseResult.isCollectionLog()) {
+                            this.collection = true;
+                        }
+                    });
             } else {
                 // ignore; we already know the pet name
             }
@@ -133,14 +142,25 @@ public class PetNotifier extends BaseNotifier {
         this.milestone = null;
         this.duplicate = false;
         this.backpack = false;
+        this.collection = false;
         this.ticksWaited.set(0);
     }
 
     private void handleNotify() {
+        Boolean previouslyOwned;
+        if (duplicate) {
+            previouslyOwned = true;
+        } else if (client.getVarbitValue(Varbits.COLLECTION_LOG_NOTIFICATION) % 2 == 1) {
+            // when collection log chat notification is enabled, presence or absence of notification indicates ownership history
+            previouslyOwned = !collection;
+        } else {
+            previouslyOwned = null;
+        }
+
         String gameMessage;
         if (backpack) {
             gameMessage = "feels something weird sneaking into their backpack";
-        } else if (duplicate) {
+        } else if (previouslyOwned != null && previouslyOwned) {
             gameMessage = "has a funny feeling like they would have been followed...";
         } else {
             gameMessage = "has a funny feeling like they're being followed";
@@ -160,7 +180,7 @@ public class PetNotifier extends BaseNotifier {
             .map(ItemUtils::getItemImageUrl)
             .orElse(null);
 
-        PetNotificationData extra = new PetNotificationData(StringUtils.defaultIfEmpty(petName, null), milestone, duplicate);
+        PetNotificationData extra = new PetNotificationData(StringUtils.defaultIfEmpty(petName, null), milestone, duplicate, previouslyOwned);
 
         createMessage(config.petSendImage(), NotificationBody.builder()
             .extra(extra)
@@ -172,20 +192,26 @@ public class PetNotifier extends BaseNotifier {
         reset();
     }
 
-    private static Optional<String> parseItemFromGameMessage(String message) {
+    private static Optional<ParseResult> parseItemFromGameMessage(String message) {
         Matcher untradeableMatcher = UNTRADEABLE_REGEX.matcher(message);
         if (untradeableMatcher.find()) {
-            return Optional.of(untradeableMatcher.group(1));
+            return Optional.of(new ParseResult(untradeableMatcher.group(1), false));
         }
 
         Matcher collectionMatcher = COLLECTION_LOG_REGEX.matcher(message);
         if (collectionMatcher.find()) {
-            return Optional.of(collectionMatcher.group("itemName"));
+            return Optional.of(new ParseResult(collectionMatcher.group("itemName"), true));
         }
 
         return Optional.empty();
     }
 
+    @Value
+    private static class ParseResult {
+        String itemName;
+        boolean collectionLog;
+    }
+
     static {
         // Note: We don't explicitly list out names that have the "Pet " prefix
         // since they are matched by filter(item -> item.startsWith("Pet ")) above
diff --git a/src/main/java/dinkplugin/notifiers/data/PetNotificationData.java b/src/main/java/dinkplugin/notifiers/data/PetNotificationData.java
index af0df87..93a1ed0 100644
--- a/src/main/java/dinkplugin/notifiers/data/PetNotificationData.java
+++ b/src/main/java/dinkplugin/notifiers/data/PetNotificationData.java
@@ -4,16 +4,43 @@ import dinkplugin.message.Field;
 import lombok.EqualsAndHashCode;
 import lombok.Value;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.List;
 
 @Value
 @EqualsAndHashCode(callSuper = false)
 public class PetNotificationData extends NotificationData {
+
+    /**
+     * The name of the pet.
+     * <p>
+     * This field is null when the name cannot be read from untradeable drop, collection log, or clan chat notification.
+     */
+    @Nullable
     String petName;
+
+    /**
+     * The milestone (e.g., skill XP, boss KC) at which this pet was acquiried.
+     * <p>
+     * This field relies upon clan chat notifications.
+     */
+    @Nullable
     String milestone;
+
+    /**
+     * Whether this pet is (currently) already owned.
+     */
     boolean duplicate;
 
+    /**
+     * Whether the pet is already owned or was previously owned.
+     * <p>
+     * This field relies upon collection log chat notifications.
+     */
+    @Nullable
+    Boolean previouslyOwned;
+
     @Override
     public List<Field> getFields() {
         if (petName == null || petName.isEmpty())
@@ -21,10 +48,17 @@ public class PetNotificationData extends NotificationData {
 
         List<Field> fields = new ArrayList<>(3);
         fields.add(new Field("Name", Field.formatBlock("", petName)));
-        if (duplicate)
-            fields.add(new Field("Status", Field.formatBlock("", "Already owned")));
+        String status = getStatus();
+        if (status != null)
+            fields.add(new Field("Status", Field.formatBlock("", status)));
         if (milestone != null)
             fields.add(new Field("Milestone", Field.formatBlock("", milestone)));
         return fields;
     }
+
+    private String getStatus() {
+        if (duplicate) return "Already owned";
+        if (previouslyOwned == null) return null;
+        return previouslyOwned ? "Previously owned" : "New!";
+    }
 }


Test patch:
diff --git a/src/test/java/dinkplugin/notifiers/PetNotifierTest.java b/src/test/java/dinkplugin/notifiers/PetNotifierTest.java
index 726d76b..081adba 100644
--- a/src/test/java/dinkplugin/notifiers/PetNotifierTest.java
+++ b/src/test/java/dinkplugin/notifiers/PetNotifierTest.java
@@ -8,6 +8,7 @@ import dinkplugin.notifiers.data.PetNotificationData;
 import dinkplugin.util.ItemSearcher;
 import dinkplugin.util.ItemUtils;
 import net.runelite.api.ItemID;
+import net.runelite.api.Varbits;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.mockito.InjectMocks;
@@ -57,7 +58,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(null, null, false))
+                .extra(new PetNotificationData(null, null, false, null))
                 .text(buildTemplate(PLAYER_NAME + " feels something weird sneaking into their backpack"))
                 .type(NotificationType.PET)
                 .build()
@@ -78,7 +79,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(null, null, true))
+                .extra(new PetNotificationData(null, null, true, true))
                 .text(buildTemplate(PLAYER_NAME + " has a funny feeling like they would have been followed..."))
                 .type(NotificationType.PET)
                 .build()
@@ -92,6 +93,7 @@ class PetNotifierTest extends MockedNotifierTest {
 
         // prepare mocks
         when(itemSearcher.findItemId("Tzrek-jad")).thenReturn(itemId);
+        when(client.getVarbitValue(Varbits.COLLECTION_LOG_NOTIFICATION)).thenReturn(1);
 
         // send fake message
         notifier.onChatMessage("You have a funny feeling like you're being followed.");
@@ -103,7 +105,34 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(petName, null, false))
+                .extra(new PetNotificationData(petName, null, false, false))
+                .text(buildTemplate(PLAYER_NAME + " got a pet"))
+                .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
+                .type(NotificationType.PET)
+                .build()
+        );
+    }
+
+    @Test
+    void testNotifyLostExistingCollection() {
+        String petName = "TzRek-Jad";
+        int itemId = ItemID.TZREKJAD;
+
+        // prepare mocks
+        when(itemSearcher.findItemId("Tzrek-jad")).thenReturn(itemId);
+        when(client.getVarbitValue(Varbits.COLLECTION_LOG_NOTIFICATION)).thenReturn(1);
+
+        // send fake message
+        notifier.onChatMessage("You have a funny feeling like you're being followed.");
+        notifier.onChatMessage("Untradeable drop: " + petName);
+        IntStream.rangeClosed(0, MAX_TICKS_WAIT).forEach(i -> notifier.onTick());
+
+        // verify handled
+        verify(messageHandler).createMessage(
+            PRIMARY_WEBHOOK_URL,
+            false,
+            NotificationBody.builder()
+                .extra(new PetNotificationData(petName, null, false, true))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
                 .type(NotificationType.PET)
@@ -129,7 +158,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(petName, null, false))
+                .extra(new PetNotificationData(petName, null, false, null))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
                 .type(NotificationType.PET)
@@ -155,7 +184,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(petName, null, true))
+                .extra(new PetNotificationData(petName, null, true, true))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
                 .type(NotificationType.PET)
@@ -178,7 +207,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(null, null, false))
+                .extra(new PetNotificationData(null, null, false, null))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
                 .type(NotificationType.PET)
@@ -217,7 +246,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(petName, "50 killcount", false))
+                .extra(new PetNotificationData(petName, "50 killcount", false, null))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
                 .type(NotificationType.PET)
@@ -255,7 +284,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(petName, "50 killcount", false))
+                .extra(new PetNotificationData(petName, "50 killcount", false, null))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .thumbnailUrl(ItemUtils.getItemImageUrl(itemId))
                 .type(NotificationType.PET)
@@ -280,7 +309,7 @@ class PetNotifierTest extends MockedNotifierTest {
             "https://example.com",
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(null, null, false))
+                .extra(new PetNotificationData(null, null, false, null))
                 .text(buildTemplate(PLAYER_NAME + " has a funny feeling like they're being followed"))
                 .type(NotificationType.PET)
                 .build()
@@ -325,7 +354,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(null, null, false))
+                .extra(new PetNotificationData(null, null, false, null))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .type(NotificationType.PET)
                 .build()
@@ -348,7 +377,7 @@ class PetNotifierTest extends MockedNotifierTest {
             PRIMARY_WEBHOOK_URL,
             false,
             NotificationBody.builder()
-                .extra(new PetNotificationData(null, null, false))
+                .extra(new PetNotificationData(null, null, false, null))
                 .text(buildTemplate(PLAYER_NAME + " got a pet"))
                 .type(NotificationType.PET)
                 .build());