Inventory UI fixes

[BenjaminAmos]

Description

This pull request fixes various issues encountered when testing the inventory UI a bit more thoroughly:

• Fixes a crash when dropping the player's shield.
• Fixes a crash when using keyboard navigation after all items are sold.
• Changes highlight behaviour to target the nearest item after the currently highlighted item is removed.

Testing

• Start a new game
• Open the inventory screen and drop each of your items one-by-one.
• When an item is dropped, the highlight cursor should move to the next item along (or the previous item, for the last item).
• The game should not crash when you drop your shield.
• Additionally, follow the instructions in list highlight crashing program #711 and ensure that the game does not crash.

Notes

This fixes #711.

[NicholasBatesNZ]

Thanks for digging into these — the item == null guard in SolShip.maybeUnequip looks right and fixes a real latent NPE. I think the selection rework needs another pass, though.

In updateItemRows() the new fallback reads items.groupCount() < selectedIndex ? items.getGroup(selectedIndex) : null — the comparison looks inverted (it calls getGroup precisely when the index is out of range) and it's missing the page * Const.ITEM_GROUPS_PER_PAGE offset that every other lookup uses, so it can throw the very IndexOutOfBoundsException it's meant to prevent.

Relatedly, the items.groupCount() > 0 guards in onKeyEvent don't cover #711's actual trace (Index: 7, Size: 7 — the container wasn't empty, selectedIndex was just stale), and since setSelected no longer falls back to selectedIndex = 0, nothing clamps selectedIndex after a removal on page 0 any more; guarding on selectedIndex + page * Const.ITEM_GROUPS_PER_PAGE < items.groupCount() would handle both. That may be what @dreaddymck is still hitting on the PR build.

One smaller thing: in getSelectionAfterRemove, idx <= 0 now conflates "not found" (-1) with "the first group" (0), so removing the top item returns no selection instead of the next one — idx < 0 looks like what you want.

Good news is it still merges cleanly onto develop after the Gradle 9.6.1 / Java 17 bump, so no rebase needed.

[coderabbitai]

Warning

Review limit reached

@soloturn, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 17 minutes

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ac0c1a13-b4f7-4b64-bda1-4ff53e83fb25

📥 Commits

Reviewing files that changed from the base of the PR and between 3d2d5b0 and fcc5f0b.

📒 Files selected for processing (3)

• engine/src/main/java/org/destinationsol/game/item/ItemContainer.java
• engine/src/main/java/org/destinationsol/game/ship/SolShip.java
• engine/src/main/java/org/destinationsol/ui/nui/screens/InventoryScreen.java

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}