Release Microbot 2.6.34: telemetry-driven crash fixes

docs/entity-guides/README.md

@@ -23,6 +23,12 @@ Route camera turns only for unreachable targets: [movement gotcha 23](movement.m
 
 Camera turns wait for the yaw to settle before returning: [movement gotcha 24](movement.md#24-a-camera-turn-is-finished-only-when-the-yaw-stops-changing).
 
+Scene tile loops on a boat worldview must use the tile array bounds: [movement gotcha 25](movement.md#25-bound-scene-tile-loops-by-the-tile-array-not-constantsscene_size).
+
+Instance location conversions must null-check `LocalPoint.fromWorld`: [movement gotcha 26](movement.md#26-null-check-localpointfromworld-before-converting-instance-coordinates).
+
+Bounds-check a `LocalPoint` against its worldview before projecting it to the canvas: [movement gotcha 27](movement.md#27-bounds-check-a-localpoint-against-its-own-worldview-before-projecting-it).
+
 ## Format
 
 Each entity guide is a numbered list of gotchas. Each entry follows this structure:

docs/entity-guides/items.md

@@ -274,6 +274,20 @@ An ordinary logout can be a break or world hop, so it must keep normal AutoLogin
 **Defensive check:** Run `AutoLoginSuppressionTest`, `AutoLoginPluginSupplyStopTest`, `ScriptStartedTest`, `TerminalSupplyLogoutTest`, and `InventorySetupSupplyLogoutTest`. Verify live that a shortage stays logged out and restarting a gameplay script permits login.
 
 
+## 16. Bank widgets can be missing or blank right after the bank opens
+
+`BANK_ITEM_CONTAINER` can be null and the `BANK_ITEM_COUNT_TOP` text can be `""` while the bank interface is opening, closing or rebuilding. Never `assert` a widget non-null (asserts are disabled at runtime) and never `Integer.parseInt` widget text without handling blank or non-numeric input. `Rs2Bank.getBankItemCount()` returns 0 for unpopulated text, and `scrollBankToSlot` returns false before computing a scroll when the container is missing. The looting bag view (`15:11`) can be unloaded or have null/empty children once the bag closes, so `depositLootingBag` waits on `Rs2Bank.isLootingBagViewEmpty()`, which reads the widget on the client thread and treats a missing widget or children as emptied.
+
+**Defensive check:** Run `Rs2BankUnloadedWidgetsTest` and `Rs2BankLootingBagViewTest`.
+
+
+## 17. An inventory container may have no child for the item's slot
+
+While a shop, bank, deposit box or Grand Exchange inventory is open or rebuilding, the active container's children may not include a widget whose index matches `Rs2ItemModel.getSlot()`. Look the child up with `findFirst().orElse(null)` and fall back to `rs2Item.getInventoryActions()`; never pass `null` to `Optional.orElseGet`, which throws an NPE instead of returning null.
+
+**Defensive check:** Run `Rs2InventoryInvokeMenuTest`.
+
+
 ## Terminal supply stops need observed logout
 
 An owning script can set `withMissingSupplyHandler` on its Inventory Setup loader to keep its scheduled loop alive while `TerminalLogoutPrompt` closes the bank and requests logout. Do not show a blocking shortage notification or cancel that loop until `LOGGED_OUT` is observed. The default loader policy stays unchanged when no handler is supplied; ordinary breaks and world hops do not arm terminal Auto Login suppression.

docs/entity-guides/movement.md

@@ -481,3 +481,70 @@ north and a pitch of 0 sends `adjustPitch` holding UP, so a fake 0 looked like a
 
 **Defensive check:** `Rs2CameraSettleTest` simulates a camera that coasts after release. It
 also covers the timeout bound and the client-thread no-op.
+
+## 25. Bound scene tile loops by the tile array, not `Constants.SCENE_SIZE`
+
+The local player's worldview is not always the 104x104 top-level scene. On a sailing boat
+the player stands in a sub-worldview whose `Scene.getTiles()` is only 8 or 16 tiles wide.
+A loop that runs `x, y < Constants.SCENE_SIZE` over that array throws
+`ArrayIndexOutOfBoundsException` at the boat's width.
+
+**Why this matters:** `Rs2GameObject.getSceneObjects` read tiles from
+`player.getWorldView().getScene()` and looped to `SCENE_SIZE`. Every scene-object query made
+from a boat threw `Index 8 out of bounds for length 8`, which aborted the walker inside
+`handleNearbyRawPathSceneObjects` and crashed banking in scripts that called `findBank`.
+
+**Pattern to follow:** take the loop bounds from the array you index (`tiles[z].length`,
+`tiles[z][0].length`), or from `worldView.getSizeX()` / `getSizeY()`, whenever the tiles come
+from a worldview that is not guaranteed to be the top-level one.
+
+**Defensive check:** `Rs2GameObjectSmallWorldViewTest` queries wall objects on an 8x8
+worldview and expects the object on the last tile.
+
+## 26. Null-check `LocalPoint.fromWorld` before converting instance coordinates
+
+`LocalPoint.fromWorld(worldView, worldPoint)` returns `null` when the plane differs from the world view's plane or the tile is outside that scene, for example in an instance while the scene is still loading. The fallback below only prevents the crash: if the player is on a boat sub-worldview, `player.getLocalLocation()` is in boat space and does not give the real sea position. `WorldPoint.fromLocalInstance` dereferences its `LocalPoint` immediately, so the result must be checked first.
+
+**Why this matters:** `Rs2Player.getWorldLocation_Internal()` passed the `fromWorld` result straight into `fromLocalInstance`. Telemetry for 2.6.30 and 2.6.31 recorded about 585 NullPointerExceptions, most from `KourendLibraryOverlay.render` calling `Rs2Player.getWorldLocation()` every frame.
+
+**Pattern to follow:**
+
+```java
+// Wrong: NPE when the world point does not resolve in the top-level scene.
+LocalPoint l = LocalPoint.fromWorld(client.getTopLevelWorldView(), player.getWorldLocation());
+return WorldPoint.fromLocalInstance(client, l);
+
+// Right: fall back to the player's own local point, then to the raw world point.
+LocalPoint l = LocalPoint.fromWorld(client.getTopLevelWorldView(), player.getWorldLocation());
+if (l == null) l = player.getLocalLocation();
+if (l == null) return player.getWorldLocation();
+return WorldPoint.fromLocalInstance(client, l);
+```
+
+**Where this applies:** `Rs2Player.getWorldLocation_Internal()` and any helper that converts an actor's world point to an instance template point.
+
+**Defensive check:** Mock a world view whose plane differs from the player's world point and assert the helper returns a fallback point, as in `Rs2PlayerInstanceWorldLocationTest`.
+
+## 27. Bounds-check a `LocalPoint` against its own worldview before projecting it
+
+`Perspective.getCanvasTilePoly` only rejects points outside the extended scene (`getSizeX() + 2 * ESCENE_OFFSET` on the top-level view). A tile with scene X/Y in 104..143, or a negative one, still reaches `WorldView.getTileHeight`, which indexes the normal scene height array and throws `ArrayIndexOutOfBoundsException`. Resolve the worldview from `localPoint.getWorldView()` and check the scene X/Y against `0..getSizeX()-1` / `0..getSizeY()-1` before projecting.
+
+**Why this matters:** `Rs2Camera.isTileOnScreen` passed NPC and tile-object local points straight to `getCanvasTilePoly`. Telemetry for 2.6.30 to 2.6.32 recorded `Index 109/111/126 out of bounds for length 105` from `Rs2NpcModel.click` and `Rs2TileObjectModel.click`, which aborted the interaction instead of turning the camera.
+
+**Pattern to follow:**
+
+```java
+// Wrong: out-of-scene points throw inside Perspective.
+Polygon poly = Perspective.getCanvasTilePoly(client, localPoint);
+
+// Right: treat a point outside its worldview's scene as off screen.
+if (localPoint == null) return false;
+WorldView wv = client.getWorldView(localPoint.getWorldView());
+if (wv == null || localPoint.getSceneX() < 0 || localPoint.getSceneY() < 0
+        || localPoint.getSceneX() >= wv.getSizeX() || localPoint.getSceneY() >= wv.getSizeY()) return false;
+Polygon poly = Perspective.getCanvasTilePoly(client, localPoint);
+```
+
+**Where this applies:** `Rs2Camera.isTileOnScreen(LocalPoint)`, `Rs2Camera.isTileOnScreen(TileObject)`, and any helper that projects an entity's local point to the canvas.
+
+**Defensive check:** Mock a worldview whose `getTileHeight` throws and assert out-of-scene points return false without calling it, as in `Rs2CameraTileOnScreenTest`.

docs/entity-guides/worlds.md

@@ -41,10 +41,10 @@ return Microbot.getClientThread().runOnClientThreadOptional(() -> {
 }).orElse(-1);
 ```
 
-**Where this applies:** `Rs2Player` accessors (`getWorldLocation_Internal`, `getPoseAnimation`, `getAnimation`, `getCombatLevel`, `getLocalPlayer`, `getLocalLocation`, `getGraphicId`, `hasSpotAnimation`), the position accessors of `Rs2ActorModel` (`getWorldView`, `getWorldLocation`, `getLocalLocation`, `projectActorLocationToMainWorld`) and its `getAnimation` (returns -1), which `new Rs2PlayerModel()` reaches with a null actor, and any new helper that reads the local player.
+**Where this applies:** `Rs2Player` accessors (`getWorldLocation_Internal`, `getPoseAnimation`, `getAnimation`, `getCombatLevel`, `getLocalPlayer`, `getLocalLocation`, `getGraphicId`, `hasSpotAnimation`), the position accessors of `Rs2ActorModel` (`getWorldView`, `getWorldLocation`, `getLocalLocation`, `projectActorLocationToMainWorld`) and its `getAnimation` (returns -1), which `new Rs2PlayerModel()` reaches with a null actor, the callers of those accessors (`Rs2Camera.angleToTile` returns the current facing so a turn is a no-op), both `Rs2NpcModel` classes' `getDistanceFromPlayer` (returns `Integer.MAX_VALUE`) and `isWithinDistanceFromPlayer` (returns `false`), `Rs2GameObject.clickObject` (returns `false` when it falls back to world-point distance), and any new helper that reads the local player.
 
-**Defensive check:** Mock `Client.getLocalPlayer()` to return `null` and assert each accessor returns its fallback, as in `Rs2PlayerNullLocalPlayerTest` and `Rs2PlayerModelNullLocalPlayerTest`.
+**Defensive check:** Mock `Client.getLocalPlayer()` to return `null` and assert each accessor returns its fallback, as in `Rs2PlayerNullLocalPlayerTest`, `Rs2PlayerModelNullLocalPlayerTest` and `Rs2NpcModelDistanceFromPlayerTest`.
 
 **Wrapped locations:** `Rs2Player.getRs2WorldPoint()` still returns an `Rs2WorldPoint`, but it can wrap a `null` `WorldPoint` in these states. `pathTo` returns `null` and `distanceToPath` returns `Integer.MAX_VALUE` for it; check `getWorldPoint() != null` before calling `getX()`, `getY()` or `getPlane()`, as in `Rs2WorldPointNullPointTest`.
 
-**Location consumers:** `Rs2Player.getWorldLocation()` itself returns `null` in these states. Guard it before passing it to `Rs2RouteRequest.to`/`toAny` (which reject a null start) or `WorldPoint.distanceTo`. `Rs2Bank.getNearestBank`, `getNearestBankRoute` and `getPathToNearestBank` return no bank, `isNearBank` returns `false`, and `Rs2Walker.getDistanceBetween` returns `Integer.MAX_VALUE` for a null endpoint, as in `Rs2BankNullPlayerLocationTest`.
+**Location consumers:** `Rs2Player.getWorldLocation()` itself returns `null` in these states. Guard it before passing it to `Rs2RouteRequest.to`/`toAny` (which reject a null start) or `WorldPoint.distanceTo`. `Rs2Bank.getNearestBank`, `getNearestBankRoute` and `getPathToNearestBank` return no bank, `isNearBank` and `walkToBank` return `false`, `BankLocation.MINING_GUILD.hasRequirements` returns `false`, and `Rs2Walker.getDistanceBetween` returns `Integer.MAX_VALUE` for a null endpoint, as in `Rs2BankNullPlayerLocationTest`.

gradle.properties

@@ -32,7 +32,7 @@ project.build.version=1.13.2
 runelite.injected-client.version=1.13.2
 
 glslang.path=
-microbot.version=2.6.33
+microbot.version=2.6.34
 microbot.commit.sha=nogit
 microbot.repo.url=http://138.201.81.246:8081/repository/microbot-snapshot/
 microbot.repo.username=

runelite-client/src/main/java/net/runelite/client/config/ConfigManager.java

@@ -29,6 +29,7 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ComparisonChain;
 import com.google.gson.Gson;
+import com.google.gson.JsonParseException;
 import lombok.Getter;
 import lombok.NonNull;
 import lombok.extern.slf4j.Slf4j;
@@ -1259,7 +1260,7 @@ public <T extends Config> void setDefaultConfiguration(T proxy, boolean override
 			// null and the empty string are treated identically in sendConfig and treated as an unset
 			// If a config value defaults to "" and the current value is null, it will cause an extra
 			// unset to be sent, so treat them as equal
-			boolean isInventorySetup = item.keyName().toLowerCase().contains("inventorysetup");
+			boolean isInventorySetup = method.getReturnType() == InventorySetup.class;
 			if (Objects.equals(current, valueString) || (Strings.isNullOrEmpty(current) && Strings.isNullOrEmpty(valueString)))
 			{
 				if (isInventorySetup) {
@@ -1274,9 +1275,16 @@ public <T extends Config> void setDefaultConfiguration(T proxy, boolean override
 			if (current != null && !current.isBlank()) {
 				if (isInventorySetup) {
 					var setups = configInventorySetupDataManager.loadV3Setups(this);
-					InventorySetup currentObj = gson.fromJson(current, InventorySetup.class);
+					InventorySetup currentObj;
+					try {
+						currentObj = gson.fromJson(current, InventorySetup.class);
+					} catch (JsonParseException | IllegalStateException e) {
+						log.debug("Unable to parse inventory setup for {}.{}", group.value(), item.keyName(), e);
+						currentObj = null;
+					}
+					String currentName = currentObj == null ? null : currentObj.getName();
 					if (!setups.isEmpty() && setups.stream()
-							.noneMatch(x -> x.getName().equalsIgnoreCase(currentObj.getName()))) {
+							.noneMatch(x -> x.getName().equalsIgnoreCase(currentName))) {
 						setConfiguration(group.value(), item.keyName(), "");
 					}
 				}

runelite-client/src/main/java/net/runelite/client/config/ProfileManager.java

@@ -43,6 +43,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Predicate;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -57,6 +58,7 @@
 public class ProfileManager {
     private static final File PROFILES_DIR = new File(RuneLite.RUNELITE_DIR, "microbot-profiles");
     private static final File PROFILES = new File(PROFILES_DIR, "profiles.json");
+    private static final ReentrantLock PROCESS_LOCK = new ReentrantLock();
 
     private final Gson gson;
 
@@ -73,11 +75,26 @@ public class Lock implements AutoCloseable {
 
         @SneakyThrows
         public Lock() {
-            lockFile = new File(PROFILES_DIR, "profiles.lck");
-            lockOut = new FileOutputStream(lockFile);
-            lockChannel = lockOut.getChannel();
-            lockChannel.lock();
-            profiles = new ArrayList<>(load());
+            PROCESS_LOCK.lock();
+            FileOutputStream out = null;
+            try {
+                lockFile = new File(PROFILES_DIR, "profiles.lck");
+                out = new FileOutputStream(lockFile);
+                out.getChannel().lock();
+                lockOut = out;
+                lockChannel = out.getChannel();
+                profiles = new ArrayList<>(load());
+            } catch (Throwable t) {
+                if (out != null) {
+                    try {
+                        out.close();
+                    } catch (IOException e) {
+                        t.addSuppressed(e);
+                    }
+                }
+                PROCESS_LOCK.unlock();
+                throw t;
+            }
         }
 
         private List<ConfigProfile> load() {
@@ -97,6 +114,19 @@ private List<ConfigProfile> load() {
         @Override
         @SneakyThrows
         public void close() {
+            try {
+                save();
+            } finally {
+                try {
+                    lockOut.close();
+                    lockFile.delete();
+                } finally {
+                    PROCESS_LOCK.unlock();
+                }
+            }
+        }
+
+        private void save() throws IOException {
             if (modified) {
                 log.debug("saving {} profiles", profiles.size());
 
@@ -119,9 +149,6 @@ public void close() {
                     Files.move(tempFile.toPath(), PROFILES.toPath(), StandardCopyOption.REPLACE_EXISTING);
                 }
             }
-
-            lockOut.close();
-            lockFile.delete();
         }
 
         public List<ConfigProfile> getProfiles() {

runelite-client/src/main/java/net/runelite/client/plugins/grounditems/GroundItemsPlugin.java

@@ -190,7 +190,10 @@ static class PriceHighlight
 	private final Map<WorldPoint, Lootbeam> lootbeams = new HashMap<>();
 
 	public static Table<WorldPoint, Integer, GroundItem> getCollectedGroundItems() {
-		return ImmutableTable.copyOf(collectedGroundItems);
+		synchronized (collectedGroundItems)
+		{
+			return ImmutableTable.copyOf(collectedGroundItems);
+		}
 	}
 
 	@Provides

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

@@ -111,7 +111,7 @@ private void loopOnce() {
         try {
             validateAndEnqueueWithBackoff();
         } catch (Throwable t) {
-            Microbot.log(Level.ERROR, "BlockingEvent loop error: %s", t);
+            log.error("BlockingEvent loop error", t);
         } finally {
             // re-schedule using the latest currentDelay (volatile)
             loopFuture = scheduler.schedule(this::loopOnce, currentDelay, TimeUnit.MILLISECONDS);
@@ -138,10 +138,7 @@ private void validateAndEnqueueWithBackoff() {
                         }
                     }
                 } catch (Exception ex) {
-                    Microbot.log(Level.ERROR,
-                            "Error validating BlockingEvent (%s): %s",
-                            event.getName(),
-                            ex);
+                    Microbot.log("Error validating BlockingEvent (" + event.getName() + ")", Level.ERROR, ex);
                 }
             }
         }
@@ -194,10 +191,7 @@ public boolean shouldBlockAndProcess()
             }
             catch (Exception ex)
             {
-                Microbot.log(Level.ERROR,
-                        "Error executing BlockingEvent (%s): %s",
-                        event.getName(),
-                        ex);
+                Microbot.log("Error executing BlockingEvent (" + event.getName() + ")", Level.ERROR, ex);
             }
             finally
             {

runelite-client/src/main/java/net/runelite/client/plugins/microbot/accountselector/AutoLoginScript.java

@@ -480,8 +480,9 @@ private void updatePlayerNameCache() {
         // detect fresh login - update player name cache
         if (currentlyLoggedIn && !wasLoggedIn) {
             Rs2PlayerModel localPlayer = Rs2Player.getLocalPlayer();
-            if (localPlayer != null && localPlayer.getName() != null) {
-                lastKnownPlayerName = localPlayer.getName();
+            String playerName = localPlayer != null ? localPlayer.getName() : null;
+            if (playerName != null) {
+                lastKnownPlayerName = playerName;
                 log.info("Updated cached player name: {}", lastKnownPlayerName);
             }
         }