Skip to content

Patchwork work#2149

Open
SJuliez wants to merge 6 commits intoMegaMek:mainfrom
SJuliez:patchwork-work
Open

Patchwork work#2149
SJuliez wants to merge 6 commits intoMegaMek:mainfrom
SJuliez:patchwork-work

Conversation

@SJuliez
Copy link
Member

@SJuliez SJuliez commented Mar 6, 2026

Some GUI changes for patchwork armor:

  • CV, ASF and BM now use the same armor setup in the right column: armor view, patchwork selector and armor allocation; other views have been moved away
  • as before, the patchwork selector is only shown when patchwork armor is selected
  • as the patchwork selector can have many locations to select for, some fields and buttons in the other views that are not used for patchwork armor are now hidden instead of disabled so that the total height stays more or less the same
  • fixes [Issue] Patchwork armor calculation based on head location armor type only #1803 (the wrong armor weight calculation is no longer shown; the armor weight in the summary is correct)
  • fixes: when an armor type is selected that doesnt fit, the armor type resetting to Standard is now actually visible
  • also some slight formatting changes, more field width for the armor tonnage, standard layout (right-aligned labels, same-size entry fields)

Also,

  • GUI code updates (GridBagLayout)
  • a help notice saying that for patchwork, armor weight is calculated from the points rather than the other way around. I found this not immediately obvious
image image image image

@SJuliez SJuliez requested a review from a team as a code owner March 6, 2026 15:33
@HammerGS HammerGS requested a review from Copilot March 6, 2026 15:36
Copy link
Contributor

Copilot AI left a 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 updates MegaMekLab’s armor UI to better support patchwork armor workflows across multiple unit types, including aligning panel placement, improving patchwork selection visibility, and clarifying patchwork-specific weight behavior (per #1803).

Changes:

  • Reorganizes armor-related panels (armor view, patchwork selector, allocation) for CV/ASF/BM and adjusts layout to keep heights consistent by hiding patchwork-irrelevant controls.
  • Improves patchwork selection UX by refreshing the patchwork selector when an ineligible armor choice is rejected.
  • Updates i18n strings and adds a patchwork help notice (“armor weight is calculated from the assigned points”).

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
megameklab/src/megameklab/ui/supportVehicle/SVArmorTab.java Patchwork rejection dialog parent + refresh patchwork selector after reset.
megameklab/src/megameklab/ui/mek/BMStructureTab.java Panel order tweak; patchwork rejection dialog parent + patchwork selector refresh.
megameklab/src/megameklab/ui/largeAero/WSStructureTab.java Removes no-op patchworkChanged override.
megameklab/src/megameklab/ui/largeAero/DSStructureTab.java Removes no-op patchworkChanged override.
megameklab/src/megameklab/ui/generalUnit/PatchworkArmorView.java Layout tweaks, filtering cleanup, prototype sizing, and minor refactor helpers.
megameklab/src/megameklab/ui/generalUnit/MVFArmorView.java Patchwork-mode UI hides irrelevant fields and shows patchwork info; i18n cleanup.
megameklab/src/megameklab/ui/generalUnit/ArmorAllocationView.java Uses display fields + i18n cleanup; hides non-patchwork-relevant controls in patchwork mode.
megameklab/src/megameklab/ui/fighterAero/ASStructureTab.java Moves transport panel; refreshes patchwork selector on rejection; uses setFromEntity(..., true) when refreshing armor view.
megameklab/src/megameklab/ui/combatVehicle/CVStructureTab.java Reorders panels to standardize armor UI placement.
megameklab/resources/megameklab/resources/Views.properties Updates labels and adds patchwork info string.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 1536 to 1543
if (getMek().getEmptyCriticalSlots(location) < crits) {
JOptionPane.showMessageDialog(
null, armor.getName()
+ " does not fit in location "
+ getMek().getLocationName(location)
+ ". Resetting to Standard Armor in this location.",
JOptionPane.showMessageDialog(this,
"%s does not fit in location %s. Resetting to Standard Armor in this location."
.formatted(armor.getName(), getMek().getLocationName(location)),
"Error",
JOptionPane.INFORMATION_MESSAGE);
UnitUtil.resetArmor(getMek(), location);
panPatchwork.setFromEntity(getMek());
} else {
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

After the failure path (getEmptyCriticalSlots < crits), the method calls UnitUtil.resetArmor(...) and then refreshes the UI. However, later in this method panArmor.setFromEntity(getMek()) (outside this hunk) may clear the patchwork checkbox / hide the patchwork UI when the entity no longer reports hasPatchworkArmor() after the reset. Other tabs call panArmor.setFromEntity(..., true) in similar situations to keep the user-selected patchwork mode stable. Consider updating the armor view refresh in this flow to use the ignoreEntityPatchwork=true overload so the reset-to-standard outcome is visible without exiting patchwork mode.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

this is valid; the patchwork checkbox should not become unchecked by selecting some armor type in a location (but it does happen), but this seemed like a complicated issue and I did not want to address it herein

Comment on lines +347 to +348
* @return True when the patchwork checkbox is selected; this is not equivalent to the unit having patchwork
* armor (entity.hasPatchworkArmor()); it only has when it there are actually two different armor types installed.
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

Javadoc for isPatchwork() has a couple grammar issues ("it only has when it there are actually...") and is a bit hard to parse. Suggest rephrasing so it clearly states that the checkbox state can differ from entity.hasPatchworkArmor() and under what condition hasPatchworkArmor() becomes true (multiple armor types installed).

Suggested change
* @return True when the patchwork checkbox is selected; this is not equivalent to the unit having patchwork
* armor (entity.hasPatchworkArmor()); it only has when it there are actually two different armor types installed.
* @return {@code true} if the patchwork checkbox is selected. This indicates only the current UI selection and
* may differ from whether the unit actually has patchwork armor
* ({@code entity.hasPatchworkArmor()}). The unit is considered to have patchwork armor only when
* multiple different armor types are installed.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

Choose a reason for hiding this comment

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

valid; changed the text


private void initUI() {
tooltipFormat = resourceMap.getString("ArmorAllocationView.locationTooltip.format");
txtUnallocated.setEditable(false);
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

Only txtUnallocated is explicitly made non-editable, but the other summary fields (txtAllocated, txtTotal, txtMaxPossible, txtWasted, txtPointsPerTon) were previously read-only too. Other views still call setEditable(false) on DisplayTextField, so if it’s editable by default these fields will now accept input that doesn’t do anything. Consider setting all of these display-only fields to setEditable(false) (and optionally setFocusable(false)) to preserve prior UX.

Suggested change
txtUnallocated.setEditable(false);
txtUnallocated.setEditable(false);
txtAllocated.setEditable(false);
txtTotal.setEditable(false);
txtMaxPossible.setEditable(false);
txtWasted.setEditable(false);
txtPointsPerTon.setEditable(false);

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@SJuliez SJuliez Mar 6, 2026

Choose a reason for hiding this comment

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

removed the setEditable; DisplayTextFields are uneditable by default

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue] Patchwork armor calculation based on head location armor type only

2 participants