Establishment of a Requested Stock % feature, and automating weekly checks to keep spare parts in stock #5321

Lapras · 2024-12-08T17:55:58Z

Making sure your mercenary company has all the spares needed to contribute to ongoing repairs is a massive manual endeavor once you have anything more than a Lance. This PR adds features to the Parts In Use report for the user. Allow them to request a percentage of stock of a part in use that they want to keep in stock. For example, if a company is using 20 medium lasers, and your stock percentage is 25%, you are asking to always have 5 spare medium lasers on hand for repairs. It adds a button that will compare current stock to requested stock and automatically put in orders (or add them via GM mode) to get you all those parts. Finally, there's a checkbox to make this check every Monday. Finally, your logistics officers can do their jobs instead of you having to direct them to buy every part necessary.

This is done with a couple of changes:

In CampaignOptions, a default stock level percentage is added to the options that is applied to every new part in use by default
We need to store our settings for parts in use now (ignore mothballed, minimum part quality, weekly check, and all of the stock percentages), so we've added some marshalling and unmarshalling in the XML so that saved games now track what parts in use are requested at what levels. Keep in mind that this is done by storing a simple map from PIU names to doubles, we don't actually store all of the information contained within the parts in use report, just a map from names to the levels, and reapply them when parts in use are generated.
In the Campaign.java, we're keeping track of our partInUse options and have implemented functions to both purchase and add parts that aren't in stock.
The Parts In Use dialog has a column added to it for the request stock % as well as buttons to fill stock that access these campaign functions
A weekly check on newDay() has been added in campaign in order to check your stock levels weekly
Parts In Use no longer derive their name (and therefore how they're compared) by the description of their part, but rather the AcquisitionName of the IAcquisitionWork object that is their partToBuy, this gives us a more consistent naming scheme and avoids problems with part in use recognition. I've added in a special edge case for turrets, as their tonnage isn't specified within their IAcquisitonWork but does matter.

… between Campaign.java and the PartsReportDialog

… save/load systems and weekly checks.

IllianiCBT · 2024-12-08T18:22:33Z

Looks like this has picked up a conflict. Once CO IIC launches, please take the time to double check any campaign options you’ve added weren’t accidentally missed

IllianiCBT · 2024-12-08T18:23:57Z

@HammerGS i want the opportunity to review this before it gets merged, but it needs to be merged before CO IIC else it’ll create a massive pain of conflicts.

HammerGS · 2024-12-08T18:28:51Z

@HammerGS i want the opportunity to review this before it gets merged, but it needs to be merged before CO IIC else it’ll create a massive pain of conflicts.

I'll let you handle the review and merge

IllianiCBT · 2024-12-12T18:24:51Z

Looks like this has picked up a conflict. @Sleet01 I don't suppose you'll have time to cast your eyes over this as well? With this being a high profile addition that's going to get a lot of attention I wanna make sure we're catching any issues before launch.

IllianiCBT

I'm really looking forward to having this merged, however there are a couple of points of feedback.

The biggest ones are relating to weird whitespacing that doesn't match stylistically to the rest of the mhq codebase, and abbreviated variable names. This might seem petty, but we need to remain consistent where possible to make it easy for other contributors to drop in without having to learn multiple styles.

IllianiCBT · 2024-12-12T18:25:30Z

MekHQ/resources/mekhq/resources/CampaignOptionsDialog.properties

@@ -76,7 +76,9 @@ lblAcquireSkill.text=Acquisition Skill:
 lblSupportStaffOnly.text=Only support personnel can make acquisition checks
 lblWaitingPeriod.text=Waiting period (in days) between acquisition rolls
 spnMaxAcquisitions.toolTipText=This number is the maximum number of acquisitions an administrator can attempt each day. Setting this to 0 makes it unlimited.
+spnDefaultStockPercent.toolTipText=This number is the default % of stock to fill for parts in use


Minor stylistic choice, can we change % to percent or percentage

IllianiCBT · 2024-12-12T18:25:51Z

MekHQ/resources/mekhq/resources/PartsInUseTableModel.properties

@@ -10,3 +10,4 @@ ordered.heading=Ordered
 part.heading=Part
 stored.heading=Stored
 storedTonnage.heading=Tonnage
+requestedStock.heading =Requested Stock Percent


Extra whitespace after .heading

In fact, I've noticed a lot of weird whitespacing throughout your code. Lots of if( or while( instead of if ( and while ( which is more standard for our codebase. I'm not going to force you to change them, but I do think we need to maintain whitespace consistency where possible.

IllianiCBT · 2024-12-12T18:26:25Z

MekHQ/src/mekhq/campaign/Campaign.java

@@ -174,6 +174,8 @@ public class Campaign implements ITechManager {
    private final TreeMap<Integer, Scenario> scenarios = new TreeMap<>();
    private final Map<UUID, List<Kill>> kills = new HashMap<>();

+    private Map<String, Double> piuStockMap = new LinkedHashMap<>(); // This map keeps track of which parts i use are assigned which stock values


This comment is a little hard to parse. Is there a typo?

IllianiCBT · 2024-12-12T18:27:42Z

MekHQ/src/mekhq/campaign/Campaign.java

@@ -271,6 +273,10 @@ public class Campaign implements ITechManager {
    private FameAndInfamyController fameAndInfamy;
    private List<Unit> automatedMothballUnits;

+     //options relating to parts in use and restock
+    private boolean ignoreMothballed, topUpWeekly;


Having a twinned variable isn't inherently wrong, but it's not something I've seen elsewhere in our codebase. Strictly personnel preference: I'd define these separately. However, I want to emphasize that this is personal preference so go with whatever your heart tells you. :)

IllianiCBT · 2024-12-12T18:28:47Z

MekHQ/src/mekhq/campaign/Campaign.java

@@ -2236,6 +2245,7 @@ private PartInUse getPartInUse(Part p) {
            p = ((MissingPart) p).getNewPart();


I know this isn't your code, but we're trying to move away from single letter or abbreviated variable names. As you're doing a lot of work in this neck of the woods would you mind changing p to part?

IllianiCBT · 2024-12-12T18:44:14Z

MekHQ/src/mekhq/gui/dialog/PartsReportDialog.java

+        int toBuy = needed-inventory;
+
+        if(piu.getIsBundle() == true) {
+            toBuy = (int)Math.ceil((float)toBuy * piu.getTonnagePerItem() / 5);


Abbreviations and same question as before: are we meaning to buy 0.2t of ammo?

IllianiCBT · 2024-12-12T18:45:25Z

MekHQ/src/mekhq/gui/dialog/PartsReportDialog.java

+        }
+
+        if(toBuy > 0) {
+            System.out.println("TPI: " + piu.getTonnagePerItem() + " " + String.format("Inv: %d needed: %d tobuy: %d", inventory, needed, toBuy));


Let's definitely push this to logger rather than System.out. System.out is fine for temporary logging, maybe, but for anything like this we need a record of it when players find issues and send in their logs.

IllianiCBT · 2024-12-12T18:46:13Z

MekHQ/src/mekhq/gui/model/PartsInUseTableModel.java

@@ -400,4 +409,24 @@ public Component getTableCellRendererComponent(JTable table, Object value, boole
            return renderButton;
        }
    }
+
+    @Override
+    public void setValueAt(Object value, int rowIndex, int columnIndex) {


Do we want to do this, or would it be better to use a JSpinner?

IllianiCBT · 2024-12-12T18:47:28Z

gradle/wrapper/gradle-wrapper.jar

Did you mean to include this?

IllianiCBT · 2024-12-12T18:47:57Z

MekHQ/userdata/data/universe/ranks.xml

I could have sworn we already addressed this change? Clearly not, and it's inclusion isn't an issue.

Lapras · 2024-12-12T19:53:20Z

I'm really looking forward to having this merged, however there are a couple of points of feedback.

The biggest ones are relating to weird whitespacing that doesn't match stylistically to the rest of the mhq codebase, and abbreviated variable names. This might seem petty, but we need to remain consistent where possible to make it easy for other contributors to drop in without having to learn multiple styles.

I've got no problem going through all these style changes, it's my first PR to the repo so I expected to have to readjust style a lot. I'm currently travelling, but I'll be able to readjust all of this over the weekend no problem.

Lapras added 8 commits November 23, 2024 17:47

initial GUI work for request PartsInUse Stock

0f1dabb

More GUI work, updating armor and ammo edge cases

94ccff3

More GUI work, updating armor and ammo edge cases

48f56f3

skeleton code for marshal/unmarshal, untested. As well as persistence…

1579f78

… between Campaign.java and the PartsReportDialog

finsihing touches on edge case for turrets, updated naming, finalized…

c5e7c38

… save/load systems and weekly checks.

Changing some language to be more clear

08d03f9

removing bloat files

29e07eb

removing bloat files

2a1db0d

IllianiCBT requested changes Dec 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Establishment of a Requested Stock % feature, and automating weekly checks to keep spare parts in stock #5321

Establishment of a Requested Stock % feature, and automating weekly checks to keep spare parts in stock #5321

Lapras commented Dec 8, 2024

IllianiCBT commented Dec 8, 2024

IllianiCBT commented Dec 8, 2024

HammerGS commented Dec 8, 2024

IllianiCBT commented Dec 12, 2024

IllianiCBT left a comment

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

IllianiCBT Dec 12, 2024

Lapras commented Dec 12, 2024

		@@ -2236,6 +2245,7 @@ private PartInUse getPartInUse(Part p) {
		p = ((MissingPart) p).getNewPart();

Establishment of a Requested Stock % feature, and automating weekly checks to keep spare parts in stock #5321

Are you sure you want to change the base?

Establishment of a Requested Stock % feature, and automating weekly checks to keep spare parts in stock #5321

Conversation

Lapras commented Dec 8, 2024

IllianiCBT commented Dec 8, 2024

IllianiCBT commented Dec 8, 2024

HammerGS commented Dec 8, 2024

IllianiCBT commented Dec 12, 2024

IllianiCBT left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lapras commented Dec 12, 2024