Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved StratConSkillGenerator #6021

Merged
merged 3 commits into from
Sep 25, 2024
Merged

Conversation

IllianiCBT
Copy link
Collaborator

@IllianiCBT IllianiCBT commented Sep 20, 2024

This PR is the mm half of MegaMek/mekhq#4865 and fixes an oversight in the implementation of the StratConSkillGenerator random skill generator. It now does all steps, with fewer reliances on TotalWarfareSkillGenerator methods that were previously causing issues.

Refactored the skill generation logic in StratConSkillGenerator to streamline skill level calculations and removed redundant bonus handling. Also cleaned up logging in AtBDynamicScenarioFactory and removed an unnecessary skill generator type setting.
A random dice roll was introduced to adjust the skill level in the StratConSkillGenerator class. A roll of 1 decreases the skill level while a roll of 6 increases it. This change aims to introduce more variability to skill level determination.
Copy link

codecov bot commented Sep 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 29.10%. Comparing base (b8ce280) to head (99329f5).
Report is 11 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #6021   +/-   ##
=========================================
  Coverage     29.10%   29.10%           
- Complexity    13972    13974    +2     
=========================================
  Files          2580     2580           
  Lines        267143   267141    -2     
  Branches      47711    47712    +1     
=========================================
+ Hits          77743    77746    +3     
+ Misses       185502   185495    -7     
- Partials       3898     3900    +2     

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

@SJuliez
Copy link
Member

SJuliez commented Sep 22, 2024

@IllianiCBT Leaving this to you to merge whenever you think it should

@HammerGS HammerGS merged commit 8a13495 into MegaMek:master Sep 25, 2024
6 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.

3 participants