Add processing of multiple fluff images (continues #5490) - #8664
Draft
HammerGS wants to merge 30 commits into
Draft
Add processing of multiple fluff images (continues #5490)#8664HammerGS wants to merge 30 commits into
HammerGS wants to merge 30 commits into
Conversation
MegaMek + Fix 5476: Update SV armor slots to 2 or 1 for Rating E or F respectively MegaMekLab + Fix ##1514: Fix Atrocious table shading render speed MekHQ + PR #4106: Use tabs in customize scenario dialog + PR #4105: Updated Post-Scenario Logging for Prisoners + PR #4104: Fixed Post-Scenario Tracking System's Handling of Multiple Personnel in autoAwards + PR #4102: Fixed Award Tier Count Calculations in PersonViewPanel + PR #4093: Added Negotiation and Scrounge Skill Settings for Administrator Personnel + PR #4054: Added Life Paths Campaign Options Tab, Added Education Module Very Important to read the documentation on this feature (See Docs folder)
# Conflicts: # megamek/docs/history.txt
# Conflicts: # megamek/data/mechfiles/vehicles/3145/Davion/Destrier Siege Vehicle.blk # megamek/src/megamek/client/ui/swing/MekViewPanel.java # megamek/src/megamek/client/ui/swing/util/FluffImageHelper.java
# Conflicts: # megamek/docs/AI and MegaMek Suite (Not Princess Stuff).txt # megamek/docs/history.txt # megamek/src/megamek/client/ui/FluffImageTooltip.java # megamek/src/megamek/client/ui/swing/MekViewPanel.java # megamek/src/megamek/client/ui/util/FluffImageHelper.java
- Fix getExtension variable bug (filename vs f) - Remove test code: use get(0) instead of random image selection - Fix double setText on imageInfoLabel (removed redundant call) - Add null safety to FluffImageRecord.getImage() for null file - Add null parent check in FluffImageTooltip.getYamlFile() - Add Files.walk depth limits for performance - Fix JavaDoc: return descriptions now match actual behavior - Remove commented-out code blocks - Remove unused prepareLabelText method - Fix path construction for PLACEHOLDER_IMAGE_NAME - Add bounds checks in isSuitableYamlFile()
Brings the multi-fluff branch up to date with main (2430 commits). Conflict resolved in FluffImageHelper: main had reworked the single-result findFluffFile() to search an ordered list of fluff directories via getFluffPaths(), so Battlefield Support Assets can fall back to the folder of their corresponding TW unit type. The branch had reworked the same method into a multi-result findFluffFiles() that also searches chassis/model subdirectories. Both are kept: findFluffFiles() now runs the branch's chassis-directory search inside main's per-fluff-path loop, for the user dir and the internal dir alike, and keeps main's hud.png record-sheet fallback as the last candidate. The candidate set is a LinkedHashSet rather than a HashSet so the most-specific-art-first ordering that both sides rely on is preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…uilds The multi-fluff work changed this public constant from String[] to List<String> to get a contains() check for the new chassis-directory scan. MegaMekLab calls Arrays.stream() on it in FluffTab and IconView, so the type swap breaks the MML build, and MegaMek's deprecation policy does not allow silently changing a public API that MML and MekHQ consume. Restore the constant to main's String[] and derive a private Set for the membership test instead, which also makes the lookup O(1). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Style and diagnosability pass over the fluff image work, no intended behaviour
change beyond the three fixes noted at the end.
- Externalise the user-facing strings ("Unit:", "Artist:", "Insignia:", the
image load error) into messages.properties, and give the browse buttons
localized tooltips. The separating space after each label is added in code so
the translated strings do not need a trailing-space escape.
- Give FluffImageTooltip the current full MegaMek license header and correct its
class javadoc, which still referred to the long-renamed MechViewPanel.
- Replace log4j LogManager with MMLogger in FluffImageTooltip and
FluffImageHelper, and add [FluffImages] diagnostic logging where failures were
previously silent: an unreadable fluff image, a missing placeholder image and
a unit with no fluff at all.
- Rename abbreviated identifiers: catch parameters e/ex to exception, lambda
parameters e/p to event/entry/candidate, and Box p to readoutAndFluffPanel.
- yamlMapper becomes YAML_MAPPER, and the yaml suffix, tooltip width and node
names become named constants.
- Scale the gap under the fluff image with UIUtil.scaleForGUI, and use
CENTER_ALIGNMENT/TOP_ALIGNMENT instead of raw 0.5f/0.
- Replace the wildcard java.util and java.awt imports with explicit ones and
drop the now-unused BoxLayout and FixedXPanel imports.
- Add @nullable and javadoc (including @PARAM tags on FluffImageRecord) where
null is a legitimate value, and wrap javadoc literals in {@code ...}.
Three real fixes came out of the pass:
- reset() cleared the leftover fluffImageComponent field rather than the label
actually on screen, so resetting the panel never cleared the fluff image. That
field is now removed and reset() clears the image, the info line, the record
list and the browse buttons.
- Selecting a unit with sprites-only enabled, or with no unit at all, left the
browse buttons enabled from the previously shown unit.
- changeFluffImageIndex built the tooltip twice per image change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
HammerGS
added
AI Assisted Development
Contributor
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds support for multiple fluff images per unit, including directory-based discovery rules and UI controls to page through available images, plus optional YAML sidecar metadata for artist/title/insignia.
Changes:
- Update fluff image discovery to return multiple candidates (including chassis/model subfolders) and preserve deterministic selection order.
- Add
EntityReadoutPanelUI controls (</>) to browse images and show metadata under the image. - Introduce
FluffImageTooltipto parse*data.yamlsidecar files and render localized tooltip content.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| megamek/src/megamek/client/ui/util/FluffImageHelper.java | Adds multi-result fluff search, chassis/model directory scanning, and a lazy-loading FluffImageRecord. |
| megamek/src/megamek/client/ui/dialogs/unitSelectorDialogs/EntityReadoutPanel.java | Adds image browsing controls, placeholder handling, and wiring for tooltip/metadata display. |
| megamek/src/megamek/client/ui/FluffImageTooltip.java | New YAML sidecar parser to generate localized HTML tooltip content for fluff images. |
| megamek/resources/megamek/client/messages.properties | Adds localization keys for new readout panel tooltips/errors and tooltip labels. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- The readout panel opened on the second fluff image. setFluffImage() reset the index to 0 and then called showNextFluffImage(), which steps by one before displaying, so a unit with two or more images showed image 2 first and the "<" button went back to image 1. Single-image units wrapped around to 0 and looked correct, which is why it went unnoticed. Now shows the first image. - Tighten the chassis and model directory checks from exists() to isDirectory(), so a stray file named like a chassis is not scanned as a directory. - Use Files.list instead of Files.walk(dir, 1) for the shallow directory scan, which also drops the directory entry itself and the Objects::toString and File::new round trip. - Sort the images found in a chassis or model directory by file name. Neither Files.list nor Files.walk guarantees an order, so without this the image a unit shows first was down to the file system. - Pad the tooltip style colour to six hex digits. A dark tooltip foreground colour such as pure blue produced "ff", which is not a valid CSS colour, so the label text fell back to the default colour. - Extract the "---empty---" model directory name into a constant. Adds FluffImageHelperChassisDirTest covering the chassis and model subdirectory rules: model directory wins over chassis directory, the chassis directory is used when no model directory matches, empty models match the ---empty--- directory, clan chassis variant directories resolve, non-image files and stray files named like a chassis are ignored, and repeated calls return the same deterministic order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This continues @SJuliez's work in #5490, rebuilt on current
main. That PR had been open sinceMay 2024 and had fallen 2430 commits behind, so this is the same feature brought up to date with
the conflict resolved.
What it does for players: a unit can now have more than one fluff image, and the unit readout
panel gets
</>buttons to page through them. Where fluff images are stored also becomes moreflexible:
fluff/Mek/Atlas AS7-D.pngstill matches that exact model, andfluff/Mek/Atlas.pngstill matches any Atlas. Unchanged by this PR.fluff/Mek/Atlas/matches any Atlas, whatever the files arecalled.
fluff/Mek/Atlas/Atlas AS7-D/matches only that model. A unitwith an empty model matches the folder
---empty---.Worked example: drop three images into
data/images/fluff/Mek/Atlas/. Open any Atlas in the unitselector and all three are available, browsable with the
<and>buttons, instead of only thesingle image that would previously have matched.
If a fluff image ships alongside a
*data.yamlfile (the convention used by the painted-minis imagepacks), the panel reads
title,authorand aninsigniaentry from it and shows them under theimage, so artists get credited in-app.
Relationship to #5490
Same feature, same author's commits (history preserved). Opened as a new PR because #5490 lives on
a fork branch that had drifted too far to update in place. #5490 can be closed in favour of this
once maintainers are happy with the approach.
What changed relative to #5490
main(2430 commits).FluffImageHelper. Both sides had rewritten the same searchmethod for different reasons.
mainhad turnedfindFluffFile()into a loop overgetFluffPaths()so Battlefield Support Assets search their ownAssetfolder and then fallback to the folder of their corresponding TW unit type; Add processing of multiple fluff images (WIP) #5490 had turned it into a multi-result
findFluffFiles()that also searches chassis/model subfolders. Both behaviours are kept: thechassis-folder search now runs inside the per-fluff-path loop, for the user dir and the internal
dir alike, and
main'shud.pngrecord-sheet fallback stays last.LinkedHashSetfor the candidate set instead of theHashSetin Add processing of multiple fluff images (WIP) #5490. Both sidesdeliberately order candidates most-specific-first, and
getFluffImage()returns the first entry— a plain
HashSetwould silently randomise which image a unit shows.EXTENSIONS_FLUFF_IMAGE_FORMATStoString[]. Add processing of multiple fluff images (WIP) #5490 changed this public constantto
List<String>, which breaks the MegaMekLab build (Arrays.stream(...)on it inFluffTaband
IconView) and would be a silent public-API change for a constant MML and MekHQ consume.The internal membership test now uses a private
Setderived from the array.Files Changed
megamek/client/ui/util/FluffImageHelper.java— multi-result fluff search, chassis/modelsubfolder lookup,
FluffImageRecord, merged withmain's asset-folder search.megamek/client/ui/dialogs/unitSelectorDialogs/EntityReadoutPanel.java—</>browsing,info label under the image, placeholder for units with no fluff.
megamek/client/ui/FluffImageTooltip.java— new; reads the*data.yamlsidecar files.Testing
./gradlew :megamek:compileJavaand:megamek:compileTestJavagreen.FluffImageHelperAssetTestpasses — this ismain's test for the Battlefield Support Assetfolder fallback, i.e. exactly the behaviour the conflict resolution had to preserve.
checkstyleMaingreen (re-run after the checklist pass).EXTENSIONS_FLUFF_IMAGE_FORMATSconstant is now byte-identical tomain,so MegaMekLab is unaffected.
Review checklist pass
The WIP items carried over from #5490 have now been addressed (commit
48faefe):messages.properties(Unit:,Artist:,Insignia:, the imageload error), and the browse buttons gained localized tooltips. The separating space after each
label is added in code so translators do not have to carry a trailing-space escape.
FluffImageTooltipgiven the current full MegaMek license header; its class javadoc no longerrefers to the long-renamed
MechViewPanel.MMLoggerreplaces log4jLogManager, with[FluffImages]diagnostic logging on the paths thatpreviously failed silently: an unreadable fluff image, a missing placeholder, and a unit with no
fluff at all.
catch (… e/ex)toexception, lambdae/ptoevent/entry/candidate,Box ptoreadoutAndFluffPanel.yamlMappertoYAML_MAPPER; the yaml suffix, tooltip width and yaml node names are now namedconstants.
UIUtil.scaleForGUI(...)on the gap under the image;CENTER_ALIGNMENT/TOP_ALIGNMENTinsteadof raw
0.5f/0.java.util.*andjava.awt.*imports replaced with explicit ones; the now-unusedBoxLayoutandFixedXPanelimports dropped.@Nullableand javadoc added wherenullis legitimate, including@paramtags onFluffImageRecord; javadoc literals wrapped in{@code ...}.Three genuine bugs surfaced during that pass and are fixed in the same commit:
reset()never cleared the fluff image. It cleared the leftoverfluffImageComponentfieldrather than the label actually on screen. That field is removed, and
reset()now clears theimage, the info line, the record list and the browse buttons.
left the
</>buttons enabled from the previously shown unit.changeFluffImageIndex.Documentation defect found while setting up test data
The class javadoc on
getFluffInChassisDirsstates that in the mainfluff/[unittype]/folder"the filename may now contain additional information after an underscore (atlas_xyz.jpg matches for
any Atlas mek)". That behaviour does not exist. The main folder is matched by
findMatchingFiles, which doesfile.getName().equalsIgnoreCase(nameCandidate + extension)- anexact match, with no underscore or prefix handling anywhere in the class. The claim appears to
describe an intention that was never implemented. It needs either implementing or removing from the
javadoc before this merges; an earlier revision of this description repeated the claim and has been
corrected.
What is NOT proven yet
exercised the actual fluff browsing, so the
</>buttons, the placeholder image and theyaml info label are unverified in the running client.
*data.yamltooltip path is entirely unexercised — it needs one of the painted-minisimage packs present to do anything, and no test or manual run has confirmed it parses.
fluff_placeholder.pngis referenced but not shipped in this PR; units with no fluff imagewill fall back to a null image until that asset is added to
mm-data.getRecordSheetFluffImage()nowgoes through the multi-result path and returns the first candidate; the ordering is preserved by
the
LinkedHashSet, but no record sheet has actually been printed to confirm it.the feature and deserve coverage before this is merged.
Continues #5490. Original work by @SJuliez.