From a36e312cec9e29ad94d453d10cd7ddfedc82cf2e Mon Sep 17 00:00:00 2001 From: Ourmond Date: Mon, 2 Mar 2026 20:42:47 -0800 Subject: [PATCH 1/4] refactor and fix issue --- .../features/facilities/CargoHoldTracker.java | 60 +++++++++++++++---- 1 file changed, 49 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java index 923ee1dc..5787601f 100644 --- a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java +++ b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java @@ -11,6 +11,7 @@ import com.google.common.collect.HashMultiset; import com.google.common.collect.ImmutableMultiset; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Multiset; import com.google.common.collect.Multisets; import java.awt.Color; @@ -80,12 +81,12 @@ public class CargoHoldTracker "Woooo wooo wooooo woooo." ); - private static final Set CARGO_INVENTORY_IDS = ImmutableSet.of( - InventoryID.SAILING_BOAT_1_CARGOHOLD, - InventoryID.SAILING_BOAT_2_CARGOHOLD, - InventoryID.SAILING_BOAT_3_CARGOHOLD, - InventoryID.SAILING_BOAT_4_CARGOHOLD, - InventoryID.SAILING_BOAT_5_CARGOHOLD + private static final Map CARGOHOLD_ID_TO_BOAT_SLOT = ImmutableMap.of( + InventoryID.SAILING_BOAT_1_CARGOHOLD, 1, + InventoryID.SAILING_BOAT_2_CARGOHOLD, 2, + InventoryID.SAILING_BOAT_3_CARGOHOLD, 3, + InventoryID.SAILING_BOAT_4_CARGOHOLD, 4, + InventoryID.SAILING_BOAT_5_CARGOHOLD, 5 ); private static final char CONFIG_DELIMITER_PAIRS = ';'; @@ -243,13 +244,24 @@ public void onStatChanged(StatChanged e) @Subscribe public void onItemContainerChanged(ItemContainerChanged e) { + //looks like here we + // 1. check if it is player inventory + // 2. assert it is a boat inventory + // 3. once asserted, replace our tracked inventory with the inventory associated + // 4. output new tracked inventory + // + // Lets also do this when selecting a boat + // However when selecting a boat we have access to multiple inventories at once so this event fires multiple times once per boat inventory + // so we can just update any inventory `seen` by the boat selection screen, no need for a new varbit if (e.getContainerId() == InventoryID.INV) { sawInventoryContainerUpdate = true; return; } - if (!CARGO_INVENTORY_IDS.contains(e.getContainerId() & 0x4FFF)) + log.debug("Current Logic: {}", e.getContainerId() & 0x4FFF); + log.debug("Raw ID Logic: {}", e.getContainerId()); + if (!CARGOHOLD_ID_TO_BOAT_SLOT.containsKey(e.getContainerId() & 0x4FFF)) //& 0x4FFF converts the `tradable` inventoryID into a true inventory ID { return; } @@ -257,7 +269,9 @@ public void onItemContainerChanged(ItemContainerChanged e) sawItemContainerUpdate = true; ItemContainer containerInv = e.getItemContainer(); - Multiset trackedInv = cargoHold(); + int boatSlot = CARGOHOLD_ID_TO_BOAT_SLOT.get(e.getContainerId() & 0x4FFF); + Multiset trackedInv = cargoHold(boatSlot); + log.debug("read cargo hold inventory for boat {} from event PRE CHANGE {}",boatSlot, trackedInv); trackedInv.clear(); for (Item item : containerInv.getItems()) { @@ -271,11 +285,23 @@ public void onItemContainerChanged(ItemContainerChanged e) continue; } - add(item.getId(), item.getQuantity()); + add(item.getId(), item.getQuantity(), boatSlot); + //i think the above line is causing the bug with cargoholds. this line implicitly adds the items to the current boat slot + //the current boat slot is defined as the last boat boarded. The last boat boarded is always the top listed boat in the UI + //this means we ALWAYS set the inventory container of the last boat boarded to equal the inventory of the most recently read boat's inventory + //since these events appear to fire at the same time this is not guaranteed to be in-order + + //to fix this we can change this function to also pass the boat inventory container ID + + //TODO: check downstream usages of the above LOC to determine impact + // Subtract 0x8000 from a TRADEABLE inventory to get the original inventory + // We can assume it is NOT the tradable variant IF the varbit SAILING_BOAT_SELECTION_TYPE > 0 + // if needed we can assert we are at a dock location using SAILING_BOAT_SELECTION_CURRENT_DOCK varplayer + // need to find a way to associate an inventory ID to a cargohold ID } log.debug("read cargo hold inventory from event {}", trackedInv); - writeToConfig(); + writeToConfig(boatSlot); } @Subscribe @@ -376,7 +402,7 @@ private Multiset cargoHold(int boatSlot) private int currentBoatSlot() { - return client.getVarbitValue(VarbitID.SAILING_LAST_PERSONAL_BOAT_BOARDED) - 1; + return client.getVarbitValue(VarbitID.SAILING_LAST_PERSONAL_BOAT_BOARDED); } private int usedCapacity() @@ -430,6 +456,18 @@ private void add(int item, int count) } } + private void add(int item, int count, int boatSlot) + { + if (isStackable(item)) + { + cargoHold(boatSlot).setCount(item, 1); + } + else + { + cargoHold(boatSlot).add(item, count); + } + } + private void set(int item, int count) { if (isStackable(item)) From 4211d881ea87892f4e7c02534d47124c87748169 Mon Sep 17 00:00:00 2001 From: Ourmond Date: Mon, 2 Mar 2026 20:52:28 -0800 Subject: [PATCH 2/4] clean --- .../features/facilities/CargoHoldTracker.java | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java index 5787601f..2e6dc70e 100644 --- a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java +++ b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java @@ -259,8 +259,6 @@ public void onItemContainerChanged(ItemContainerChanged e) return; } - log.debug("Current Logic: {}", e.getContainerId() & 0x4FFF); - log.debug("Raw ID Logic: {}", e.getContainerId()); if (!CARGOHOLD_ID_TO_BOAT_SLOT.containsKey(e.getContainerId() & 0x4FFF)) //& 0x4FFF converts the `tradable` inventoryID into a true inventory ID { return; @@ -286,21 +284,9 @@ public void onItemContainerChanged(ItemContainerChanged e) } add(item.getId(), item.getQuantity(), boatSlot); - //i think the above line is causing the bug with cargoholds. this line implicitly adds the items to the current boat slot - //the current boat slot is defined as the last boat boarded. The last boat boarded is always the top listed boat in the UI - //this means we ALWAYS set the inventory container of the last boat boarded to equal the inventory of the most recently read boat's inventory - //since these events appear to fire at the same time this is not guaranteed to be in-order - - //to fix this we can change this function to also pass the boat inventory container ID - - //TODO: check downstream usages of the above LOC to determine impact - // Subtract 0x8000 from a TRADEABLE inventory to get the original inventory - // We can assume it is NOT the tradable variant IF the varbit SAILING_BOAT_SELECTION_TYPE > 0 - // if needed we can assert we are at a dock location using SAILING_BOAT_SELECTION_CURRENT_DOCK varplayer - // need to find a way to associate an inventory ID to a cargohold ID } - log.debug("read cargo hold inventory from event {}", trackedInv); + log.debug("written cargo hold to boat {} from event {}", boatSlot, trackedInv); writeToConfig(boatSlot); } From cea0c5b0ebd4f1dc2e18be9cba1ef6fdb753b97d Mon Sep 17 00:00:00 2001 From: Ourmond Date: Mon, 2 Mar 2026 20:53:41 -0800 Subject: [PATCH 3/4] clean --- .../sailing/features/facilities/CargoHoldTracker.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java index 2e6dc70e..7b5c19cb 100644 --- a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java +++ b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java @@ -244,15 +244,6 @@ public void onStatChanged(StatChanged e) @Subscribe public void onItemContainerChanged(ItemContainerChanged e) { - //looks like here we - // 1. check if it is player inventory - // 2. assert it is a boat inventory - // 3. once asserted, replace our tracked inventory with the inventory associated - // 4. output new tracked inventory - // - // Lets also do this when selecting a boat - // However when selecting a boat we have access to multiple inventories at once so this event fires multiple times once per boat inventory - // so we can just update any inventory `seen` by the boat selection screen, no need for a new varbit if (e.getContainerId() == InventoryID.INV) { sawInventoryContainerUpdate = true; From 9adf3e32fb794131501f826e517cbae9bed9e459 Mon Sep 17 00:00:00 2001 From: Ourmond Date: Tue, 3 Mar 2026 14:37:48 -0800 Subject: [PATCH 4/4] use builder --- .../features/facilities/CargoHoldTracker.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java index 7b5c19cb..67485cf4 100644 --- a/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java +++ b/src/main/java/com/duckblade/osrs/sailing/features/facilities/CargoHoldTracker.java @@ -81,13 +81,13 @@ public class CargoHoldTracker "Woooo wooo wooooo woooo." ); - private static final Map CARGOHOLD_ID_TO_BOAT_SLOT = ImmutableMap.of( - InventoryID.SAILING_BOAT_1_CARGOHOLD, 1, - InventoryID.SAILING_BOAT_2_CARGOHOLD, 2, - InventoryID.SAILING_BOAT_3_CARGOHOLD, 3, - InventoryID.SAILING_BOAT_4_CARGOHOLD, 4, - InventoryID.SAILING_BOAT_5_CARGOHOLD, 5 - ); + private static final Map CARGOHOLD_ID_TO_BOAT_SLOT = ImmutableMap.builder() + .put(InventoryID.SAILING_BOAT_1_CARGOHOLD, 1) + .put(InventoryID.SAILING_BOAT_2_CARGOHOLD, 2) + .put(InventoryID.SAILING_BOAT_3_CARGOHOLD, 3) + .put(InventoryID.SAILING_BOAT_4_CARGOHOLD, 4) + .put(InventoryID.SAILING_BOAT_5_CARGOHOLD, 5) + .build(); private static final char CONFIG_DELIMITER_PAIRS = ';'; private static final char CONFIG_DELIMITER_KV = ':';