-
Notifications
You must be signed in to change notification settings - Fork 171
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
base: master
Are you sure you want to change the base?
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.52% 10.52% -0.01%
- Complexity 6042 6045 +3
============================================
Files 956 958 +2
Lines 134346 134475 +129
Branches 19514 19520 +6
============================================
+ Hits 14145 14151 +6
- Misses 118851 118974 +123
Partials 1350 1350 ☔ 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 |
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.