-
Notifications
You must be signed in to change notification settings - Fork 173
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
Implemented Resupply Module #5158
base: master
Are you sure you want to change the base?
Conversation
Created the SupplyDrop class to handle collecting, processing, and delivering supply drops within the campaign. Includes methods for initializing parts pools, generating part weightings, managing drop rewards, and displaying the supply drop dialog to users.
Created SupplyDrops.properties to support in-game supply drop scenarios. Integrated the feature into the campaign with imports and constants for various morale levels.
Removed the bonus parts labels, related configurations, and methods from various classes. Adjusted relevant logic to accommodate these changes and simplified related processes.
Updated the ResolveScenarioWizardDialog constructor to accept `Campaign` as a parameter. This change ensures consistency across different parts of the application and improves code clarity.
Updated the copyright years in various files across the project to reflect the year 2024. This change ensures all legal notices are up-to-date and consistent.
Updated the getWeight method to only take a Part object, removing the dependency on Unit. Adjusted the weighting logic to increase the weight multiplier for MissingPart objects to 5.
Implemented the conversion of remaining support points to supply drops for AtB Contracts when strategic operations are used. This ensures that any leftover points are utilized effectively, enhancing campaign dynamics and resource management.
Adjusted supply drops to respect faction tech sharing restrictions before the Battle of Tukayyid. Fixed a bug with calculating the value and quantity of certain parts. Increased logging and added new reference imports for future functionality.
Revised SupplyDrops to incorporate enemy faction units and parts. Updated methods to handle separate fetching and displaying logic for unit drops and part drops. Altered the initialization process based on whether it is a LosTechCache.
Introduced a mechanic that triggers suspicious deaths related to ComStar interest levels, particularly on Mondays. Also added detailed error logging for these events to improve traceability. Updated resource file with new dud dialog descriptions.
Removed ComStar interest and suspicious death condition from AbstractDeath. Renamed the SupplyDrops package path for better organization. Updated relevant imports and logic to reflect these changes.
Created new `starLeagueCache` and `comStarInterest` classes in supplyDrops package. Renamed SupplyDrops to SupplyDrop and modified related imports and logic in BriefingTab. Updated SupplyDrops.properties to SupplyDrop.properties with new supply drop messages.
# Conflicts: # MekHQ/src/mekhq/campaign/Campaign.java
Removed redundant supply drop texts and streamlined the file by introducing new, organized texts for various operational statuses. This improves the clarity and relevance of messages received during different situational contexts.
Refactored the handling of dialog interactions in the SupplyDrop class to improve code maintainability. Introduced the `processConvoy`, `createConvoyMessage`, and `pickLogisticsRepresentative` methods to manage dialog creation and message processing more effectively. Simplified the message retrieval logic and updated logistic messages in the resource file.
Lowered the interception random number range from 50 to 25 to adjust event frequency. Consolidated and simplified supply drop descriptions to enhance readability and maintain consistency.
Renamed the SupplyDrop class to Resupply and updated related references across the project for clarity. The package was also reorganized from supplyDrops to resupplyAndCaches to better reflect its contents.
Refactored the resupply mission logic to improve readability and efficiency. Added morale level checks and updated message formatting for intercepted supplies. Renamed the properties file from `SupplyDrop.properties` to `Resupply.properties` and updated relevant messages.
Moved resupply logic to correct conditional block for clarity. Ensured resupply parts are calculated and obtained immediately after adding the report. This change improves code organization and readability.
Refactored the convoy message creation logic in the `Resupply` class to ensure intercepted messages are generated correctly. Consolidated duplicate code, adjusted the ordering of condition checks, and introduced an overloaded `createConvoyMessage` method for clarity and simplicity.
Inserted 'UNEDITED' placeholder tags to various status update messages for better identification and tracking. This change will facilitate easier referencing during future edits and improvements.
Renamed resupplyAndCaches package by removing 'atb' for better organization. Added more detailed supply logic to accommodate ration packs and medical supplies based on personnel roles.
Eliminated unnecessary commented-out code related to resupply operations in Campaign.java. This cleanup ensures the final step before returning true remains clear and concise, improving code readability.
Refactored the resupply process to handle convoy interceptions instead of destruction. Added new messages for both StratCon and AtB scenarios to indicate that a special defense scenario has been added.
Consolidated import statements in the Resupply.java class and introduced a Logger for debugging. Modified the Stratcon track selection logic along with adjustments in reporting methods. Updated Resupply.properties for better readability and maintainability.
# Conflicts: # MekHQ/src/mekhq/campaign/stratcon/StratconContractInitializer.java
# Conflicts: # MekHQ/src/mekhq/campaign/Campaign.java # MekHQ/src/mekhq/campaign/force/Force.java # MekHQ/src/mekhq/campaign/stratcon/StratconCampaignState.java # MekHQ/src/mekhq/campaign/stratcon/StratconContractInitializer.java # MekHQ/src/mekhq/campaign/stratcon/StratconRulesManager.java # MekHQ/src/mekhq/gui/adapter/TOEMouseAdapter.java # MekHQ/src/mekhq/gui/dialog/ContractMarketDialog.java # MekHQ/src/mekhq/gui/stratcon/CampaignManagementDialog.java
Co-authored-by: Luana Coppio <[email protected]>
Removed command rights check from processAbandonedConvoy and extracted crew fate logic into a new method. This improves code readability and separation of concerns by isolating the status determination logic. Defensive programming was applied with error handling to manage unexpected exceptions.
Introduced a method to estimate cargo requirements for campaigns and integrated it into MissionViewPanel. This feature provides additional information for campaigns using strategic formations by displaying estimated cargo requirements, enhancing decision-making and campaign management. Updated the UI layout to include the new cargo requirement labels and values.
Removed recursive subForce status setting in `setConvoyForce`. Updated method calls to reflect this change across the codebase. This simplifies the setting of convoy force and improves clarity in convoy operations.
Flipping this to live. Description has been updated accordingly. |
Updated the copyright information in Scenario.java and ScenarioType.java to include the year 2024 for the MegaMek Team. This change ensures the files reflect the current year and comply with licensing requirements.
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.
The quality is as always great. But there are some parts that require special attention, and at least two classes need a large refactor, alongside a few functions need to be extracted from where they are now.
private void processAbandonedConvoy(AtBContract contract, AtBDynamicScenario scenario) { | ||
convoyFinalMessageDialog(this, contract.getEmployerFaction()); | ||
|
||
for (Integer forceId : scenario.getPlayerTemplateForceIDs()) { | ||
try { | ||
Force force = getForce(forceId); | ||
|
||
if (force.isConvoyForce()) { | ||
for (UUID unitID : force.getAllUnits(false)) { | ||
Unit unit = getUnit(unitID); | ||
|
||
for (Person crewMember : unit.getCrew()) { | ||
decideCrewMemberFate(crewMember); | ||
} | ||
|
||
removeUnit(unitID); | ||
} | ||
} | ||
} catch (Exception ex) { | ||
logger.warn(ex.getMessage(), ex); | ||
} | ||
} | ||
} |
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.
Move everything that you added to Campaign to a new class, Dont matter if its just a static function which receives Campaign and a couple of args (better if it doesnt), make it return a list of units to kill and kill those in another function (can be inside the same class, just separate into two or more stages so they can be easily identified, tested, edited, expanded).
The thing is that Campaign is already giant, and there is no need for us to keep making it larger. It has too many responsibilities and instead of breaking each new behavior into a ner service or handler we are just adding more and more stuff into the deathstar model that Campaign has become. We really need to move those things out, and you added so little code to Campaign, and it is very isolated as it currently stands, I would prefer to see it in any other place.
PS.: The idea od a class with static functions can be worked as a class with many functions, but if we are doing it with a stateless class, it should work quite whell, with very little memory overhead.
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.
Wholeheartedly agree. The issue we have right now is that campaign.java is so all encompassing and touching so many different aspects, the likelihood that a new function will be related to something in campaign.java (and therefore logically placed in that class) is fairly high.
Something I think we'd really benefit from is someone ripping campaign.java apart and democratizing a lot of the methods housed there.
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.
Campaign became one of two "façade" objects for the project, MHQ being the other one. It is fine, and it happens, however this also means that our efforts should be focused on every time we "touch" the campaing, instead of adding code to it, we take the code that we want to interact with and move to a new place. It is better if campaign become something like a "context", as if we had to communicate with a database and campaign were that database.
private void decideCrewMemberFate(Person person) { | ||
PersonnelStatus status = KIA; | ||
if (Compute.d6(2) > 7) { | ||
status = PersonnelStatus.POW; | ||
} | ||
person.changeStatus(this, currentDay, status); | ||
} |
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.
see previous comment.
@@ -3906,6 +3964,11 @@ private void processNewDayATB() { | |||
if (!report.isBlank()) { | |||
addReport(report); | |||
} | |||
|
|||
// Resupply | |||
if (getLocation().isOnPlanet() && getLocation().getCurrentSystem().equals(contract.getSystem())) { |
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.
extract this condition as a function
private void processResupply(AtBContract contract) { | ||
if (!contract.getContractType().isGuerrillaWarfare() || Compute.d6(1) > 4) { | ||
int dropCount = (int) Math.max(1, Math.floor((double) contract.getRequiredLances() / 3)); | ||
Resupply resupplies = new Resupply(this, contract, false); | ||
resupplies.getResupply(dropCount, false); | ||
} | ||
} |
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.
This is not returning anything, the object contract is not being changed, and if it is, its being changed due to a collateral effect, if that is the case then this must be changed due to the complexity of dealing with collateral effects (this is not explicit in the javadoc, and I am just assuming thats the case here).
If thats the case, either make it return a resuply action that you then "apply" somewhere, or move that internal ot the Resupply object and call a specific action on it, line "initialize" or something like that. Make it yell to everyone what it is doing.
public boolean isCombatForce() { | ||
return combatForce; | ||
return combatForce && !convoyForce; | ||
} |
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.
Its the case of changing the boolean to an Enum
public void createDudDialog(StratconTrackState track, StratconScenario scenario) { | ||
StratconCoords stratconCoords = scenario.getCoords(); | ||
|
||
// Dialog dimensions and representative | ||
final int DIALOG_WIDTH = 400; | ||
final int DIALOG_HEIGHT = 200; | ||
|
||
// Creates and sets up the dialog | ||
JDialog dialog = new JDialog(); | ||
dialog.setTitle(resources.getString("dialog.title")); | ||
dialog.setLayout(new BorderLayout()); | ||
dialog.setSize(UIUtil.scaleForGUI(DIALOG_WIDTH, DIALOG_HEIGHT)); | ||
dialog.setLocationRelativeTo(null); |
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.
If the GUIs and dialogs are just to allow users to interact with a very small a mount of things that can be interacted also through the API of the class I am more prone to be ok with it, but I still would prefer it to be a separated class which this one can instantiate and show to the user.
public void useSupportPoints(int increment) { | ||
supportPoints -= increment; | ||
} |
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 guess thats a decrement
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 do believe you would be correct, yes :D
public void convertVictoryToSupportPoint() { | ||
victoryPoints--; | ||
supportPoints++; | ||
} |
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.
lacking check if VP are greater than zero.
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'm glad you spotted this, because that whole method should have been removed when merging in master.
// if (Compute.randomInt(100) == 0) { | ||
// ScenarioTemplate template = ScenarioTemplate.Deserialize( | ||
// "data/scenariotemplates/Chasing a Rumor.xml"); | ||
// | ||
// if (template != null) { | ||
// StratconScenario hiddenCache = addHiddenExternalScenario(campaign, contract, | ||
// null, template, false); | ||
// | ||
// if (hiddenCache != null) { | ||
// logger.info(String.format("A secret cache has been spawned for contract %s", | ||
// contract.getName())); | ||
// } | ||
// } else { | ||
// logger.error("'Chasing a Rumor' scenario failed to deserialize"); | ||
// } | ||
// } | ||
} | ||
|
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.
just to mark
// if (false) { // TODO replace placeholder value | ||
// cache.createDudDialog(track, scenario); | ||
// } else { | ||
// if (Objects.equals(cache.getFaction().getShortName(), "SL")) { | ||
// cache.createProposalDialog(); | ||
// } | ||
// } |
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.
Just to mark
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.
You might have noticed a couple of these - they're related to the LosTech portion of this module that will be launched separately. We decided to split implementation in two mid-production, as this allows us to work on LosTech Caches while addressing bugs and feedback with the logistics portion of the module. I should have mentioned this in the PR description. mea culpa.
While I see this has been approved, it should not be merged until I've had the opportunity to review Luana's suggested changes. |
Improved the Resupply class by refactoring the part preparation logic to include null-checks, preventing potential errors. Enhanced the code readability and maintainability by simplifying conditional statements and ensuring a more robust handling of part cloning failures.
Introduced tooltips to several labels in the MissionViewPanel to enhance user guidance with morale information. Adjusted GridBag layout configurations for better UI alignment and positioning. Also, changed the calculation of targetCargoTonnage to use a constant multiplier for better maintainability.
Replaced iterations over strategic formations with all forces to streamline convoy checks. Modified setting of convoy status to ensure accurate force representation, including parents and sub-forces. Simplified force name formatting by consolidating logic into a single string format.
Removed adHocResupply flag from the Resupply constructor to simplify the method signature and updated the Resupply instantiation across several files. Modified cargo tonnage calculation to remove ad-hoc multipliers, adjusted it to use strategic formations, and improved the logic for calculating cargo capacity totals.
Corrected the pluralization error in the player convoy text to ensure proper grammar. This improves the clarity and professionalism of the in-game messaging.
Revised `calculateTargetCargoTonnage` to use integers instead of doubles for improved precision and consistency. Incorporated faction-specific formation sizes and adjusted tonnage based on derived allowances. Updated related methods to align with the new calculation logic.
Increased the cargo divider from 250 to 300 to refine the tonnage allowance calculation. This change improves accuracy in resupply estimations, aligning it better with target values.
Revised the calculation for target cargo tonnage to include clearer logic and ensure accurate results. Introduced tonnage caps, individual allowances, and streamlined computation steps for better maintainability and employer-specific constraints. Enhanced comments and eliminated unused variables for improved code clarity.
This PR implements the new Resupply Module. For more information please see here
This is a fairly complicated module and documentation will be provided prior to the launch of 50.02. However, for the benefit of QA I'm going to provide a brief summary.
Each month, while on contract, the player will be provided a resupply of items tailored to their TO&E. The size of the convoy's delivery is based on the average weight of combat units in the TO&E. Players can provide their own units in exchange for a bonus. For independent contracts players must supply their own units.
Convoys can be intercepted, with the player able to intercede should they wish it. Interception chance is based on force tonnage, with larger convoys easier for the enemy to detect; and enemy morale, with higher morale increasing the chance of an interception. Convoys can weigh up to 200t before suffering a detection malus, while lighter convoys will enjoy a bonus. Abandoning a player convoy will result in the convoy's automatic defeat. All units will be lost and all crew will be captured or killed.
Ground convoys can run into role-play events. I would like to extend these events to aerial convoys, but that will take a bunch of work and time is short.
On guerrilla contracts players cannot provide their own convoys, but instead are approached by smugglers. These resupplies cost a value based on the contents of the convoy with a chance the player will be scammed.
To define a force as a convoy the force must be selected in the TO&E and assigned as a supply convoy via the right-click menu. While any forces can be assigned as a convoy there are some limitations. All forces descended from the selected force will be considered a part of the convoy. It is not possible to tell MekHQ to ignore specific forces. It's an all or nothing. A force that has an ancestor that is classified as a 'convoy' cannot itself be made as a convoy. When you assign a convoy all ancestors and descendants will have their convoy status revoked.