Skip to content

Implement Obsolete quirk effects for campaign play#8474

Merged
HammerGS merged 10 commits intomainfrom
feature/obsolete-quirk-campaign-effects
Dec 15, 2025
Merged

Implement Obsolete quirk effects for campaign play#8474
HammerGS merged 10 commits intomainfrom
feature/obsolete-quirk-campaign-effects

Conversation

@HammerGS
Copy link
Member

@HammerGS HammerGS commented Dec 12, 2025

PR Notes

Summary

Implements campaign effects for the Obsolete quirk from BattleMech Manual pg 86.

Units with the Obsolete quirk now suffer penalties in MekHQ campaigns:

  • Maintenance rolls: +1 TN per 15 years obsolete (max +5)
  • Repair rolls: +1 TN per 15 years obsolete (max +5)
  • Resale value: -10% per 20 years obsolete (min 50%)

Changes

Maintenance & Repair TN Modifiers

  • Maintenance.java: Added obsolete modifier to getTargetForMaintenance()
  • Part.java: Added obsolete modifier to getAllMods() (repair work) and getAllModsForMaintenance()

Resale Value

  • Unit.java: Applied entity.getObsoleteResaleModifier() in getSellValue() for both standard units and Infantry

Internationalization

  • Maintenance.properties: Added Maintenance.modifier.obsolete
  • Parts.properties: Added Part.modifier.obsolete

Dependencies

Test Plan

  • Load campaign with unit that has Obsolete quirk set
  • Verify maintenance rolls show "obsolete" modifier in TN breakdown
  • Verify repair rolls show "obsolete" modifier in TN breakdown
  • Verify unit sell value is reduced based on years obsolete
  • Test with Infantry units (uses different sell value calculation)

  Adds campaign effects for the Obsolete quirk per BMM pg 86:
  - Maintenance rolls: +1 to +5 TN modifier based on years obsolete
  - Repair rolls: +1 to +5 TN modifier based on years obsolete
  - Resale value: 50%-100% multiplier based on years obsolete

  Changes:
  - Maintenance.java: Added obsolete modifier to getTargetForMaintenance()
  - Part.java: Added obsolete modifier to getAllMods() and
    getAllModsForMaintenance()
  - Unit.java: Applied obsolete resale modifier in getSellValue()
  - Added i18n keys for modifier descriptions

  ---
  PR Notes

  ## Summary
  Implements campaign effects for the Obsolete quirk from BattleMech Manual pg 86.

  Units with the Obsolete quirk now suffer penalties in MekHQ campaigns:
  - **Maintenance rolls**: +1 TN per 15 years obsolete (max +5)
  - **Repair rolls**: +1 TN per 15 years obsolete (max +5)
  - **Resale value**: -10% per 20 years obsolete (min 50%)

  ## Changes

  ### Maintenance & Repair TN Modifiers
  - **Maintenance.java**: Added obsolete modifier to `getTargetForMaintenance()`
  - **Part.java**: Added obsolete modifier to `getAllMods()` (repair work) and `getAllModsForMaintenance()`

  ### Resale Value
  - **Unit.java**: Applied `entity.getObsoleteResaleModifier()` in `getSellValue()` for both standard units and
  Infantry

  ### Internationalization
  - **Maintenance.properties**: Added `Maintenance.modifier.obsolete`
  - **Parts.properties**: Added `Part.modifier.obsolete`

  ## Dependencies
  - Requires MegaMek PR with Obsolete quirk implementation (provides `Entity.getObsoleteRepairModifier()` and
  `Entity.getObsoleteResaleModifier()`)

  ## Test Plan
  - [ ] Load campaign with unit that has Obsolete quirk set
  - [ ] Verify maintenance rolls show "obsolete" modifier in TN breakdown
  - [ ] Verify repair rolls show "obsolete" modifier in TN breakdown
  - [ ] Verify unit sell value is reduced based on years obsolete
  - [ ] Test with Infantry units (uses different sell value calculation)
