Skip to content

Fix #8504: Fixed DropShip Force Experience Rating Contributions#8505

Merged
HammerGS merged 3 commits intoMegaMek:mainfrom
IllianiBird:dropShipExperienceContributionFix
Dec 15, 2025
Merged

Fix #8504: Fixed DropShip Force Experience Rating Contributions#8505
HammerGS merged 3 commits intoMegaMek:mainfrom
IllianiBird:dropShipExperienceContributionFix

Conversation

@IllianiBird
Copy link
Collaborator

Fix #8504

DropShips (and Small Craft) don't have commanders with both gunnery & piloting skills. That meant that the unit's experience rating contributions would be calculated as if the unit either had no gunnery, or no piloting. This resulting in the unit's contributions having a negative effect on Reputation.

Now we take an average of the piloting skills across all pilots, and the gunnery skill across all gunners.

@IllianiBird IllianiBird self-assigned this Dec 14, 2025
@IllianiBird IllianiBird requested a review from a team as a code owner December 14, 2025 02:24
@IllianiBird IllianiBird added Bug Unit Rating Severity: Medium Issues described as medium severity as per the new issue form labels Dec 14, 2025
@codecov
Copy link

codecov bot commented Dec 14, 2025

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 12.36%. Comparing base (b89f865) to head (9a3894a).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
...aign/camOpsReputation/AverageExperienceRating.java 92.85% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8505      +/-   ##
============================================
+ Coverage     12.32%   12.36%   +0.04%     
- Complexity     7425     7448      +23     
============================================
  Files          1287     1287              
  Lines        165312   165359      +47     
  Branches      24875    24886      +11     
============================================
+ Hits          20375    20453      +78     
+ Misses       142983   142950      -33     
- Partials       1954     1956       +2     

☔ 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
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 fixes issue #8504 where DropShips and Small Craft were incorrectly contributing to Force Experience Rating calculations. Previously, these unit types would use commander-only skills, which resulted in missing gunnery or piloting skills and negative reputation effects. The fix introduces averaging logic that calculates the mean piloting skill across all drivers and the mean gunnery skill across all gunners for SmallCraft and DropShip units.

Key Changes

  • Added special handling for SmallCraft (including DropShips) to average piloting skills across all drivers and gunnery skills across all gunners
  • Moved commander-based skill calculation into an else branch for non-SmallCraft units
  • Added necessary imports for List, Set, and SmallCraft classes

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

Copy link
Member

@psikomonkie psikomonkie left a comment

Choose a reason for hiding this comment

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

Nice! One comment.

Comment on lines +170 to +177
double totalGunneryTargetNumbers = 0;
Set<Person> gunners = unit.getGunners();
for (Person gunner : gunners) {
SkillModifierData skillModifierData = gunner.getSkillModifierData(true);
totalGunneryTargetNumbers += getSkillTargetNumber(gunner, entity, skillModifierData, false);
}
int gunnerCount = gunners.size();
gunneryTargetNumber = gunnerCount == 0 ? 0 : (int) round(totalGunneryTargetNumbers / gunnerCount);
Copy link
Member

Choose a reason for hiding this comment

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

But what if (tbh writing it out I don't love it)

    Set<Person> gunners = unit.getGunners();
    gunneryTargetNumber = (int) round(gunners.stream()
                                            .mapToInt(g -> getSkillTargetNumber(g,
                                                  entity,
                                                  g.getSkillModifierData(true),
                                                  false))
                                            .average()
                                            .orElse(0));
    int gunnerCount = gunners.size();

& for piloting above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A stream here feels like overkill.

@HammerGS HammerGS merged commit a7a9f8a into MegaMek:main Dec 15, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Severity: Medium Issues described as medium severity as per the new issue form Unit Rating

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue] Dropships lower unit experience rating

4 participants