fix(world-hop): wait for the world switcher to load before hopping

runelite-client/src/main/java/net/runelite/client/plugins/microbot/Microbot.java

@@ -171,6 +171,8 @@ public class Microbot {
     private static TooltipManager tooltipManager;
     private static ScheduledFuture<?> xpSchedulorFuture;
     private static net.runelite.api.World quickHopTargetWorld;
+    private static final int WORLD_SWITCHER_LOAD_TIMEOUT_MS = 5000;
+    private static final int WORLD_SWITCHER_REOPEN_INTERVAL_MS = 600;
     /**
      * PouchScript is injected in the main MicrobotPlugin as it's being used in multiple scripts
      */
@@ -374,7 +376,8 @@ public static boolean hopToWorld(int worldNumber) {
             log.error("Can't hop world, already trying to hop");
             return false;
         }
-        boolean hopIssued = Microbot.getClientThread().runOnClientThreadOptional(() -> {
+        final net.runelite.api.World[] target = new net.runelite.api.World[1];
+        boolean prepared = Microbot.getClientThread().runOnClientThreadOptional(() -> {
             if (Rs2Combat.inCombat()) {
                 log.error("Player is in combat, cannot hop worlds");
                 return false;
@@ -400,19 +403,29 @@ public static boolean hopToWorld(int worldNumber) {
             if (rsWorld == null) return false;
 
             quickHopTargetWorld = rsWorld;
+            target[0] = rsWorld;
             rsWorld.setActivity(newWorld.getActivity());
             rsWorld.setAddress(newWorld.getAddress());
             rsWorld.setId(newWorld.getId());
             rsWorld.setPlayerCount(newWorld.getPlayers());
             rsWorld.setLocation(newWorld.getLocation());
             rsWorld.setTypes(WorldUtil.toWorldTypes(newWorld.getTypes()));
 
-            Microbot.getClient().openWorldHopper();
-            Microbot.getClient().hopToWorld(rsWorld);
-            quickHopTargetWorld = null;
             return true;
         }).orElse(false);
-        if (!hopIssued) {
+        if (target[0] != null) {
+            boolean hopIssued;
+            try {
+                hopIssued = prepared && hopViaWorldSwitcher(target[0], WORLD_SWITCHER_LOAD_TIMEOUT_MS);
+            } finally {
+                quickHopTargetWorld = null;
+            }
+            if (prepared && !hopIssued) {
+                log.warn("World switcher did not load in time, could not hop to world {}", worldNumber);
+                return false;
+            }
+        }
+        if (!prepared) {
             log.error("Failed to hop to world {}", worldNumber);
             return false;
         }
@@ -421,7 +434,6 @@ public static boolean hopToWorld(int worldNumber) {
         // wait inside it is a no-op and the success check fires before the server has
         // even processed the request. That's the source of "Failed to hop" spam.
         if (Microbot.getClient().getWorld() != worldNumber) {
-            sleep(600);
             sleepUntil(() -> Microbot.isHopping()
                             || Microbot.getClient().getWorld() == worldNumber
                             || Rs2Widget.getWidget(193, 0) != null, 5000);
@@ -452,6 +464,32 @@ public static boolean hopToWorld(int worldNumber) {
         return success;
     }
 
+    static boolean hopViaWorldSwitcher(net.runelite.api.World rsWorld, int loadTimeoutMs) {
+        if (!isWorldSwitcherLoaded()
+                && !sleepUntil(Microbot::isWorldSwitcherLoaded, Microbot::openWorldSwitcher, loadTimeoutMs, WORLD_SWITCHER_REOPEN_INTERVAL_MS)) {
+            return false;
+        }
+        return Microbot.getClientThread().runOnClientThreadOptional(() -> {
+            if (Microbot.getClient().getWidget(InterfaceID.Worldswitcher.BUTTONS) == null) {
+                return false;
+            }
+            Microbot.getClient().hopToWorld(rsWorld);
+            return true;
+        }).orElse(false);
+    }
+
+    private static void openWorldSwitcher() {
+        Microbot.getClientThread().runOnClientThreadOptional(() -> {
+            Microbot.getClient().openWorldHopper();
+            return true;
+        });
+    }
+
+    private static boolean isWorldSwitcherLoaded() {
+        return Microbot.getClientThread().runOnClientThreadOptional(
+                () -> Microbot.getClient().getWidget(InterfaceID.Worldswitcher.BUTTONS) != null).orElse(false);
+    }
+
     public static void showMessage(String message) {
         try {
             Runnable messageRunnable = () ->

runelite-client/src/test/java/net/runelite/client/plugins/microbot/MicrobotWorldHopTest.java

@@ -0,0 +1,114 @@
+package net.runelite.client.plugins.microbot;
+
+import net.runelite.api.Client;
+import net.runelite.api.World;
+import net.runelite.api.gameval.InterfaceID;
+import net.runelite.api.widgets.Widget;
+import net.runelite.client.callback.ClientThread;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.InOrder;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.util.Optional;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.atLeastOnce;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+public class MicrobotWorldHopTest {
+
+    private MockedStatic<Microbot> microbot;
+    private Client client;
+    private World target;
+    private final AtomicInteger clientThreadCalls = new AtomicInteger();
+    private final AtomicBoolean switcherLoaded = new AtomicBoolean();
+    private final AtomicInteger openCall = new AtomicInteger(-1);
+    private final AtomicInteger hopCall = new AtomicInteger(-1);
+
+    @Before
+    public void setUp() {
+        client = mock(Client.class);
+        target = mock(World.class);
+        ClientThread clientThread = mock(ClientThread.class);
+        when(clientThread.runOnClientThreadOptional(any())).thenAnswer(inv -> {
+            clientThreadCalls.incrementAndGet();
+            Callable<?> callable = inv.getArgument(0);
+            return Optional.ofNullable(callable.call());
+        });
+        Widget buttons = mock(Widget.class);
+        when(client.getWidget(InterfaceID.Worldswitcher.BUTTONS)).thenAnswer(inv -> switcherLoaded.get() ? buttons : null);
+        doAnswer(inv -> {
+            openCall.set(clientThreadCalls.get());
+            return null;
+        }).when(client).openWorldHopper();
+        doAnswer(inv -> {
+            hopCall.set(clientThreadCalls.get());
+            return null;
+        }).when(client).hopToWorld(any());
+
+        microbot = Mockito.mockStatic(Microbot.class, Mockito.CALLS_REAL_METHODS);
+        microbot.when(Microbot::getClient).thenReturn(client);
+        microbot.when(Microbot::getClientThread).thenReturn(clientThread);
+    }
+
+    @After
+    public void tearDown() {
+        microbot.close();
+    }
+
+    @Test
+    public void opensSwitcherThenHopsOnceListHasLoaded() {
+        doAnswer(inv -> {
+            openCall.set(clientThreadCalls.get());
+            new Thread(() -> {
+                try {
+                    Thread.sleep(200);
+                } catch (InterruptedException ignored) {
+                }
+                switcherLoaded.set(true);
+            }).start();
+            return null;
+        }).when(client).openWorldHopper();
+
+        assertTrue(Microbot.hopViaWorldSwitcher(target, 3000));
+
+        InOrder order = inOrder(client);
+        order.verify(client, atLeastOnce()).openWorldHopper();
+        order.verify(client).hopToWorld(target);
+        assertNotEquals(openCall.get(), hopCall.get());
+    }
+
+    @Test
+    public void hopsImmediatelyWhenSwitcherAlreadyOpen() {
+        switcherLoaded.set(true);
+
+        assertTrue(Microbot.hopViaWorldSwitcher(target, 3000));
+
+        verify(client, never()).openWorldHopper();
+        verify(client).hopToWorld(target);
+    }
+
+    @Test
+    public void doesNotHopWhenSwitcherNeverLoads() {
+        assertFalse(Microbot.hopViaWorldSwitcher(target, 300));
+
+        verify(client, atLeastOnce()).openWorldHopper();
+        verify(client, never()).hopToWorld(any());
+        assertEquals(-1, hopCall.get());
+    }
+}