Skip to content

Unit selector G/P option#8289

Open
SJuliez wants to merge 1 commit into
MegaMek:mainfrom
SJuliez:unit-selector-gp
Open

Unit selector G/P option#8289
SJuliez wants to merge 1 commit into
MegaMek:mainfrom
SJuliez:unit-selector-gp

Conversation

@SJuliez

@SJuliez SJuliez commented May 25, 2026

Copy link
Copy Markdown
Member

This moves the unit selector gunnery/piloting toggle directly into the unit selector where it is used exclusively and removes it from the GUI settings, see below. The G/P toggle still saves its state to the client preferences, so the unit selector will remember the latest setting.

Other changes:

  • moved the G/P fields down and into a single line that is hidden when G/P is turned off
  • internal changes to how they are read; when the value is illegal, they show a red error border
  • when G/P is turned off, the values reset to 4/5 so that there is no hidden BV multiplier
  • the shown unit count is now updated also when first showing the unit selector
  • bit of cleanup of code repetition
image

@SJuliez SJuliez requested a review from a team as a code owner May 25, 2026 18:06
@SJuliez SJuliez added the For New Dev Cycle This PR should be merged at the beginning of a dev cycle label May 25, 2026
@codecov

codecov Bot commented May 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 29.99%. Comparing base (ebc0fbd) to head (b937de9).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...nitSelectorDialogs/AbstractUnitSelectorDialog.java 0.00% 53 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8289      +/-   ##
============================================
- Coverage     30.00%   29.99%   -0.01%     
+ Complexity    17927    17915      -12     
============================================
  Files          3199     3199              
  Lines        311610   311606       -4     
  Branches      54369    54364       -5     
============================================
- Hits          93492    93477      -15     
- Misses       208420   208455      +35     
+ Partials       9698     9674      -24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@psikomonkie psikomonkie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One change needed: Copyright on CommonSettingsDialog. Everything code wise looks great & I'm otherwise ready to approve.

I love learning about a feature by seeing a PR where it's replaced by a better version!!

This looks fantastic. The caveats about the old implementation just make it seem so... Unhelpful. This is such a good change that I can see myself using. Thank you!

@@ -1,3 +1,3 @@
/*
* Copyright (c) 2003-2005 - Ben Mazur (bmazur@sev.org)
* Copyright (C) 2003-2025 The MegaMek Team. All Rights Reserved.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copyright

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

Labels

For New Dev Cycle This PR should be merged at the beginning of a dev cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants