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

Auto-Resolve: Make princess take the reins so you can play a game of spreadsheets! #5155

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Scoppio
Copy link
Collaborator

@Scoppio Scoppio commented Oct 31, 2024

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.

MekHQ/src/mekhq/AtBGameThread.java Fixed Show resolved Hide resolved
MekHQ/src/mekhq/AtBGameThread.java Show resolved Hide resolved
MekHQ/src/mekhq/AtBGameThread.java Show resolved Hide resolved
MekHQ/src/mekhq/AtBGameThread.java Fixed Show resolved Hide resolved
MekHQ/src/mekhq/campaign/Campaign.java Show resolved Hide resolved
@Scoppio
Copy link
Collaborator Author

Scoppio commented Oct 31, 2024

Also, I just realised, I need to document the feature.

Copy link
Collaborator

@IllianiCBT IllianiCBT left a 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/MekHQ.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

Expression always evaluates to the same value.
Copy link
Collaborator Author

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-commenter
Copy link

codecov-commenter commented Oct 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.52%. Comparing base (2b3d042) to head (48cc7d9).
Report is 17 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

@Scoppio Scoppio force-pushed the autoresolve branch 3 times, most recently from ca4a357 to 5b9d8d0 Compare October 31, 2024 13:13
Copy link
Collaborator

@IllianiCBT IllianiCBT left a 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/AtBGameThread.java Outdated Show resolved Hide resolved
@SJuliez
Copy link
Member

SJuliez commented Nov 5, 2024

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.

@Scoppio
Copy link
Collaborator Author

Scoppio commented Nov 5, 2024

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.
This one was my "first step" into contributing to MM, I went with something that allows me to just shortcut the process of adding a bot to play for me.

Added information on current limitations on the auto-resolve
@HammerGS
Copy link
Member

HammerGS commented Nov 9, 2024

@Scoppio This has conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants