Fix: Fixed Reinforcements So That the Force Leader's Strategy is Used for Reinforcement Time and Not the Overall Drop Commander's Strategy #7056
Conversation
… for Reinforcement Time and Not the Overall Drop Commander's Strategy
| UUID commanderId = playerForce.getForceCommanderID(); | ||
| Person commander = campaign.getPerson(commanderId); | ||
| if (commander != null && commander.hasSkill(S_STRATEGY)) { | ||
| strategy = commander.getSkill(S_STRATEGY).getTotalSkillLevel(); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7056 +/- ##
=========================================
Coverage 12.03% 12.03%
+ Complexity 6891 6889 -2
=========================================
Files 1103 1103
Lines 142282 142289 +7
Branches 21991 21992 +1
=========================================
+ Hits 17123 17124 +1
- Misses 123324 123328 +4
- Partials 1835 1837 +2 ☔ 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 changes reinforcement timing logic to use each force leader’s Strategy skill rather than the drop commander’s skill.
- Removed
scenario.getLanceCommanderSkill(...)and instead fetch the specific force commander’s Strategy level. - Added static import for
SkillType.S_STRATEGYand importedPerson. - Initialize
strategyto 0 and conditionally set it from the force commander, defaulting to 0 if absent.
Comments suppressed due to low confidence (1)
MekHQ/src/mekhq/campaign/mission/AtBDynamicScenarioFactory.java:4099
- Consider adding a unit test to verify that the force commander’s Strategy skill correctly influences deployment turns and that the fallback to 0 is applied when no skill is present.
UUID commanderId = playerForce.getForceCommanderID();
| @@ -4083,7 +4085,7 @@ public static void setPlayerDeploymentTurns(AtBDynamicScenario scenario, Campaig | ||
| } | ||
|
|
||
| // Make note of battle commander strategy | ||
| int strategy = scenario.getLanceCommanderSkill(SkillType.S_STRATEGY, campaign); | ||
| int strategy = 0; |
There was a problem hiding this comment.
[nitpick] The initial assignment to 0 is redundant since both branches of the following conditional reassign strategy. Consider declaring it without an initializer and relying on the conditional to set its value.
| int strategy = 0; | |
| int strategy; |
Copilot uses AI. Check for mistakes.
| @@ -4094,6 +4096,13 @@ public static void setPlayerDeploymentTurns(AtBDynamicScenario scenario, Campaig | |||
|
|
|||
| List<Entity> forceEntities = new ArrayList<>(); | |||
| Force playerForce = campaign.getForce(forceID); | |||
| UUID commanderId = playerForce.getForceCommanderID(); | |||
| Person commander = campaign.getPerson(commanderId); | |||
There was a problem hiding this comment.
[nitpick] It may be safer to guard against a null commanderId before passing it to campaign.getPerson(...) to avoid potential NPEs if no commander is assigned.
| Person commander = campaign.getPerson(commanderId); | |
| Person commander = null; | |
| if (commanderId != null) { | |
| commander = campaign.getPerson(commanderId); | |
| } |
Copilot uses AI. Check for mistakes.
Previously we were using the Strategy skill of the overall drop commander, meaning that Strategy on reinforcing units was worthless. While this does make some kind of sense, I definitely prefer players needing to invest in skills across more characters.