Refactor StratCon Reinforcement Logic and Improve Skill Checks #6709
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
- Refactored reinforcement logic to utilize the new Centralized Skill Check utility class when evading enemy interceptions.
- Updated reinforcement target number to better match the original intent. Now the skill of the Admin character will act as a modifier, rather than the primary target number. The base Target Number is currently 7, adjusted by the experience level of the admin character.
### Pull Request Summary - **Refactored reinforcement interception logic**: - Consolidated morale-based reinforcement success checks. - Removed redundant routed enemy success check. - **Implemented `SkillCheckUtility` for skill evaluations**: - Simplified and standardized the handling of Tactics skill checks. - Added success/failure reporting for reinforcement evasion. - **Updated morale and skill modifier calculations**: - Adjusted targeting logic using skill and morale modifiers. - Improved handling of default skill behavior for unskilled commanders. - **Refined user-facing reinforcement text resources**: - Removed redundant or obsolete text for routed enemy scenarios. - Simplified evasion success and failure messaging. - **General cleanup and optimizations**: - Standardized imports and removed unused variables. - Refined reinforcement target calculation method for better clarity. This refactor simplifies the reinforcement logic while maintaining functionality and improving usability, making skill-based outcomes more consistent and clear to users.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6709 +/- ##
============================================
+ Coverage 11.40% 11.45% +0.04%
- Complexity 6482 6483 +1
============================================
Files 1095 1094 -1
Lines 141048 140613 -435
Branches 21884 21799 -85
============================================
+ Hits 16082 16101 +19
+ Misses 123308 122859 -449
+ Partials 1658 1653 -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 pull request refactors the StratCon reinforcement logic and modifies the skill check procedure for better alignment with the original intent. Key changes include:
- Integrating the new Centralized Skill Check utility for enemy evasion.
- Adjusting the reinforcement target number computation by applying the admin character’s skill as a modifier.
- Updating reporting logic to directly add campaign reports.
Files not reviewed (1)
- MekHQ/resources/mekhq/resources/AtBStratCon.properties: Language not supported
Comments suppressed due to low confidence (1)
MekHQ/src/mekhq/campaign/stratcon/StratconRulesManager.java:1855
- [nitpick] Consider extracting the literal '7' into a named constant to enhance clarity and facilitate future adjustments of the base target number.
TargetRoll reinforcementTargetNumber = new TargetRoll(7, "Base Target Number");
Comment on lines
1692
to
1694
| // Check passed | ||
| if (interceptionRoll >= interceptionOdds) { | ||
| if (interceptionRoll >= interceptionOdds || contract.getMoraleLevel().isRouted()) { | ||
| reportStatus.append(' '); |
There was a problem hiding this comment.
Verify that combining the interception roll check with the enemy morale routed condition is intentional, as this change may bypass some reporting or handling scenarios previously managed separately.
Copilot uses AI. Check for mistakes.
Comment on lines
+1742
to
1749
| campaign.addReport(skillCheckUtility.getResultsText()); | ||
|
|
||
| if (roll >= targetNumber) { | ||
| reportStatus.append(' '); | ||
| if (skillCheckUtility.isSuccess()) { | ||
| String reportString = tactics != null ? | ||
| resources.getString("reinforcementEvasionSuccessful.text") : | ||
| resources.getString("reinforcementEvasionSuccessful.noSkill"); | ||
| reportStatus.append(String.format(reportString, | ||
| campaign.addReport(String.format(reportString, | ||
| spanOpeningWithCustomColor(MekHQ.getMHQOptions().getFontColorPositiveHexColor()), |
There was a problem hiding this comment.
[nitpick] Consider standardizing the reporting approach by either accumulating messages in reportStatus or using campaign.addReport consistently, to ensure clarity in the report output.
Copilot uses AI. Check for mistakes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.