Improvement: Made Planetary Acquisition Verbose Mode Even More Verbose #6840
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request increases the verbosity of Planetary Acquisition output by including target numbers and detailed modifier information, and refactors the maintenance method call for improved readability.
- Added detailed bonus outputs (Tech, Industry, and Outputs) to the verbose messages in the acquisition methods.
- Reformatted the call to doMaintenanceOnUnitPart to improve code clarity.
| @@ -3884,7 +3889,18 @@ public PartAcquisitionResult findContactForAcquisition(IAcquisitionWork acquisit | |||
| acquisition.getAcquisitionName() + | |||
| " on " + | |||
| system.getPrintableName(getLocalDate()) + | |||
| "</b></font>"); | |||
| " at TN: " + | |||
There was a problem hiding this comment.
[nitpick] Consider using String.format or a similar approach to build the verbose message, improving readability and maintainability of the string construction.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Not strictly relevant. Can be ignored.
| @@ -3874,6 +3874,11 @@ public PartAcquisitionResult findContactForAcquisition(IAcquisitionWork acquisit | |||
| } | |||
| return PartAcquisitionResult.PlanetSpecificFailure; | |||
| } | |||
| SocioIndustrialData socioIndustrial = system.getPrimaryPlanet().getSocioIndustrial(getLocalDate()); | |||
There was a problem hiding this comment.
[nitpick] Consider extracting the verbose string construction logic into a dedicated helper method to reduce duplication across both branches.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Not strictly relevant. Can be ignored.
| (tech > 0 ? "+" : "") + | ||
| tech + | ||
| ", Industry: " + | ||
| (industry > 0 ? "+" : "") + |
There was a problem hiding this comment.
So one small comment, external to the review. If you were writing this method fresh we'd ask you to use a resource bundle. In the event that wasn't possible you'd absolutely want to use a StringBuilder here, instead of just gluing 101 strings together.
There was a problem hiding this comment.
I 100% would use String Builder here.
Codecov ReportAttention: Patch coverage is
Scoppio
approved these changes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
|
Added visibility for the target numbers and modifiers going into Planetary Acquisition verbose mode.