fix: avoid undercounting completed diaries after teleports (#373)

* fix: avoid undercounting completed diaries after teleports

* chore: update changelog

* refactor: ignore LOADING state across all notifiers

* chore: update mock tests

* chore: prevent collection data reset when hopping

* chore(collection): simplify conditional

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Unreleased
 
+- Bugfix: Avoid undercounting diary completions for notifications that occur shortly after a teleport. (#373)
+
 ## 1.7.2
 
 - Dev: Utilize runelite event for unsired loot instead of custom widget handler (#375).

src/main/java/dinkplugin/DinkPlugin.java

@@ -22,6 +22,7 @@
 import dinkplugin.util.Utils;
 import lombok.extern.slf4j.Slf4j;
 import net.runelite.api.ChatMessageType;
+import net.runelite.api.GameState;
 import net.runelite.api.events.AccountHashChanged;
 import net.runelite.api.events.ActorDeath;
 import net.runelite.api.events.ChatMessage;
@@ -52,6 +53,7 @@
 
 import javax.inject.Inject;
 import java.awt.Color;
+import java.util.concurrent.atomic.AtomicReference;
 
 @Slf4j
 @PluginDescriptor(
@@ -86,6 +88,8 @@ public class DinkPlugin extends Plugin {
     private @Inject LeaguesNotifier leaguesNotifier;
     private @Inject MetaNotifier metaNotifier;
 
+    private final AtomicReference<GameState> gameState = new AtomicReference<>();
+
     @Override
     protected void startUp() {
         log.debug("Started up Dink");
@@ -98,6 +102,7 @@ protected void startUp() {
     protected void shutDown() {
         log.debug("Shutting down Dink");
         this.resetNotifiers();
+        gameState.lazySet(null);
     }
 
     void resetNotifiers() {
@@ -145,12 +150,24 @@ public void onUsernameChanged(UsernameChanged usernameChanged) {
 
     @Subscribe
     public void onGameStateChanged(GameStateChanged gameStateChanged) {
-        settingsManager.onGameState(gameStateChanged);
-        collectionNotifier.onGameState(gameStateChanged);
+        GameState newState = gameStateChanged.getGameState();
+        if (newState == GameState.LOADING) {
+            // an intermediate state that is irrelevant for our notifiers; ignore
+            return;
+        }
+
+        GameState previousState = gameState.getAndSet(newState);
+        if (previousState == newState) {
+            // no real change occurred (just momentarily went through LOADING); ignore
+            return;
+        }
+
+        settingsManager.onGameState(previousState, newState);
+        collectionNotifier.onGameState(newState);
         levelNotifier.onGameStateChanged(gameStateChanged);
         diaryNotifier.onGameState(gameStateChanged);
         grandExchangeNotifier.onGameStateChange(gameStateChanged);
-        metaNotifier.onGameState(gameStateChanged);
+        metaNotifier.onGameState(previousState, newState);
     }
 
     @Subscribe

src/main/java/dinkplugin/SettingsManager.java

@@ -200,14 +200,19 @@ void onConfigChanged(ConfigChanged event) {
         }
     }
 
-    void onGameState(GameStateChanged event) {
-        if (event.getGameState() != GameState.LOGGED_IN) {
+    void onGameState(GameState oldState, GameState newState) {
+        if (newState != GameState.LOGGED_IN) {
             justLoggedIn.set(false);
             return;
         } else {
             justLoggedIn.set(true);
         }
 
+        if (oldState == GameState.HOPPING) {
+            // avoid repeated warnings after login
+            return;
+        }
+
         // Since varbit values default to zero and no VarbitChanged occurs if the
         // newly received value is equal to the existing value, we must manually
         // check those where 0 is an invalid value deserving of a warning.

src/main/java/dinkplugin/notifiers/CollectionNotifier.java

@@ -15,7 +15,6 @@
 import net.runelite.api.VarClientStr;
 import net.runelite.api.Varbits;
 import net.runelite.api.annotations.Varp;
-import net.runelite.api.events.GameStateChanged;
 import net.runelite.api.events.VarbitChanged;
 import net.runelite.client.callback.ClientThread;
 import net.runelite.client.game.ItemManager;
@@ -73,8 +72,8 @@ public void reset() {
         this.popupStarted.set(false);
     }
 
-    public void onGameState(GameStateChanged event) {
-        if (event.getGameState() != GameState.LOGGED_IN)
+    public void onGameState(GameState newState) {
+        if (newState != GameState.HOPPING && newState != GameState.LOGGED_IN)
             this.reset();
     }
 

src/main/java/dinkplugin/notifiers/MetaNotifier.java

@@ -42,7 +42,6 @@ public class MetaNotifier extends BaseNotifier {
     static final @VisibleForTesting int INIT_TICKS = 10; // 6 seconds after login
 
     private final AtomicInteger loginTicks = new AtomicInteger(-1);
-    private final AtomicReference<GameState> gameState = new AtomicReference<>();
 
     @Inject
     private ClientThread clientThread;
@@ -63,13 +62,8 @@ protected String getWebhookUrl() {
         return config.metadataWebhook();
     }
 
-    public void onGameState(GameStateChanged event) {
-        GameState newState = event.getGameState();
-        if (newState == GameState.LOADING) {
-            // ignore this intermediate state
-            return;
-        }
-        GameState oldState = gameState.getAndSet(newState);
+    public void onGameState(GameState oldState, GameState newState) {
+        // inspect oldState because we don't want a notification on each world hop
         if (oldState == GameState.LOGGING_IN && newState == GameState.LOGGED_IN) {
             loginTicks.set(INIT_TICKS);
         }

src/test/java/dinkplugin/notifiers/MetaNotifierTest.java

@@ -12,7 +12,6 @@
 import net.runelite.api.Skill;
 import net.runelite.api.VarPlayer;
 import net.runelite.api.Varbits;
-import net.runelite.api.events.GameStateChanged;
 import net.runelite.client.config.RuneLiteConfig;
 import org.apache.commons.lang3.StringUtils;
 import org.junit.jupiter.api.Assertions;
@@ -87,9 +86,7 @@ protected void setUp() {
     @Test
     void testNotify() {
         // fire event
-        notifier.onGameState(event(GameState.LOGGING_IN));
-        notifier.onGameState(event(GameState.LOADING));
-        notifier.onGameState(event(GameState.LOGGED_IN));
+        notifier.onGameState(GameState.LOGGING_IN, GameState.LOGGED_IN);
         IntStream.rangeClosed(0, MetaNotifier.INIT_TICKS).forEach(i -> notifier.onTick());
 
         // verify handled
@@ -127,9 +124,7 @@ void testNotifyWithoutCollection() {
         when(client.getVarpValue(CollectionNotifier.TOTAL_VARP)).thenReturn(0);
 
         // fire events
-        notifier.onGameState(event(GameState.LOGGING_IN));
-        notifier.onGameState(event(GameState.LOADING));
-        notifier.onGameState(event(GameState.LOGGED_IN));
+        notifier.onGameState(GameState.LOGGING_IN, GameState.LOGGED_IN);
         IntStream.rangeClosed(0, MetaNotifier.INIT_TICKS).forEach(i -> notifier.onTick());
 
         // verify handled
@@ -166,9 +161,7 @@ void testDisabled() {
         when(config.metadataWebhook()).thenReturn("");
 
         // fire event
-        GameStateChanged event = new GameStateChanged();
-        event.setGameState(GameState.LOGGED_IN);
-        notifier.onGameState(event);
+        notifier.onGameState(GameState.LOGGING_IN, GameState.LOGGED_IN);
         IntStream.rangeClosed(0, MetaNotifier.INIT_TICKS).forEach(i -> notifier.onTick());
 
         // ensure no notification
@@ -192,10 +185,4 @@ void testPetDeserialization() {
         );
         Assertions.assertEquals(expected, notifier.getPets());
     }
-
-    private static GameStateChanged event(GameState state) {
-        GameStateChanged event = new GameStateChanged();
-        event.setGameState(state);
-        return event;
-    }
 }