Add Blob Crews to debiting

[heckindoc]

The individual payouts for blob crews were being tracked and logged, however the actual costs of blob crews was not being deducted or added to the total which was calculated within the financial tab. Additionally the calculation for payment for blob crews included infantry, regardless of what settings were selected.

The calculation for blob crews was added to the getTheoreticalPayroll method, wish a conditional statement to allow the deduction of infantry from contract calculations. In the previous commit I was doing this in the summary method which would change the calculation for paying infantry a salary at all, and the option in the settings is for including infantry pay in contract calculations.

If the point behind that setting is to not pay your infantry then we should probably consider renaming the option and I will revert this back to the previous commit.

[codecov]

Codecov Report

❌ Patch coverage is 25.80645% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 15.21%. Comparing base (295b496) to head (0ddb105).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...Q/src/mekhq/campaign/finances/FinancialReport.java	38.09%	13 Missing ⚠️
MekHQ/src/mekhq/campaign/finances/Accountant.java	0.00%	10 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9110      +/-   ##
============================================
- Coverage     15.21%   15.21%   -0.01%     
- Complexity     9344     9353       +9     
============================================
  Files          1308     1308              
  Lines        173416   173420       +4     
  Branches      26220    26223       +3     
============================================
- Hits          26388    26386       -2     
- Misses       143961   143965       +4     
- Partials       3067     3069       +2     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[IllianiBird]

Leaving this for Psi to review, as it touches blob crews

[psikomonkie]

Hey Doc, thanks for the PR!

One change I need, one change I'd like, and one comment for next time.

Thanks for fixing the silly formatting error I left in Accountant, and for refactoring FinancialReport's r to financialReport!

[psikomonkie]

I think this looks very similar to Accountant.sumTempCrewPay().

Could sumTempCrewPay() be overloaded like sumTempCrewPay(boolean isInfantryDontCount)(with the old sumTempCrewPay() now just calling sumTempCrewPay(false))?

Then we can use sumTempCrewPay(getCampaignOptions().isInfantryDontCount()) in this method.

That would let us have one method that is responsible for summing and returning temp crew pays.

[heckindoc]

I actually tried that originally. This was all in the sumTempCrewPay() method, but I ran into a problem that I think is more indicative of a larger problem that's outside the scope of this PR.

When everything is in sumTempCrewPay() and is called by getTheoreticalPayroll(), the pay displayed when you change the "Infantry Not Counted Toward Contract Pay" option changes as well -- which is incorrect. Based on how that option is worded this should have no affect on what you're actually paying your infantry. Below are photo examples:

Before any changes were made:

After changes with option enabled:

After changes with option disabled:

When everything was moved up to getTheoreticalPayroll() we no longer saw any of those changes in the financial report, but we did see appropriate appearing changes noted to the contract.

I opted not to have a boolean passed into the method as we were already calling CampaignOptions at a class level, so instead of having to pass into a boolean we are just pulling whether that option is enabled directly with invoking campaignOptions.isInfantryDontCount(). But if it would be better to have the boolean passed in, I'm happy to do that

I did notice, looking through everything for this reply, that I had neglected to get rid of the addition from the summed amount from getTheoreticalPayroll() and so I was actually charge the player double for their temp crews. I'll fix this in the next commit

[psikomonkie]

If we add another PersonnelRole in the future as a type of temp personnel, should we need to seek out and update this, or can we assume that anything new is not infantry?

If we want to assume any new PersonnelRole is some kind of Infantry that should be excluded, then this should stay as is. If we want to assume any new PersonnelRole is NOT some kind of Infantry, we should switch this to a list of PersonnelRole[] isInfantry.

[heckindoc]

I'm happy to do it whichever way you would prefer. The right way to do it would probably to make this a method within the HumanResources.java class that way it can be pulled from there, and if we use it anywhere else it only has to be modified in one place.

[psikomonkie]

Were there any changes to this other than renaming r to financialReport? I appreciate making r more readable, it's just a little distracting mixed in with a normal PR. If you ever come across one of these that you want to refactor, especially if it's part of a larger PR you're doing, consider making a separate branch + pr just for the rename so it can get review and merged immediately, and then it won't show on the diff mixed in with actual changes.

Please don't remove this from this PR, I just want to mention this for next time you're improving the legibility to ensure it's merged as smoothly and quickly as possible. I don't always do this myself but I still feel better for the reviewer when I can separate it from code changes.

[heckindoc]

No there weren't. I normally do these kind of refactorings in separate PRs and I don't know why I included it here. Sorry about that, and thank you for the recommendation!