Skip to content

Make the armor kit dropdown searchable - #8963

Open
HammerGS wants to merge 5 commits into
mainfrom
Implement-searchable-armor-kit-dropdown
Open

Make the armor kit dropdown searchable#8963
HammerGS wants to merge 5 commits into
mainfrom
Implement-searchable-armor-kit-dropdown

Conversation

@HammerGS

@HammerGS HammerGS commented Sep 12, 2026

Copy link
Copy Markdown
Member

What this changes

The armor kit dropdown in Configure Pilots is searchable, like the ammunition lists.

The list runs to dozens of kits, so typing a few letters beats scrolling for one.

Also replaces the display-name-to-internal-name side map with an ArmorKitChoice record, so the selection carries its own internal name and reading it back no longer means looking the display text up in a map.

No issue number; asked for directly.

Testing

Tested in game. Typing filters the list, a kit can be picked and selected, and the selection round-trips when the dialog is reopened.

compileJava, checkstyleMain and spotlessCheck pass.

Two Copilot findings fixed. An import left unused once the display-name map became a record, and a tooltip that could never be seen: SearchableComboBox sets its own search hint on the editor field, and on an editable combo box that is what the pointer is over, so a caller's tooltip sat behind it. Fixed in the combo box rather than here, since every caller that sets a tooltip on one hits it.

What is not proven yet

  • No unit test. The change is a combo box swap plus a record; the behaviour worth testing is the round-trip through the dialog, which is what was checked by hand.
  • The tooltip change affects every SearchableComboBox, not just this one. Other callers were not re-checked on screen.
  • Setting a kit back to None was not separately confirmed.

  • This PR is focused on one issue or RFE
  • Every file in the diff has a deliberate change
  • Tests added or updated - not applicable, a dropdown swap with no rule or game-state change; verified in game instead
  • Javadoc literals use {@code true} / {@code null} rather than bare or quoted text
  • Dev team only: AI tools used -> AI Assisted Development label applied

The Configure Pilots armor kit list runs to dozens of entries, so it now
uses the same searchable dropdown as the ammunition lists: type a few
letters instead of scrolling.

Replaces the display-name-to-internal-name map with an ArmorKitChoice
record, so the selection carries its own internal name and the read-back
no longer has to look the display text up in a side map.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 12, 2026 14:31
@HammerGS HammerGS added the AI Assisted Development This project/code contains AI use under the supervision of a human developer. With Human testing. label Sep 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The current implementation has a user-visible tooltip regression with SearchableComboBox (and a small cleanup needed) that should be addressed before merging.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR updates the “Configure Pilots” UI to make the armor kit dropdown searchable (matching the UX used by the ammunition selectors) and to store armor kit selections as a value object that carries both display and internal names.

Changes:

  • Replace the armor kit JComboBox<String> with a SearchableComboBox<ArmorKitChoice> so users can type-to-filter long kit lists.
  • Replace the display-name-to-internal-name side map with an ArmorKitChoice record so reading back the selection no longer depends on a lookup map.
  • Update armor kit population/selection logic to operate on ArmorKitChoice and return the selected internal name (or null for none).
File summaries
File Description
megamek/src/megamek/client/ui/dialogs/customMek/CustomPilotViewPanel.java Switch armor kit selection to a searchable combo box and carry internal names via an ArmorKitChoice record.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

HammerGS and others added 3 commits September 12, 2026 11:02
SearchableComboBox puts its "type to search" hint on the editor field,
which is what the pointer is over on an editable combo box. A tooltip
set by the caller went on the combo box and was never seen. Overrides
setToolTipText to put it on the editor as well, falling back to the
hint when cleared.

Also removes java.util.Map from CustomPilotViewPanel, unused since the
display-name map became a record.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y1Lb2sbXg7jYcZoGDgyhWB
@HammerGS
HammerGS marked this pull request as ready for review September 12, 2026 21:51
@HammerGS
HammerGS requested a review from a team as a code owner September 12, 2026 21:51
@HammerGS HammerGS added the AI ready for Review Indicates that is has been in game tested and is ready for review as it can be label Sep 12, 2026
@HammerGS

Copy link
Copy Markdown
Member Author

Tested in game and working. Out of draft.

Typing filters the list, a kit can be picked, and the selection round-trips when the dialog is reopened. Both Copilot findings from the earlier round are fixed in 41423ab, and a formatting-only commit followed for the import order Spotless wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Assisted Development This project/code contains AI use under the supervision of a human developer. With Human testing. AI ready for Review Indicates that is has been in game tested and is ready for review as it can be

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants