From 7e771e875239e5e3c43fd2e0dabdaf8c88191336 Mon Sep 17 00:00:00 2001 From: Benjamin Amos Date: Sun, 6 Apr 2025 11:12:32 +0100 Subject: [PATCH 1/4] fix: fix keyboard navigation crash after all items sold --- .../destinationsol/ui/nui/screens/InventoryScreen.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java b/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java index 3ca548f90..330ed77ef 100644 --- a/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java +++ b/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java @@ -251,7 +251,9 @@ public boolean onKeyEvent(NUIKeyEvent event) { previousButton.getClickSound().play(previousButton.getClickVolume()); } - items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); + if (items.groupCount() > 0) { + items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); + } updateItemRows(); return true; @@ -269,7 +271,9 @@ public boolean onKeyEvent(NUIKeyEvent event) { nextButton.getClickSound().play(nextButton.getClickVolume()); } - items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); + if (items.groupCount() > 0) { + items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); + } updateItemRows(); return true; From fa6f561ad31ac38c62b6e7080e8f8ec58074c836 Mon Sep 17 00:00:00 2001 From: Benjamin Amos Date: Sun, 6 Apr 2025 12:10:36 +0100 Subject: [PATCH 2/4] fix: fix highlight after item removal --- .../game/item/ItemContainer.java | 7 +++-- .../ui/nui/screens/InventoryScreen.java | 29 +++++++++++-------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java b/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java index 09378b7e7..ed3cb56e7 100644 --- a/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java +++ b/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java @@ -130,11 +130,12 @@ public List getSelectionAfterRemove(List selected) { if (selected.size() > 1) { return selected; } - int idx = groups.indexOf(selected) + 1; - if (idx <= 0 || idx >= groupCount()) { + int groupCount = groupCount(); + int idx = groups.indexOf(selected); + if (idx <= 0 || groupCount <= 1) { return null; } - return groups.get(idx); + return groups.get(idx == (groupCount - 1) ? idx - 1 : idx + 1); } public SolItem getRandom() { diff --git a/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java b/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java index 330ed77ef..633286059 100644 --- a/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java +++ b/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java @@ -73,6 +73,7 @@ public class InventoryScreen extends NUIScreenLayer { private ColumnLayout inventoryActionButtons; private UIWarnButton closeButton; private InventoryOperationsScreen inventoryOperations; + private List selectedItemGroup; private int selectedIndex; private int page; @@ -102,6 +103,7 @@ public void initialise() { nextButton.subscribe(button -> { nextPage(button); selectedIndex = 0; + selectedItemGroup = null; updateItemRows(); }); @@ -110,6 +112,7 @@ public void initialise() { previousButton.subscribe(button -> { previousPage(button); selectedIndex = 0; + selectedItemGroup = null; updateItemRows(); }); @@ -250,6 +253,7 @@ public boolean onKeyEvent(NUIKeyEvent event) { selectedIndex--; previousButton.getClickSound().play(previousButton.getClickVolume()); } + selectedItemGroup = null; if (items.groupCount() > 0) { items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); @@ -270,6 +274,7 @@ public boolean onKeyEvent(NUIKeyEvent event) { selectedIndex++; nextButton.getClickSound().play(nextButton.getClickVolume()); } + selectedItemGroup = null; if (items.groupCount() > 0) { items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); @@ -318,18 +323,7 @@ public SolItem getSelectedItem() { * @param itemGroup the item group to select */ public void setSelected(List itemGroup) { - ItemContainer items = inventoryOperations.getItems(solApplication.getGame()); - if (!items.containsGroup(itemGroup)) { - selectedIndex = 0; - } else { - for (int groupNo = 0; groupNo < items.groupCount(); groupNo++) { - if (items.getGroup(groupNo) == itemGroup) { - page = groupNo / Const.ITEM_GROUPS_PER_PAGE; - selectedIndex = groupNo % Const.ITEM_GROUPS_PER_PAGE; - } - } - } - + selectedItemGroup = itemGroup; updateItemRows(); } @@ -541,6 +535,17 @@ private UIWidget createItemRow(int index) { public void updateItemRows() { ItemContainer items = inventoryOperations.getItems(solApplication.getGame()); + if (selectedItemGroup != null && items.containsGroup(selectedItemGroup)) { + for (int groupNo = 0; groupNo < items.groupCount(); groupNo++) { + if (items.getGroup(groupNo) == selectedItemGroup) { + page = groupNo / Const.ITEM_GROUPS_PER_PAGE; + selectedIndex = groupNo % Const.ITEM_GROUPS_PER_PAGE; + } + } + } else { + selectedItemGroup = items.groupCount() < selectedIndex ? items.getGroup(selectedIndex) : null; + } + Iterator rowsIterator = inventoryRows.iterator(); rowsIterator.next(); // Ignore the first row, since it's the header. UIWidget row = rowsIterator.next(); From 1a9d7c544454bb1e73eb7817778bcac1aa7fafe7 Mon Sep 17 00:00:00 2001 From: Benjamin Amos Date: Sun, 6 Apr 2025 12:10:51 +0100 Subject: [PATCH 3/4] fix: fix crash on shield removal --- .../src/main/java/org/destinationsol/game/ship/SolShip.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/engine/src/main/java/org/destinationsol/game/ship/SolShip.java b/engine/src/main/java/org/destinationsol/game/ship/SolShip.java index 1cbe4a663..19f17ffa4 100644 --- a/engine/src/main/java/org/destinationsol/game/ship/SolShip.java +++ b/engine/src/main/java/org/destinationsol/game/ship/SolShip.java @@ -589,6 +589,10 @@ public boolean maybeUnequip(SolGame game, SolItem item, boolean unequip) { } public boolean maybeUnequip(SolGame game, SolItem item, boolean secondarySlot, boolean unequip) { + if (item == null) { + return false; + } + if (!secondarySlot) { if (myHull.getEngine() == item) { if (unequip) { From fcc5f0b8c855be6417c13d94b424955643900fde Mon Sep 17 00:00:00 2001 From: soloturn Date: Sun, 23 Aug 2026 13:17:42 +0200 Subject: [PATCH 4/4] fix: address remaining review feedback from @NicholasBatesNZ - ItemContainer.getSelectionAfterRemove: idx <= 0 conflated 'not found' (-1) with 'first group' (0); removing the first item now correctly selects the next group instead of returning null - InventoryScreen.onKeyEvent: replace the items.groupCount() > 0 guard with a bounds check that accounts for the current selectedIndex/page, so a stale selectedIndex after a removal can't overrun items.getGroup() - InventoryScreen.updateItemRows: fix the inverted fallback comparison and add the missing page * Const.ITEM_GROUPS_PER_PAGE offset, so it can no longer throw the IndexOutOfBoundsException it was meant to prevent --- .../java/org/destinationsol/game/item/ItemContainer.java | 2 +- .../org/destinationsol/ui/nui/screens/InventoryScreen.java | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java b/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java index ed3cb56e7..c4e653556 100644 --- a/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java +++ b/engine/src/main/java/org/destinationsol/game/item/ItemContainer.java @@ -132,7 +132,7 @@ public List getSelectionAfterRemove(List selected) { } int groupCount = groupCount(); int idx = groups.indexOf(selected); - if (idx <= 0 || groupCount <= 1) { + if (idx < 0 || groupCount <= 1) { return null; } return groups.get(idx == (groupCount - 1) ? idx - 1 : idx + 1); diff --git a/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java b/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java index 633286059..dff2593ce 100644 --- a/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java +++ b/engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java @@ -255,7 +255,7 @@ public boolean onKeyEvent(NUIKeyEvent event) { } selectedItemGroup = null; - if (items.groupCount() > 0) { + if (selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE < items.groupCount()) { items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); } @@ -276,7 +276,7 @@ public boolean onKeyEvent(NUIKeyEvent event) { } selectedItemGroup = null; - if (items.groupCount() > 0) { + if (selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE < items.groupCount()) { items.seen(items.getGroup(selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE)); } @@ -543,7 +543,8 @@ public void updateItemRows() { } } } else { - selectedItemGroup = items.groupCount() < selectedIndex ? items.getGroup(selectedIndex) : null; + int groupNo = page * Const.ITEM_GROUPS_PER_PAGE + selectedIndex; + selectedItemGroup = groupNo < items.groupCount() ? items.getGroup(groupNo) : null; } Iterator rowsIterator = inventoryRows.iterator();