-
Notifications
You must be signed in to change notification settings - Fork 176
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
Auto-Resolve: Make princess take the reins so you can play a game of spreadsheets! #5155
Conversation
Also, I just realised, I need to document the feature. |
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.
I have some points of feedback, and some concerns - largely around code redundancy. In particular, it's hard to tell what is new code in AutoResolveBehaviorSettingsDialog.java
and what is duplicated from the normal bot dialog.
One of the big reasons we want to avoid code redundancy here, imo (and I stress, this is just an opinion), is because any changes made to the original bot behavior dialog would need to be replicated in your new class.
MekHQ/src/mekhq/gui/dialog/AutoResolveBehaviorSettingsDialog.java
Outdated
Show resolved
Hide resolved
} else if (xn.equalsIgnoreCase("autoResolveBehaviorSettings")) { | ||
retVal.setAutoResolveBehaviorSettings( | ||
firstNonNull(BehaviorSettingsFactory.getInstance().getBehavior(wn.getTextContent()), | ||
BehaviorSettingsFactory.getInstance().DEFAULT_BEHAVIOR) |
Check warning
Code scanning / CodeQL
Expression always evaluates to the same value Warning
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.
False positive, this evaluates to the correct value, either the behavior setting with the name which is present in the "getTextContent", or the default behavior.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5155 +/- ##
============================================
- Coverage 10.50% 10.49% -0.02%
+ Complexity 6044 6041 -3
============================================
Files 957 959 +2
Lines 134716 134851 +135
Branches 19572 19582 +10
============================================
- Hits 14149 14147 -2
- Misses 119216 119353 +137
Partials 1351 1351 ☔ View full report in Codecov by Sentry. |
ca4a357
to
5b9d8d0
Compare
…r preset for auto resolve
…n for the auto resolve in the dialog
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.
A couple of minor nitpicks. Great work. :)
MekHQ/src/mekhq/gui/dialog/AutoResolveBehaviorSettingsDialog.java
Outdated
Show resolved
Hide resolved
…s and deleted comments
Maybe it would make sense (not as part this PR) to offer to auto-resolve battles entirely without opening MM at all? Would be safer, quicker and not overtax Princess with mission goals that she is not equipped to handle. |
I really wanted to do that, I will explore somethings to be able to get to an approximation of an auto-resolve, I will also explore how other games do that. It is out of the scope of this PR but I do want to get a king of auto-resolve that is very short and sweet for the player, and which does take the environment into consideration. |
Added information on current limitations on the auto-resolve
@Scoppio This has conflicts |
Sorry, I had not seen your comment here. I fixed the conflicts. |
@IllianiCBT the github actions are failing but it seems to be something related to it pulling the repo, github was kinda funky a minute ago, might be related to a short outage? |
@@ -337,6 +340,7 @@ | |||
quartermaster = new Quartermaster(this); | |||
fieldKitchenWithinCapacity = false; | |||
fameAndInfamy = new FameAndInfamyController(); | |||
autoResolveBehaviorSettings = BehaviorSettingsFactory.getInstance().DEFAULT_BEHAVIOR; |
Check warning
Code scanning / CodeQL
Expression always evaluates to the same value Warning
I've tasked the failed tests to rerun. If they still fail, make sure your branch is fully up to date with master on all three repos. |
Marking this as draft as I know this is still in development and I don't want it accidentally merged. |
This autoresolve is working as intended and I dont think there is much that can be done on it outside of AI improvements. |
Is it worth merging this one in, if we're going to replace it? You'd end up having to support two different auto resolution methods |
I think it is, they are very different, and I dont know if the one based on ACS will come to become anything to be honest. This one is complete and relies on tech that is already done, is tested and works very well. |
Understood, removing draft |
First small contribution.
Motivation:
I like to play it from time to time, but as the game progresses I realize that what I really like is managing the company, MekHQ is what I really like, and the battles were getting in the way of my fun. So after some talk with other maintainers I learned that I could load a bot to play for me, and that was fun, some games I really dont want to spend the time to hunt a single mek, or fight a space or low-atmo battle is fun for fluff but not my personal taste for the game.
Implementation
This pull request adds the an auto-resolve button, it opens Megamek with an extra BotForce with the name of your company + ":AI", it has the same config as the player including color, camouflage, starting location, and all the player owned entities are then transferred to this AI.
The AI Behavior setting is persisted as a preset with the same name as your company + ":AI", so if its name "Assault Scout Company" the preset is going to be named "Assault Scout Company:AI". If by any chance you delete the preset, it will load the Default behavior setting in its place.
If you open an older save, it will load the Default behavior too, so it is backward compatible.
Any change to the configuration is persisted the moment you hit "OK" in the settings dialog, if you hit cancel it just closes without propagating the changes, it keeps the "help" button that explain Princess behavior configuration.
A new function was added in the "management/gm thingamajig" option at the top, the same where you can create a company, where you can configure the Behaviour Setting for your company.