-
Notifications
You must be signed in to change notification settings - Fork 178
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
feat: uses ACAR to resolve refused scenarios. #5792
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks fine, insofar as I can see. Just some minor suggested changes.
I do have a question, though: how many iterations of ACAR do we run here to get our result; just the one, or is it defined by the campaign option?
@@ -242,7 +248,7 @@ public void setDeployedBackground(Color value) { | |||
|
|||
public Color getBelowContractMinimumForeground() { | |||
return new Color(userPreferences.node(MHQConstants.DISPLAY_NODE) | |||
.getInt(MHQConstants.BELOW_CONTRACT_MINIMUM_FOREGROUND, Color.RED.getRGB())); | |||
.getInt(MHQConstants.BELOW_CONTRACT_MINIMUM_FOREGROUND, CRIMSON_RED.getRGB())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're updating this, it would be better to use the negative or warning event color, so users can configure it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe, but I dont know what you are referencing here
if (stub) { | ||
if (getCampaignOptions().isUseStratCon() && (scenario instanceof AtBDynamicScenario atBDynamicScenario)) { | ||
|
||
// if (processIgnoredScenario(atBDynamicScenario, contract.getStratconCampaignState())) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're confident we're making this move, we probably want to remove the code rather than commenting it out.
if (scenario.isTurningPoint()) { | ||
campaignState.updateVictoryPoints(-1); | ||
} | ||
// this is already processed in the simulateTheResultsOfIgnoredScenario |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, too. If we're removing this functionality let's remove the dead code instead of just commenting it out.
It would run ACAR for each refused scenario, I would still add a campaign option for it to be activated (I guess people could dislike the fact that the auto resolve is a homebrew system). For some reason it went up as a normal PR and not as a draft, its not by any means ready, so there is some things missing that I want to add before calling that a feature. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5792 +/- ##
============================================
- Coverage 10.30% 10.29% -0.01%
Complexity 6124 6124
============================================
Files 1038 1038
Lines 139020 139120 +100
Branches 20585 20600 +15
============================================
- Hits 14330 14327 -3
- Misses 123296 123398 +102
- Partials 1394 1395 +1 ☔ View full report in Codecov by Sentry. |
I will leave this PR open as a Draft. It is small enough that can be picked up again in the future, however it has too many small problems, and I don't know how it can "actually" give anything to us unless we have persistent OpFor. Otherwise it will be as random as just disappearing with a scenario by rolling a dice |
Experiment - using ACAR to resolve refused scenarios.
If there is team 1 in the scenario in the form of attached allies, it will run a game and see what happens, otherwise it follows the normal ways.