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

Added Nag Dialog for Prisoners of War Outside of Contracts #3942

Merged
merged 4 commits into from
Apr 2, 2024

Conversation

IllianiCBT
Copy link
Collaborator

@IllianiCBT IllianiCBT commented Mar 31, 2024

Current Implementation

None

Problem

I am definitely not the only person who has done this: end the contract, enjoy the spoils of war, get halfway to Galatea only to remember you have a couple hundred prisoners of war still hanging about. You're not about to return to the contract planet so you just dump them out into space. Everyone is sad.

Solution

On a new day, if not on a contract, the system will check whether any of the users' active personnel are prisoners. If any prisoners are found the nag dialog is triggered.

image image

Update

I removed the check for Bondsmen, as these can still be assigned to roles they don't really have the same issue as Prisoners.

@IllianiCBT IllianiCBT self-assigned this Mar 31, 2024
@IllianiCBT IllianiCBT added Personnel Personnel-related Issues GUI labels Mar 31, 2024
Copy link

codecov bot commented Mar 31, 2024

Codecov Report

Attention: Patch coverage is 0% with 19 lines in your changes are missing coverage. Please review.

Project coverage is 10.65%. Comparing base (2676968) to head (cf0e4c9).
Report is 4 commits behind head on master.

Files Patch % Lines
...ekhq/gui/dialog/nagDialogs/PrisonersNagDialog.java 0.00% 9 Missing ⚠️
MekHQ/src/mekhq/gui/dialog/MHQOptionsDialog.java 0.00% 7 Missing ⚠️
MekHQ/src/mekhq/gui/CampaignGUI.java 0.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3942      +/-   ##
============================================
- Coverage     10.65%   10.65%   -0.01%     
  Complexity     5528     5528              
============================================
  Files           836      837       +1     
  Lines        114275   114294      +19     
  Branches      17185    17188       +3     
============================================
  Hits          12176    12176              
- Misses       100871   100890      +19     
  Partials       1228     1228              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AaronGullickson
Copy link
Member

One thing to keep in mind is that people use MekHQ for all kinds of things not just AtB, so we should be careful about assuming purpose and intent of having prisoners. I don't love having this nag if I want to keep prisoners for some reason, but I suppose it can be dismissed easily enough.

@AaronGullickson AaronGullickson self-requested a review April 2, 2024 00:22
@IllianiCBT
Copy link
Collaborator Author

One thing to keep in mind is that people use MekHQ for all kinds of things not just AtB, so we should be careful about assuming purpose and intent of having prisoners. I don't love having this nag if I want to keep prisoners for some reason, but I suppose it can be dismissed easily enough.

Valid point. My thinking is that, in instances where users want to keep their prisoners, the nag is easy to suppress and re-enable at a later date. Much like unmaintained units.

@AaronGullickson
Copy link
Member

Yeah, that's fine. If you can just clear the other excess parenthesis, I can approve and merge.

@IllianiCBT
Copy link
Collaborator Author

Yeah, that's fine. If you can just clear the other excess parenthesis, I can approve and merge.

Done (assuming there wasn't others). I also changed hasPrisoners() to private, as there was no reason for it to be public.

@AaronGullickson AaronGullickson merged commit ed49493 into MegaMek:master Apr 2, 2024
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI Personnel Personnel-related Issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants