ACS-Like autoresolve [WIP] #5271
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Scoppio
force-pushed
the
acs-autoresolve
branch
from
2aa4deb to
c6ede75
Compare
|
I am eager to take a look at this, but it might not be until the end of the week as I really need to maximize my CO IIC time once thanksgiving guests leave, tomorrow. Is there anything specific you want me to take a look at? |
Scoppio
force-pushed
the
acs-autoresolve
branch
from
c6ede75 to
7286fa4
Compare
Scoppio
force-pushed
the
acs-autoresolve
branch
from
7286fa4 to
386ac31
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5271 +/- ##
============================================
- Coverage 10.47% 10.26% -0.21%
Complexity 6069 6069
============================================
Files 959 1031 +72
Lines 135559 138417 +2858
Branches 19750 20111 +361
============================================
+ Hits 14204 14213 +9
- Misses 119997 122849 +2852
+ Partials 1358 1355 -3 ☔ View full report in Codecov by Sentry. |
Yes, the force creation/setup process, if there is anything that I should take into consideration but isnt, then the post game resole too. The Damage handlers so you can see if it lacks anything you wanted. |
|
CodeQL is pointing false positives for errors that are not errors. |
| <modifier>0</modifier> | ||
| <isRepeatable>0</isRepeatable> | ||
| </KeyBind> | ||
| xsi:noNamespaceSchemaLocation="keyBindingSchema.xsl"> |
There was a problem hiding this comment.
I know xml can sometimes get grumpy when we change whitespaces. Just wanting to verify the changes here won't cause any issues?
MekHQ/src/mekhq/MekHQ.java
Outdated
| } | ||
|
|
||
| // should I do something here? Maybe do the trigger? | ||
| // MekHQ.triggerEvent(new ScenarioResolvedEvent(currentScenario)); |
There was a problem hiding this comment.
I think we want the trigger here, so it can be picked up by subscribers - though I don't know any that listen for this event, it's unlikely to hurt anything.
MekHQ/src/mekhq/MekHQ.java
Outdated
| // MekHQ.triggerEvent(new ScenarioResolvedEvent(currentScenario)); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
There seems to be a lot of duplicated code here, is there value in grabbing the original code and partitioning them into discrete methods and calling those, instead?
| @@ -706,6 +805,64 @@ public static void updateGuiScaling() { | |||
| setLookAndFeel(GUIPreferences.getInstance().getUITheme()); | |||
| } | |||
|
|
|||
| public void startAutoResolve(AtBScenario scenario, List<Unit> units) { | |||
There was a problem hiding this comment.
Let's have a JavaDoc here, when you get round to it. The method is fairly well laid out, so it probably doesn't need to be super verbose.
MekHQ/src/mekhq/MekHQ.java
Outdated
| String message = "This will auto resolve the battle. Do you want to proceed?"; | ||
| var autoResolveMethod = getCampaign().getCampaignOptions().getAutoResolveMethod(); | ||
| if (getCampaign().getCampaignOptions().isAutoResolveVictoryChanceEnabled() | ||
| // unfortunately UNITS MATTER force creating has a bug that makes it impossible to calculate victory chance without causing |
There was a problem hiding this comment.
Is this still the case? I know you did some work on erasing the bugs earlier in the week
| @@ -0,0 +1,7 @@ | |||
| package mekhq.campaign.ai.utility; | |||
There was a problem hiding this comment.
Do we need an interface if we've only got one implementing class?
| import java.util.*; | ||
| import java.util.function.Function; | ||
|
|
||
| public class Memory { |
There was a problem hiding this comment.
We definitely need a JavaDoc on this class as it's not immediately obvious what it does.
| @@ -0,0 +1,7 @@ | |||
| package mekhq.campaign.ai.utility; | |||
There was a problem hiding this comment.
Don't forget the legal notice on your new classes. You're missing them in a couple of places. I know this is still WIP, so you might not have got around to them yet, but figured it was worth mentioning just in case.
| @@ -0,0 +1,6 @@ | |||
| package mekhq.campaign.autoResolve.damageHandler; | |||
There was a problem hiding this comment.
Similar to my last comment, does this need to be a separate class? Can we merge the two enums, seeing that they have the same entries with identical ords
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public class RandomUtils { |
There was a problem hiding this comment.
We could do with some JavaDocs for this class, and/or methods. It might also be worth taking a look at ObjectUtility to see whether there is value in merging the two classes. I would have mentioned this back when you first suggested a random workhorse method, but I only discovered it existed earlier today.
|
|
||
| @Nullable | ||
| public static Entity breakEntityRef(Unit unit) { | ||
| var entity = ASConverter.getUndamagedEntity(unit.getEntity()); |
There was a problem hiding this comment.
While it's not marked as such getEntity() can return null. Something to be aware of as I believe if that occurs here it will result in an NPE.
| static public UnitStrength getUnitStrength(Entity entity) { | ||
| // The values here are completely arbitrary and should be adjusted as needed, probably should be in a config file. | ||
| if (entity instanceof Mek) { | ||
| return new UnitStrength(entity.calculateBattleValue(), +1, entity); |
There was a problem hiding this comment.
It might be worth taking a look at entity.getGenericBattleValue() - GBV is static per unit type & weight, so you might be able to do some math to derive inter-unit type power modifier from that. I don't think the arbitrary values are inherently wrong, mind, just wanted to give you options. :)
| @@ -0,0 +1,90 @@ | |||
| /* | |||
There was a problem hiding this comment.
It's 02:30 local, so I'm going to call it here. Leaving this message so I know where I was up to.
…e, a little bit of performance improvements
Scoppio
force-pushed
the
acs-autoresolve
branch
from
e9072d7 to
6d16be0
Compare
ACS-like autoresolve
This autoresolve implements a mechanism that is inspired in the Abstract Combat System to play out the result of combats instantaneously.
How it works
It uses alot of the work created by @SJuliez for SBF, like the action handlers, phase managers, mostly "dumb" units (so only handlers and services can do things), it also implements a fairly generic memory to units, so they can remember actions they took in their turn, attacks they suffered, etc.
It first redistributes all the forces into ACS Valid force formations, then it creates each formation and links up the AlphaStrike representation of each Entity with its original entity counterpart.
It also reloads every entity as new references, and recreates all crews as new references, otherwise they will stay attached to the original units on the Campaign, which will cause damage and other changes to propagate to the units even if you cancel the resolution.
After all formations are set, a game starts, it uses the initiative, deployment, movement, firing and end phases. Movement phase is used for Engagement and Control which currently is kind of a "thermometer", it mostly change the balance of the game betwen rounds so you dont have a single formation steamrolling everything always.
During firing phase the units hit random units inside enemy formations, as in the quick/simple ACS rules for damage designation.
During the endphase units have their morale tested as per SBF rules (maybe I should change for ACS rules here), they are also checked for withdraw (there is a low chance for a formation to withdraw).
All units that leave the game take some amount of damage.
How to use it
There is a toggle implemented in the campaign options, where you can select if you want your autoresolve to be played by bots (selected as default) or not. With the option unchecked it will use the ACS-like autoresolve instead of opening MM with bots. Now just click in the Autoresolve