Skip to content

Fix #8848: Fix Large Vessel Temp Crew Issues#9088

Merged
IllianiBird merged 1 commit into
MegaMek:mainfrom
psikomonkie:issue/8848/large-vessel-temp-crew
May 25, 2026
Merged

Fix #8848: Fix Large Vessel Temp Crew Issues#9088
IllianiBird merged 1 commit into
MegaMek:mainfrom
psikomonkie:issue/8848/large-vessel-temp-crew

Conversation

@psikomonkie
Copy link
Copy Markdown
Member

Fixes #8848

Temp Crew is now more role-aware when determining how many more crew members a unit needs to ensure it is filled with the correct temp crew and not just pilots.

Also consolidates some code from Campaign to use HumanResources instead.

@psikomonkie psikomonkie requested a review from a team as a code owner May 25, 2026 02:27
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

❌ Patch coverage is 76.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.23%. Comparing base (969d837) to head (a361ba0).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
MekHQ/src/mekhq/campaign/HumanResources.java 76.19% 2 Missing and 3 partials ⚠️
MekHQ/src/mekhq/campaign/Campaign.java 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #9088      +/-   ##
============================================
+ Coverage     15.21%   15.23%   +0.01%     
- Complexity     9296     9305       +9     
============================================
  Files          1304     1304              
  Lines        172561   172518      -43     
  Branches      26080    26063      -17     
============================================
+ Hits          26254    26277      +23     
+ Misses       143260   143198      -62     
+ Partials       3047     3043       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
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.

Couple of questions before I merge this

VESSEL_PILOT -> unit.getDriverRole() == role;
case VESSEL_GUNNER -> unit.getGunnerRole() == role;
case VESSEL_CREW -> unit.canTakeMoreVesselCrew();
case VESSEL_CREW -> (unit.getEntity() instanceof Aero aero && !(aero instanceof ConvFighter))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was going to say there was a potential NPE here, but remembered that instanceof includes null protection. Man, that tiny piece of QoL makes me so happy.

*
* @return available slots (negative = surplus temp crew)
*/
private int getRoleSpecificNeeds(Unit unit, PersonnelRole role) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not seeing Navigator, and I believe JumpShips and WarShips both count as Aero (could be wrong)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There are no temp navigators.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's all I needed to hear :)

doReturn(new HashSet<>(activeCrew)).when(unit).getGunners();
doReturn(crewSize).when(unit).getTotalGunnerNeeds();
}
case VESSEL_CREW -> {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you do need add a case for navigator elsewhere, might be worth adding a cheeky addition to your test here

@IllianiBird IllianiBird merged commit 8a792e7 into MegaMek:main May 25, 2026
8 checks passed
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] Large Vessel Temporary Crew Bug

2 participants