Buildings - #2328
Open
exeea wants to merge 18 commits into
Open
Conversation
exeea
commented
Sep 11, 2026
|
|
||
| private static final Map<SmallCraft, CrewConfiguration> AUTO_FILLED_CREW = new WeakHashMap<>(); | ||
| // Entity equality uses mutable game IDs; separate unassigned craft all have ID -1. | ||
| private static final Map<SmallCraft, CrewConfiguration> AUTO_FILLED_CREW = |
Collaborator
Author
There was a problem hiding this comment.
this is a mini fix unrelated
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| private Element element(Element parent, String name, String... attributes) { | ||
| Element element = getSVGDocument().createElementNS(svgNS, name); | ||
| for (int i = 0; i < attributes.length; i += 2) { | ||
| element.setAttribute(attributes[i], attributes[i + 1]); |
| } | ||
|
|
||
| static int floor(AbstractBuildingEntity entity, String text) { | ||
| return Math.toIntExact(("Ground".equals(text) ? 0 : Long.parseLong(text)) - BuildingConstruction.baseLevel(entity)); |
| assertEquals(List.of("7", "6", "5", "4", "3", "2"), elements(sheet, "g", "building-map-layer").stream() | ||
| .map(e -> e.getAttribute("data-building-floor")).toList()); | ||
| assertEquals(24, elements(sheet, "g", "building-inventory-entry").stream() | ||
| .filter(row -> Integer.parseInt(row.getAttribute("data-location")) >= 0).count()); |
Copilot stopped reviewing on behalf of
exeea due to an error
September 11, 2026 13:59
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Pull request overview
Adds full Building/Mobile Structure support across the editor, printing pipeline, and verification logic, plus extensive new test coverage for building geometry/services and record sheet output.
Changes:
- Introduce Building editor UI (structure/placement/equipment/transport/systems) and integrate it into menus and UI loader.
- Add building record sheet printing (map layers, inventory pagination, doors/elevators/features) and include buildings in print queue.
- Fix/adjust related mechanics and printing behaviors (capital bay AV rounding, infantry crew-operated range mods, bay ammo selection, crew autofill identity), with new unit/UI/printing tests and fixture BLKs.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| megameklab/unittests/megameklab/util/UnitUtilTest.java | Adds targeted regression tests for building armor capacity and crew identity behavior. |
| megameklab/unittests/megameklab/util/BuildingUtilTest.java | Adds unit tests for building geometry edits, placement round-trips, and labels. |
| megameklab/unittests/megameklab/ui/building/BuildingMainUITest.java | Adds Swing-level integration tests covering building editor workflows and undo/redo. |
| megameklab/unittests/megameklab/printing/WeaponBayDamageTest.java | Verifies capital bay damage rounding after fractional contributions. |
| megameklab/unittests/megameklab/printing/PrintInfantryRangeTest.java | Tests crew-operated/underwater range modifier printing behavior. |
| megameklab/unittests/megameklab/printing/PrintBuildingTest.java | Adds record-sheet rendering/layout tests for buildings (layers, features, paging). |
| megameklab/unittests/megameklab/printing/InventoryWriterBayAmmoTest.java | Tests bay ammo association now sourced from the bay itself. |
| megameklab/testresources/Longinus metadata lifecycle.blk | Adds metadata-lifecycle fixture for BattleArmor. |
| megameklab/testresources/Dragonstar metadata lifecycle.blk | Adds metadata-lifecycle fixture for SmallCraft. |
| megameklab/src/megameklab/util/UnitUtil.java | Adds building handling for max armor points, verifier selection, and editor-type resolution. |
| megameklab/src/megameklab/util/UnitPrintManager.java | Adds PrintBuilding sheet creation for building entities. |
| megameklab/src/megameklab/util/CConfig.java | Adds window-position persistence key for BuildingMainUI. |
| megameklab/src/megameklab/util/BuildingUtil.java | New shared building construction/coordinate/utilities used by UI and printing. |
| megameklab/src/megameklab/util/BuildingMap.java | New shared “feature” model for map rendering and printed structure maps. |
| megameklab/src/megameklab/util/AeroUtil.java | Fixes auto-filled crew tracking via identity-based weak keys. |
| megameklab/src/megameklab/ui/util/EquipmentDatabaseCategory.java | Expands category visibility rules to support building equipment use-cases. |
| megameklab/src/megameklab/ui/generalUnit/TransportTab.java | Keeps building bay placement metadata consistent when bays are rebuilt/resized. |
| megameklab/src/megameklab/ui/dialog/UiLoader.java | Routes building entity types to BuildingMainUI construction editor. |
| megameklab/src/megameklab/ui/building/BuildingTransportTab.java | New building-specific transport + quarters UI with placement dialogs. |
| megameklab/src/megameklab/ui/building/BuildingSystemsTab.java | New construction options/service access UI plus always-on validation report. |
| megameklab/src/megameklab/ui/building/BuildingStructureTab.java | New footprint editor (top/pancake), geometry transforms, fuel/levels controls. |
| megameklab/src/megameklab/ui/building/BuildingPlacementDialogs.java | New placement editors for equipment distribution, bays, doors, and elevators. |
| megameklab/src/megameklab/ui/building/BuildingMainUI.java | New main Building editor container and refresh/location-selection logic. |
| megameklab/src/megameklab/ui/building/BuildingEquipmentTab.java | New building equipment placement UI + filtered equipment database view. |
| megameklab/src/megameklab/ui/StartupGUI.java | Adds “New Building” entry to startup menu. |
| megameklab/src/megameklab/ui/MenuBar.java | Adds “Building” to File → New menu. |
| megameklab/src/megameklab/ui/MegaMekLabTabbedUI.java | Adds new-tab creation entries for Building and Mobile Structure. |
| megameklab/src/megameklab/printing/WeaponBayInventoryEntry.java | Fixes capital bay AV formatting to round correctly. |
| megameklab/src/megameklab/printing/SVGMassPrinter.java | Adjusts export toggles and refactors UnitData metadata read timing. |
| megameklab/src/megameklab/printing/PrintInfantry.java | Adjusts range modifier logic to account for crew-operated weapons. |
| megameklab/src/megameklab/printing/PrintBuilding.java | New building record sheet implementation (maps, inventory, services, paging). |
| megameklab/src/megameklab/printing/InventoryWriter.java | Changes bay ammo association and exposes constants/helpers for planning pages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+100
to
+107
| public static int exteriorFacing(AbstractBuildingEntity entity, CubeCoords hex) { | ||
| for (int side = 0; side < 6; side++) { | ||
| if (!entity.getInternalBuilding().getOriginalCoordsList().contains(hex.toOffset().translated(side).toCube())) { | ||
| return side; | ||
| } | ||
| } | ||
| return 0; | ||
| } |
Comment on lines
+744
to
+750
| Path output = Path.of("build", "building-review", name + ".png"); | ||
| Files.createDirectories(output.getParent()); | ||
| ImageIO.write(image, "png", output.toFile()); | ||
| } catch (Exception exception) { | ||
| fail(exception); | ||
| } finally { | ||
| editor.removeNotify(); |
IllianiBird
requested changes
Sep 11, 2026
IllianiBird
left a comment
IllianiBird
left a comment
Collaborator
There was a problem hiding this comment.
Mostly approved, but there are some areas of concern. The chief among them is the mass use of hardcoded player facing Strings. These should be stored in a RESOURCE_BUNDLE and accessed with whatever the mml equivalent of getTextAt is.
Comment on lines
+481
to
+485
| for (WeaponMounted bay : weapons) { | ||
| WeaponBayText wbt = new WeaponBayText(bay.getLocation(), bay.isRearMounted()); | ||
| for (WeaponMounted weaponMounted : bay.getBayWeapons()) { | ||
| if (!wbt.addBayWeapon(weaponMounted)) {continue;} | ||
| for (AmmoMounted ammo : ammoMountedList) { | ||
| if (ammo.getLocation() == weaponMounted.getLocation() | ||
| && weaponMounted.getType().getAmmoType() == ammo.getType().getAmmoType()) { | ||
| for (AmmoMounted ammo : bay.getBayAmmo()) { |
Collaborator
There was a problem hiding this comment.
Is there a way for us to avoid nesting a loop within a loop within a loop?
| @Override | ||
| protected String getRecordSheetTitle() { | ||
| if (building instanceof MobileStructure) { | ||
| return "Mobile Structure Record Sheet"; |
Collaborator
There was a problem hiding this comment.
Hardcoded player facing strings should be avoided
| BuildingSystemsTab(BuildingMainUI editor) { | ||
| this.editor = editor; | ||
| setLayout(new BorderLayout(8, 8)); | ||
| setBorder(BorderFactory.createEmptyBorder(8, 8, 8, 8)); |
Collaborator
There was a problem hiding this comment.
These should be using scaleForGUI. We should avoid hardcoding gui sizes as it won't react to user changes to the GUI Scale client option
| break; | ||
| } | ||
| int index = column * rows + row; | ||
| if (row >= rows || index >= protection.size()) { |
Member
Review: MegaMekLab PR #2328 "Buildings" (exeea/buildings -> main)Reviewed at head 56999c7 (merge-base = current main 44081e5). 32 files, +5,909 / -50. Depends on MegaMek Verdict: NOT MERGEABLE as-is. Blocked on #8946, on red CI of its own, and on two hard gates (no i18n, Swing A. Blockers
B. Style and hygiene
C. Automated review triage
D. Printing pipeline notes
E. PR body and process
F. What is done well (keep)
|
Member
Follow-up: duplicated logic and hex namingItems that touch this repo, following the duplication pass on MegaMek #8946.
|
Member
Recommendation: resolving the duplication (this repo's share)Full plan is on MegaMek #8946. The parts that land here:
|
| } | ||
| // we matched all | ||
| return true; | ||
| Map<WeaponType, Map<EquipmentType, Integer>> augmentations = new HashMap<>(); |
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.
This PR adds Buildings and Mobile Buildings construction
depends on MegaMek/megamek#8946