Fix #8496: Fix MRMS carryover check being applied before work time modifiers#8519
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8519 +/- ##
============================================
- Coverage 12.32% 12.32% -0.01%
+ Complexity 7426 7425 -1
============================================
Files 1287 1287
Lines 165312 165312
Branches 24875 24875
============================================
- Hits 20376 20375 -1
+ Misses 142987 142983 -4
- Partials 1949 1954 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #8496 where the MRMS (MekHQ Repair and Maintenance System) carryover check was incorrectly applied before work time modifiers were calculated. This caused the system to not account for low-skill technicians needing extra time or high-skill technicians being able to rush repairs when determining if a repair would fit within a tech's remaining work time.
The fix moves the work time modifier calculation before the carryover check, ensuring accurate time estimates are used when deciding whether to assign repairs.
Key Changes
- Refactored work time calculation logic into reusable helper methods (
getWorkTime,getExpectedWorkTime,setPartWorkTime) - Modified carryover check to use expected work time that accounts for skill-based modifiers
- Added comprehensive test coverage for various carryover scenarios including low-skill techs needing extra time
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| MekHQ/src/mekhq/service/mrms/MRMSService.java | Refactored carryover check logic to calculate expected work time with modifiers before checking if tech has sufficient time; extracted helper methods for work time management |
| MekHQ/unittests/mekhq/service/mrms/MRMSServiceTest.java | Added new nested test class with 6 test cases covering carryover scenarios; updated breakArmor helper to properly simulate time consumption and added mockPartInventory setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fixes #8496
When doing MRMS, it was checking if a part's work would be more than a tech's time and cancel if the "carryover" is false. This check was done before work time modifiers, so it was not accounting for low skill techs that needed to use extra time, or high skill techs able to rush.