-
Notifications
You must be signed in to change notification settings - Fork 344
Random army generators (continued) #7938
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7938 +/- ##
============================================
- Coverage 29.74% 29.74% -0.01%
+ Complexity 16562 16557 -5
============================================
Files 3138 3142 +4
Lines 301504 301553 +49
Branches 52711 52731 +20
============================================
+ Hits 89688 89702 +14
- Misses 202518 202552 +34
- Partials 9298 9299 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| //region ACTIONS | ||
|
|
||
| private class ToleranceAction extends AbstractAction { |
Check failure
Code scanning / CodeQL
No clone method Error
| } | ||
| } | ||
|
|
||
| private class UnitCountAction extends AbstractAction { |
Check failure
Code scanning / CodeQL
No clone method Error
| } | ||
| } | ||
|
|
||
| private class TargetPvAction extends AbstractAction { |
Check failure
Code scanning / CodeQL
No clone method Error
| } | ||
| } | ||
|
|
||
| private class BalanceByAction extends AbstractAction { |
Check failure
Code scanning / CodeQL
No clone method Error
| } | ||
| } | ||
|
|
||
| private class ManualValueAction extends AbstractAction { |
Check failure
Code scanning / CodeQL
No clone method Error
| parameters.asPanel = asSearchFilter; | ||
| parameters.meks = Integer.parseInt(mekCount.getText()); | ||
| parameters.tanks = Integer.parseInt(veeCount.getText()); | ||
| parameters.ba = Integer.parseInt(baCount.getText()); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
| parameters.meks = Integer.parseInt(mekCount.getText()); | ||
| parameters.tanks = Integer.parseInt(veeCount.getText()); | ||
| parameters.ba = Integer.parseInt(baCount.getText()); | ||
| parameters.infantry = Integer.parseInt(infCount.getText()); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
| parameters.ba = Integer.parseInt(baCount.getText()); | ||
| parameters.infantry = Integer.parseInt(infCount.getText()); | ||
| parameters.canon = onlyCanon.isSelected(); | ||
| parameters.maxBV = Integer.parseInt(bvMax.getText()); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
| parameters.infantry = Integer.parseInt(infCount.getText()); | ||
| parameters.canon = onlyCanon.isSelected(); | ||
| parameters.maxBV = Integer.parseInt(bvMax.getText()); | ||
| parameters.minBV = Integer.parseInt(bvMin.getText()); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
|
|
||
| @Override | ||
| public List<MekSummary> generateMekSummaries() { | ||
| int units = Integer.parseInt(unitCount.getText()); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the random army generation UI into dedicated tab classes, adds a new “simple” lance creator that can balance forces by several metrics (BV/PV/cost/tonnage), and introduces some supporting utilities (e.g., LambdaAction, SimpleRandomLanceCreator, new table models).
Changes:
- Introduces
SimpleRandomLanceCreatorandSimpleRandomLancePanelto generate simple lances from filtered rosters by arbitrary “strength” functions (BV, PV, cost, tons, etc.). - Refactors
AbstractRandomArmyDialoginto a tab-based architecture using theRandomArmyTabinterface, with new BV-matching, RAT, RAT generator, and formation tabs plus shared preview/selection UI viaRandomArmyUnitTableandRatTableModel. - Cleans up older
RandomArmyCreatorandRandomArmyDialogAPIs, adjusts lobby utilities and GUIPreferences, and improves localization keys and labels for the random army UI.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
megamek/src/megamek/common/util/SimpleRandomLanceCreator.java |
New generic “simple lance” generator that builds a fixed-size force close to a target strength using a simulated-annealing-style search, plus helper for advanced MekSummary filtering. |
megamek/src/megamek/common/util/RandomArmyCreator.java |
Simplified generateArmy API (no more StringBuilder side channels) and made AS/TW filters null-safe; keeps core BV-matching logic. |
megamek/src/megamek/client/ui/util/LambdaAction.java |
Small Swing Action wrapper that binds a name to a Consumer<ActionEvent> for cleaner action wiring. |
megamek/src/megamek/client/ui/panels/phaseDisplay/lobby/LobbyUtility.java |
Switched RAT unit-table use to the new RatTableModel, generalized BV/cost actions to accept any Collection<Entity>, and streamlined row-selection helper. |
megamek/src/megamek/client/ui/dialogs/randomArmy/SimpleRandomLancePanel.java |
New UI tab for the simple lance creator, including filter integration, unit count/target/tolerance controls, and wiring to SimpleRandomLanceCreator. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RatTableModel.java |
Extracted RAT table model into its own class; provides read-only columns for weight, unit name, BV, tech base, and role plus getUnitAt. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RandomArmyUnitTable.java |
New reusable table subclass for displaying rolled/selected units with context menu actions for readout, BV, and cost via LobbyUtility. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RandomArmyTab.java |
New interface for random-army tabs, unifying “generate Mek summaries” plus optional game-options and skill-generator hooks. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RandomArmyRatTab.java |
New RAT-based random army tab: RAT tree, unit-count field, and generation via RandomUnitGenerator. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RandomArmyRatGenTab.java |
New RAT generator tab using ForceGenerationOptionsPanel and RatTableModel, with context menu actions and “add from RAT to chosen” behaviour. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RandomArmyDialog.java |
Adapts OK logic to new models (forceGeneratorPanel, chosenUnitsModel, formationPanel), while preserving faction and entity-loading behaviour. |
megamek/src/megamek/client/ui/dialogs/randomArmy/RandomArmyBvTab.java |
New BV-matching tab that wraps RandomArmyCreator with year/BV ranges, canon-only option, advanced search filters, and per-unit-type counts with roster summaries. |
megamek/src/megamek/client/ui/dialogs/randomArmy/MMMainMenuRandomArmyDialog.java |
Updates to use forceGeneratorPanel and chosenUnitsModel when returning units to the main menu path. |
megamek/src/megamek/client/ui/dialogs/randomArmy/MMLForceBuilderRandomArmyDialog.java |
Same as above, but for the force-builder integration path. |
megamek/src/megamek/client/ui/dialogs/randomArmy/ForceGenerationOptionsPanel.java |
Implements RandomArmyTab by moving the formation-generation logic here (previously in AbstractRandomArmyDialog), adding logging, and returning generated MekSummary lists. |
megamek/src/megamek/client/ui/dialogs/randomArmy/AbstractRandomArmyDialog.java |
Major refactor: removes monolithic tab logic and ad-hoc popups, introduces RandomArmyTab registration, separate rolled/chosen tables with BV totals, and a generic roll/add/clear workflow. |
megamek/src/megamek/client/ui/clientGUI/calculationReport/HTMLCalculationReport.java |
Makes HTML_START public so other components can reuse the constant. |
megamek/src/megamek/client/ui/clientGUI/GUIPreferences.java |
Removes now-unused RAT BV/tech/count/year/pad preference setters/getters while keeping getRATSelectedRAT. |
megamek/resources/megamek/client/messages*.properties |
Adds strings for the simple tab and refactors several random-army labels (BV total with {0}, year label text, button texts, etc.) to match the new UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public List<T> buildForce(List<? extends T> available, int unitCount, int targetStrength, int strengthMargin) { | ||
|
|
||
| if (available.isEmpty()) { | ||
| return Collections.emptyList(); | ||
| } | ||
|
|
||
| List<T> currentResult = randomSample(available, unitCount); | ||
| double currentStrength = currentResult.stream().mapToDouble(strengthFunction).sum(); | ||
|
|
||
| double temperature = currentStrength / 2.0; | ||
|
|
||
| for (int iterations = 0; iterations < MAX_ITERATIONS; iterations++) { | ||
| if (Math.abs(currentStrength - targetStrength) <= strengthMargin) { | ||
| return currentResult; // success | ||
| } | ||
|
|
||
| int replacementIndex = rnd.nextInt(unitCount); | ||
| T out = currentResult.get(replacementIndex); | ||
| T in = available.get(rnd.nextInt(available.size())); | ||
|
|
||
| double newStrength = currentStrength - strengthFunction.applyAsDouble(out) + strengthFunction.applyAsDouble(in); | ||
| double delta = Math.abs(newStrength - targetStrength) - Math.abs(currentStrength - targetStrength); | ||
|
|
||
| // temperature allows worsening of the result in the beginning while increasingly rejecting it later on | ||
| if (delta <= 0 || rnd.nextDouble() < Math.exp(-delta / temperature)) { | ||
| currentResult.set(replacementIndex, in); | ||
| currentStrength = newStrength; | ||
| } | ||
|
|
||
| temperature *= 0.995; // lower the temperature over iterations | ||
| } | ||
|
|
||
| return currentResult; // best-so-far | ||
| } | ||
|
|
||
| private List<T> randomSample(List<? extends T> available, int unitCount) { | ||
| List<T> result = new ArrayList<>(unitCount); | ||
| for (int i = 0; i < unitCount; i++) { | ||
| result.add(available.get(rnd.nextInt(available.size()))); | ||
| } | ||
| return result; |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
buildForce assumes unitCount is �e1 and will throw at runtime for zero or negative values: randomSample calls rnd.nextInt(unitCount) and the simulated annealing loop uses rnd.nextInt(unitCount) as well. Since SimpleRandomLancePanel allows the user to enter arbitrary counts (including 0) via ManualValueAction, this will result in an IllegalArgumentException. Consider rejecting non-positive unitCount up front (e.g., via argument validation) or treating unitCount �e0 as a special case that immediately returns an empty list without entering the loop.
| List<MekSummary> result = creator.buildForce(roster, unitCount, targetValue, tolerancePV()); | ||
| double achievedValue = result.stream().mapToDouble(strengthMapper).sum(); | ||
| resultLabel.putClientProperty(FlatClientProperties.STYLE_CLASS, "small"); | ||
| resultLabel.setText("Created units: " + formatResult(achievedValue) + " " + balanceBy.displayName()); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generateMekSummaries sets resultLabel twice in a row (once with a literal string and then immediately with the i18n message), so the first assignment is never visible. You can remove the first setText call and keep only the Messages.getString("RandomArmyDialog.Simple.Created", ...) assignment to avoid dead code and make the intent clearer.
| resultLabel.setText("Created units: " + formatResult(achievedValue) + " " + balanceBy.displayName()); |
| private class ManualValueAction extends AbstractAction { | ||
|
|
||
| private final Consumer<Integer> writeToField; | ||
|
|
||
| ManualValueAction(Consumer<Integer> writeToField) { | ||
| super("Other..."); | ||
| this.writeToField = writeToField; | ||
| } | ||
|
|
||
| @Override | ||
| public void actionPerformed(ActionEvent e) { | ||
| Object result = JOptionPane.showInputDialog("Enter value"); | ||
| if (result != null) { | ||
| try { | ||
| int value = Integer.parseInt(result.toString()); | ||
| writeToField.accept(value); | ||
| } catch (NumberFormatException ignored) { | ||
| // bad input, not necessary to do anything | ||
| } | ||
| update(); | ||
| } | ||
| } |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ManualValueAction uses hard-coded UI strings ("Other..." and "Enter value") instead of pulling them from the Messages bundle, while the rest of this panel already uses Messages.getString for user-facing text. For consistency with the rest of the dialog and existing i18n support, consider moving these strings into the resource bundle and referencing them via Messages.getString(...).
| JOptionPane.showMessageDialog(this, | ||
| "Some numbers could not be parsed. Please check the input fields."); | ||
| } finally { |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message shown when parsing numeric inputs fails ("Some numbers could not be parsed. Please check the input fields.") is hard-coded and bypasses the Messages resource bundle used elsewhere in this dialog. To keep the UI fully localizable and consistent with the rest of the codebase, it would be better to add a RandomArmyDialog message key and retrieve the text via Messages.getString(...) instead of embedding the English string here.
| ratModel.refreshData(); | ||
| } | ||
| formationPanel.setYear(gameYear); | ||
| forceGeneratorPanel.setYear(gameYear); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updateRATYear now only updates formationPanel and forceGeneratorPanel, but the RAT generator tab (RandomArmyRatGenTab) also depends on the current game year via its internal ForceGenerationOptionsPanel. Previously, updateRATYear called setYear on the RAT generator options as well; after this refactor, that panel is never updated when game options change, so RAT generation may use a stale or default year until the user adjusts it manually. Consider keeping a reference to the RAT generator tab or its options panel and updating its year here (or delegating via RandomArmyTab.setGameOptions) to preserve the old behaviour.
| forceGeneratorPanel.setYear(gameYear); | |
| forceGeneratorPanel.setYear(gameYear); | |
| // Ensure all generator tabs (including the RAT generator tab) are updated | |
| // when the game options change, so they use the correct year. | |
| if (generators != null) { | |
| for (RandomArmyTab tab : generators.values()) { | |
| if (tab != null) { | |
| tab.setGameOptions(gameOptions); | |
| } | |
| } | |
| } |
| parameters.asPanel = asSearchFilter; | ||
| parameters.meks = Integer.parseInt(mekCount.getText()); | ||
| parameters.tanks = Integer.parseInt(veeCount.getText()); | ||
| parameters.ba = Integer.parseInt(baCount.getText()); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
| parameters.meks = Integer.parseInt(mekCount.getText()); | ||
| parameters.tanks = Integer.parseInt(veeCount.getText()); | ||
| parameters.ba = Integer.parseInt(baCount.getText()); | ||
| parameters.infantry = Integer.parseInt(infCount.getText()); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
| parameters.ba = Integer.parseInt(baCount.getText()); | ||
| parameters.infantry = Integer.parseInt(infCount.getText()); | ||
| parameters.canon = onlyCanon.isSelected(); | ||
| parameters.maxBV = Integer.parseInt(bvMax.getText()); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
| parameters.infantry = Integer.parseInt(infCount.getText()); | ||
| parameters.canon = onlyCanon.isSelected(); | ||
| parameters.maxBV = Integer.parseInt(bvMax.getText()); | ||
| parameters.minBV = Integer.parseInt(bvMin.getText()); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
|
|
||
| @Override | ||
| public List<MekSummary> generateMekSummaries() { | ||
| int units = Integer.parseInt(unitCount.getText()); |
Copilot
AI
Jan 30, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential uncaught 'java.lang.NumberFormatException'.
| int units = Integer.parseInt(unitCount.getText()); | |
| int units; | |
| try { | |
| units = Integer.parseInt(unitCount.getText()); | |
| } catch (NumberFormatException ex) { | |
| // Fall back to the default number of units if the input is invalid | |
| units = 4; | |
| unitCount.setText(String.valueOf(units)); | |
| } |
Separates out the random army generator tabs that had still been part of the central class and unifies behavior somewhat. The new tab classes are also refactored to make the UI code less obnoxious.
Also adds a tab with a new "simple" lance creator. The backing creator class is similar to the BV matching creator but more generic. It can generate a group of units (no force structure) of any class (Entity, MekSummary, AS element, SBF Unit) so long as it is given a roster to choose from and a "strength" function with which to match the target value. The roster is easiest to get from the unit cache (for class MekSummary). The standard strength function is MekSummary::getBV but others can be used instead. The random army tab supports BV, PV, weight and cost, i.e. a lance of units for e.g. exactly 15000000 c-bills can be rolled up.