@HammerGS HammerGS requested a review from a team as a code owner December 12, 2025 17:57
Copilot AI review requested due to automatic review settings December 12, 2025 17:57
@HammerGS HammerGS added Draft Work in Progress AI Generated Code AI-generated fix. Requires human testing and review before merging. labels Dec 12, 2025
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 implements campaign effects for the Obsolete quirk from BattleMech Manual pg 86. Units with the Obsolete quirk now suffer time-based penalties affecting maintenance difficulty, repair difficulty, and resale value. The implementation adds modifier calculations at three key points: maintenance operations, repair operations, and unit sales.

  • Adds obsolete quirk modifiers to maintenance and repair target numbers based on how many years past obsolescence
  • Applies resale value reduction multiplier to both Infantry and standard units
  • Adds internationalization strings for the obsolete modifier labels

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
MekHQ/src/mekhq/campaign/unit/Unit.java Applies obsolete resale modifier to both Infantry and standard unit sell value calculations
MekHQ/src/mekhq/campaign/unit/Maintenance.java Adds obsolete modifier to maintenance target rolls (contains duplicate application bug)
MekHQ/src/mekhq/campaign/parts/Part.java Adds obsolete modifier to both repair (getAllMods) and maintenance (getAllModsForMaintenance) target rolls
MekHQ/resources/mekhq/resources/Parts.properties Adds "obsolete" modifier description string
MekHQ/resources/mekhq/resources/Maintenance.properties Adds "obsolete" modifier description string

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

 Summary of MekHQ Fixes (PR #8474):

  | Issue                 | File                     | Fix
                                                 |
  |-----------------------|--------------------------|--------------------------------------------------------------
  -----------------------------------------------|
  | 1. Duplicate modifier | Maintenance.java:423-432 | Removed duplicate obsolete quirk modifier block (already
  applied in Part.getAllModsForMaintenance())        |
  | 2. Null pointer risk  | Part.java:973-987        | Added if (getUnit().getEntity() != null) wrapper around
  entity-dependent code in getAllMods()               |
  | 3. Null pointer risk  | Part.java:1041-1054      | Added if (getUnit().getEntity() != null) wrapper around
  entity-dependent code in getAllModsForMaintenance() |
  | 4. Null pointer risk  | Unit.java:1561-1570      | Added if (entity != null) wrapper around getPriceMultiplier()
   and obsolete resale modifier code             |
  | 5. Naming convention  | Unit.java:1472,1566      | Renamed obsoleteMult to obsoleteMultiplier (2 instances)
  The dialog will now show:

  Buy New: 6,880,705 C-bills
  Quality (D): x0.70
  Current Worth: 4,500,000 C-bills
  Obsolete (25 years): x0.75
  -----------------
  Sell For: 3,375,000 C-bills

  What each line means:
  - Buy New: Full construction cost from MegaMek (includes weight multiplier, quirk multipliers)
  - Quality: The part quality and its price multiplier from campaign options
  - Current Worth: Value based on parts summed with quality applied, before obsolete
  - Obsolete: Only shown if the unit has the obsolete quirk (shows years and multiplier)
  - Sell For: Final sale price

  Changes made:
  1. Removed "Entity Multiplier" from the display (it's baked into the calculations, not useful to show separately)
  2. Added "Buy New" price using getBuyCost() which calls MegaMek's Entity.getCost()
  3. Renamed "Final value" to "Sell For" for clarity
  4. Added "Current Worth" as an intermediate value
  5. Created helper method getSellValueBeforeObsolete() to calculate worth before obsolete modifier
@HammerGS HammerGS removed the Draft Work in Progress label Dec 14, 2025
Copy link
Collaborator

@IllianiBird IllianiBird left a comment

Choose a reason for hiding this comment

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

There is a little more work that needs to be done before I can approve this PR. It's 99% there, the actual functionality is fine insofar as I can see there are just some variable name issues and hardcoded values I'd like to see us move away from.

  - Rename 'js' to 'jumpShip' and 'wType' to 'weaponType' for clarity
  - Move getSellValueBreakdown() strings to Unit.properties resource bundle
  - Cache campaign.getCampaignOptions() call in getSellValueBreakdown()
  - Document spacecraft pricing magic numbers with TM pg 274-284 reference
  - Replace JOptionPane with ImmersiveDialogSimple for sell unit dialog
  - Add sell unit dialog strings to GUI.properties resource bundle
  - Use HTML formatting (<br>, <hr>) for GM mode breakdown display

  Note: Null checks and variable naming (obsoleteMultiplier) were already
  in place from the original implementation.
HammerGS and others added 3 commits December 14, 2025 18:52
  sellUnit.message={0}, I''ve received your authorization to liquidate the {1}...
  sellUnit.messageGM={0}, I''ve received your authorization to liquidate the {1}... (with breakdown)
  sellUnit.buttonConfirm=Say the word and I'll finalize the paperwork
  sellUnit.buttonCancel=Hold off on that

  UnitTableMouseAdapter.java - Updated to:
  - Use getSeniorAdminPerson(LOGISTICS) as the speaker
  - Use getFormattedTextAt() for proper i18n
  - Use campaign.getCommanderAddress() for the {0} placeholder
  - Simplified button list with List.of()
…hen X is clicked, it stays at 0 (confirm). The simplest fix is to swap the button order so Cancel is first (index 0, the default), and Confirm is second (index 1).
@HammerGS HammerGS merged commit 4d2fd86 into main Dec 15, 2025
6 checks passed
@HammerGS HammerGS deleted the feature/obsolete-quirk-campaign-effects branch December 15, 2025 03:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI Generated Code AI-generated fix. Requires human testing and review before merging.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants