fix: Fix some bugs in dialogue parsing (in ChatHandler) [ci skip] (#2357

)

* fix: Use ticks for measuring slow packet difference, fix tick scheduler manager having an inconsistent order with the tick events

The logic was correct, but using `System.currentTimeMillis` is too "inaccurate" for tick timing in many cases. Since the packet difference is actually measured in ticks, we might as well use that here as well.
I've bumped the timeout ticks from 10 to 20, since the previous value was too low for some cases.

This also fixes the tick scheduler manager having an inconsistent order with the tick events, which also caused the same issue.
The new order of operations is:
1. TickAlwaysEvent
2. Run the tick scheduler (highest TickAlwaysEvent priority)
3. TickEvent

This fixes the issue where tick events could schedule a task for the next tick, but the tick scheduler would run it at the same tick event, since the tick always event was run after the tick event.

* fix: Fix chat handler "missing" a tick when collecting dialogue

This bug resulted in the handler not collecting chat lines correctly rarely, while in dialogue, dumping a lot of duplicated background text.

* fix: Fix the "last" bad tick logic in ChatHandler

common/src/main/java/com/wynntils/core/mod/TickSchedulerManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © Wynntils 2022-2023.
+ * Copyright © Wynntils 2022-2024.
  * This file is released under LGPLv3. See LICENSE for full license details.
  */
 package com.wynntils.core.mod;
@@ -10,6 +10,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import net.minecraftforge.eventbus.api.EventPriority;
 import net.minecraftforge.eventbus.api.SubscribeEvent;
 
 public final class TickSchedulerManager extends Manager {
@@ -27,7 +28,10 @@ public void scheduleNextTick(Runnable runnable) {
         tasks.put(runnable, 0);
     }
 
-    @SubscribeEvent
+    // The priority is set to HIGHEST to ensure that the tasks are run
+    // before any other tick event listeners could schedule new tasks
+    // making it run in the same tick
+    @SubscribeEvent(priority = EventPriority.HIGHEST)
     public void onTick(TickAlwaysEvent e) {
         Iterator<Map.Entry<Runnable, Integer>> it = tasks.entrySet().iterator();
         while (it.hasNext()) {

common/src/main/java/com/wynntils/handlers/chat/ChatHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © Wynntils 2022-2023.
+ * Copyright © Wynntils 2022-2024.
  * This file is released under LGPLv3. See LICENSE for full license details.
  */
 package com.wynntils.handlers.chat;
@@ -85,7 +85,7 @@ public final class ChatHandler extends Handler {
             Pattern.compile("^ *§[47cf](Select|CLICK) §[47cf]an option (§[47])?to continue$");
 
     private static final Pattern EMPTY_LINE_PATTERN = Pattern.compile("^\\s*(§r|À+)?\\s*$");
-    private static final long SLOWDOWN_PACKET_DIFF_MS = 500;
+    private static final long SLOWDOWN_PACKET_TICK_DELAY = 20;
     private static final int CHAT_SCREEN_TICK_DELAY = 1;
 
     private final Set<Feature> dialogExtractionDependents = new HashSet<>();
@@ -121,8 +121,10 @@ public void onConnectionChange(WynncraftConnectionEvent event) {
     public void onTick(TickEvent event) {
         if (collectedLines.isEmpty()) return;
 
+        // Tick event runs after the chat packets, with the same tick number
+        // as the chat packets. This means we can allow equality here.
         long ticks = McUtils.mc().level.getGameTime();
-        if (ticks > chatScreenTicks + CHAT_SCREEN_TICK_DELAY) {
+        if (ticks >= chatScreenTicks + CHAT_SCREEN_TICK_DELAY) {
             // Send the collected screen lines
             processCollectedChatScreen();
         }
@@ -152,7 +154,7 @@ public void onStatusEffectUpdate(MobEffectEvent.Update event) {
 
                 postNpcDialogue(dialogue, delayedType, true);
             } else {
-                lastSlowdownApplied = System.currentTimeMillis();
+                lastSlowdownApplied = McUtils.mc().level.getGameTime();
             }
         }
     }
@@ -192,7 +194,10 @@ private void handleWithSeparation(ChatPacketReceivedEvent event) {
                 || (styledText.isEmpty() && (currentTicks <= chatScreenTicks + CHAT_SCREEN_TICK_DELAY))) {
             // This is a "chat screen"
             List<Component> lines = ComponentUtils.splitComponentInLines(message);
-            if (currentTicks < chatScreenTicks + CHAT_SCREEN_TICK_DELAY) {
+
+            // Allow ticks to be equal, since we we want to
+            // collect all lines in the this tick and the next one
+            if (currentTicks <= chatScreenTicks + CHAT_SCREEN_TICK_DELAY) {
                 // We are collecting lines, so add to the current collection
                 collectedLines.addAll(lines);
             } else {
@@ -337,7 +342,7 @@ private void handleScreenNpcDialog(List<Component> dialog, boolean isSelection)
 
         NpcDialogueType type = isSelection ? NpcDialogueType.SELECTION : NpcDialogueType.NORMAL;
 
-        if ((System.currentTimeMillis() <= lastSlowdownApplied + SLOWDOWN_PACKET_DIFF_MS)) {
+        if (McUtils.mc().level.getGameTime() <= lastSlowdownApplied + SLOWDOWN_PACKET_TICK_DELAY) {
             // This is a "protected" dialogue if we have gotten slowdown effect just prior to the chat message
             // This is the normal case
             postNpcDialogue(dialog, type, true);

common/src/main/java/com/wynntils/mc/mixin/MinecraftMixin.java

@@ -1,5 +1,5 @@
 /*
- * Copyright © Wynntils 2021-2023.
+ * Copyright © Wynntils 2021-2024.
  * This file is released under LGPLv3. See LICENSE for full license details.
  */
 package com.wynntils.mc.mixin;
@@ -45,8 +45,12 @@ private void setScreenPre(Screen screen, CallbackInfo ci, @Share("oldScreen") Lo
 
     @Inject(method = "tick()V", at = @At("HEAD"))
     private void tickPost(CallbackInfo ci) {
-        MixinHelper.post(new TickEvent());
+        // TickAlwaysEvent is posted before TickEvent to ensure
+        // that the tasks in TickSchedulerManager are run before
+        // any other tick event listeners could schedule new tasks
+        // making it run in the same tick
         MixinHelper.postAlways(new TickAlwaysEvent());
+        MixinHelper.post(new TickEvent());
     }
 
     @Inject(method = "resizeDisplay()V", at = @At("RETURN"))