Merge pull request #7343 from IllianiBird/skillCheckUtilityReputationModifierBug

HammerGS · web-flow · commit 5eb80923cdaa · 2025-07-18T17:25:55.000-06:00
Fix: Fixed Skill Check Utility Not Correctly Using Reputation Modifiers
diff --git a/MekHQ/src/mekhq/campaign/Campaign.java b/MekHQ/src/mekhq/campaign/Campaign.java
@@ -3502,8 +3502,6 @@ public int getPatientsFor(Person doctor) {
             for (Person person : getActivePersonnel(false)) {
                 int effectiveMaxAcquisitions = defaultMaxAcquisitions;
 
-                PersonnelOptions options = person.getOptions();
-
                 if (isIneligibleToPerformProcurement(person, acquisitionCategory)) {
                     continue;
                 }
@@ -5278,7 +5276,10 @@ public void processNewDayPersonnel() {
             person.resetMinutesLeft(campaignOptions.isTechsUseAdministration());
             person.setAcquisition(0);
 
-            medicalController.processMedicalEvents(person);
+            medicalController.processMedicalEvents(person,
+                  campaignOptions.isUseAgeEffects(),
+                  isClanCampaign(),
+                  currentDay);
 
             processAnniversaries(person);
 
diff --git a/MekHQ/src/mekhq/campaign/personnel/medical/MedicalController.java b/MekHQ/src/mekhq/campaign/personnel/medical/MedicalController.java
@@ -36,6 +36,7 @@
 import static mekhq.campaign.personnel.skills.SkillType.S_SURGERY;
 import static mekhq.utilities.MHQInternationalization.getFormattedTextAt;
 
+import java.time.LocalDate;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -88,18 +89,35 @@ public MedicalController(Campaign campaign) {
         isUseMedicalController = campaignOptions.isUseAdvancedMedical();
     }
 
+    /**
+     * Use {@link #processMedicalEvents(Person, boolean, boolean, LocalDate)} instead
+     */
+    @Deprecated(since = "0.50.07", forRemoval = true)
+    public void processMedicalEvents(Person patient) {
+        processMedicalEvents(patient, false, false, LocalDate.of(3151, 1, 1));
+    }
 
     /**
-     * Processes the medical events for the given patient. This includes doctor-assisted healing,
-     * natural healing, and advanced medical rules (if enabled).
+     * Processes daily medical events for a given patient, handling both standard and advanced medical healing.
      *
-     * <p>For non-advanced medical rules, the method validates and assigns doctors and handles
-     * natural healing if the doctor cannot assist. For advanced medical rules, it delegates
-     * the processing to the advanced medical subsystem.</p>
+     * <p>The method orchestrates healing for a {@link Person} by applying doctor-assisted healing, natural healing
+     * rolls, and advanced medical rules, depending on campaign settings and the patient's needs.</p>
+     * <ul>
+     *   <li>If the patient requires healing and advanced medical rules are <b>not</b> enabled,
+     *       it attempts to validate and assign a doctor for assisted healing. If a doctor is not available or able
+     *       to help, it attempts natural healing instead. Successful natural healing is logged, and related
+     *       unit states are reset.</li>
+     *   <li>If advanced medical rules are enabled, the method defers the healing process to the advanced
+     *       medical subsystem and resets unit-related state as needed.</li>
+     * </ul>
      *
-     * @param patient the {@link Person} being healed
+     * @param patient           the {@link Person} undergoing healing
+     * @param isUseAgingEffects {@code true} if aging effects should be included when applying healing
+     * @param isClanCampaign    {@code true} if the campaign uses clan-based rules
+     * @param today             the current {@link LocalDate} for time-dependent calculations
      */
-    public void processMedicalEvents(Person patient) {
+    public void processMedicalEvents(Person patient, boolean isUseAgingEffects, boolean isClanCampaign,
+          LocalDate today) {
         Person doctor = campaign.getPerson(patient.getDoctorId());
 
         if (doctor != null) {
@@ -111,7 +129,7 @@ public void processMedicalEvents(Person patient) {
             patient.decrementDaysToWaitForHealing();
 
             if (doctor != null && patient.getDaysToWaitForHealing() <= 0) {
-                healPerson(patient, doctor);
+                healPerson(patient, doctor, isUseAgingEffects, isClanCampaign, today);
             } else if (checkNaturalHealing(patient)) {
                 // TODO change logging level from info to debug in 50.08
                 logger.info(getFormattedTextAt(RESOURCE_BUNDLE, "MedicalController.report.natural",
@@ -133,7 +151,6 @@ public void processMedicalEvents(Person patient) {
         }
     }
 
-
     /**
      * Checks if the patient can heal naturally (without a doctor) and processes the healing if possible.
      *
@@ -149,29 +166,39 @@ public boolean checkNaturalHealing(Person patient) {
         return false;
     }
 
+    /**
+     * Use {@link #healPerson(Person, Person, boolean, boolean, LocalDate)} instead
+     */
+    @Deprecated(since = "0.50.07", forRemoval = true)
+    private void healPerson(Person patient, Person doctor) {
+        healPerson(patient, doctor, false, false, LocalDate.of(3151, 1, 1));
+    }
 
     /**
-     * Heals the given patient with assistance from the specified doctor if they meet all necessary conditions.
+     * Applies medical treatment to the specified patient, using the given doctor as the medical provider.
      *
-     * <p>The method calculates modifiers using campaign rules, performs a skill check for the doctor, and updates
-     * the patient's healing status based on the result.</p>
+     * <p>This method performs a skill check for the doctor using current campaign rules and relevant situational
+     * modifiers. If the skill check succeeds, the patient is healed and any associated unit state is reset. Regardless
+     * of the outcome, the patient's healing waiting period is reset.</p>
      *
-     * @param patient the {@link Person} receiving medical treatment
-     * @param doctor  the {@link Person} performing the treatment
+     * @param patient           the {@link Person} receiving treatment
+     * @param doctor            the {@link Person} performing the medical treatment
+     * @param isUseAgingEffects {@code true} if aging effects should influence the healing process
+     * @param isClanCampaign    {@code true} if campaign-specific (clan) rules apply to healing
+     * @param today             the current date, used for time-dependent effects
      */
-    private void healPerson(Person patient, Person doctor) {
-        // TODO change logging level from info to debug in 50.08
-        logger.info(getFormattedTextAt(RESOURCE_BUNDLE, "MedicalController.report.intro",
+    private void healPerson(Person patient, Person doctor, boolean isUseAgingEffects, boolean isClanCampaign,
+          LocalDate today) {
+        logger.debug(getFormattedTextAt(RESOURCE_BUNDLE, "MedicalController.report.intro",
               doctor.getHyperlinkedFullTitle(), patient.getHyperlinkedFullTitle()));
 
         SkillCheckUtility skillCheckUtility = new SkillCheckUtility(doctor, S_SURGERY,
               getAdditionalHealingModifiers(patient),
               0,
               isUseSupportEdge,
-              false);
+              false, isUseAgingEffects, isClanCampaign, today);
 
-        // TODO change logging level from info to debug in 50.08
-        logger.info(skillCheckUtility.getResultsText());
+        logger.debug(skillCheckUtility.getResultsText());
 
         if (skillCheckUtility.isSuccess()) {
             patient.heal();
@@ -184,7 +211,6 @@ private void healPerson(Person patient, Person doctor) {
         patient.setDaysToWaitForHealing(healingWaitingPeriod);
     }
 
-
     /**
      * Retrieves additional healing modifiers for the given patient. These modifiers are based on campaign-specific
      * rules, such as staff shortages.
diff --git a/MekHQ/src/mekhq/campaign/personnel/skills/SkillCheckUtility.java b/MekHQ/src/mekhq/campaign/personnel/skills/SkillCheckUtility.java
@@ -43,6 +43,7 @@
 import static mekhq.utilities.ReportingUtilities.CLOSING_SPAN_TAG;
 import static mekhq.utilities.ReportingUtilities.spanOpeningWithCustomColor;
 
+import java.time.LocalDate;
 import java.util.List;
 
 import megamek.common.TargetRoll;
@@ -92,6 +93,27 @@ public class SkillCheckUtility {
     private int roll;
     private boolean usedEdge;
 
+    /**
+     * Use
+     * {@link SkillCheckUtility#SkillCheckUtility(Person, String, List, int, boolean, boolean, boolean, boolean,
+     * LocalDate)} instead
+     */
+    @Deprecated(since = "0.50.07", forRemoval = true)
+    public SkillCheckUtility(final Person person, final String skillName,
+          @Nullable List<TargetRollModifier> externalModifiers, final int miscModifier, final boolean useEdge,
+          final boolean includeMarginsOfSuccessText) {
+        this.person = person;
+        this.skillName = skillName;
+        SkillCheckUtility proxy = new SkillCheckUtility(person, skillName, externalModifiers, miscModifier, useEdge,
+                includeMarginsOfSuccessText,
+              false, false, LocalDate.of(3151, 1, 1));
+        marginOfSuccess = proxy.getMarginOfSuccess();
+        resultsText = proxy.getResultsText();
+        targetNumber = proxy.getTargetNumber();
+        roll = proxy.getRoll();
+        usedEdge = proxy.isUsedEdge();
+    }
+
     /**
      * Executes a skill check for the specified person and skill type.
      *
@@ -105,7 +127,8 @@ public class SkillCheckUtility {
      * include margins of success text as part of the results, if desired.</p>
      *
      * <p><b>Usage:</b> This constructor offers detailed control over the skill check process.
-     * For simpler use-cases, the {@link #performQuickSkillCheck(Person, String, List, int)} method
+     * For simpler use-cases, the
+     * {@link #performQuickSkillCheck(Person, String, List, int, boolean, boolean, LocalDate)} method
      * provides a more streamlined approach.</p>
      *
      * @param person                       the {@link Person} performing the skill check
@@ -125,13 +148,17 @@ public class SkillCheckUtility {
      *                                     attempt fails
      * @param includeMarginsOfSuccessText  whether to include detailed margins of success information
      *                                     in the results
+     * @param isUseAgingEffects            if {@code true}, considers aging effects during the check
+     * @param isClanCampaign               if {@code true}, applies rules specific to clan campaigns
+     * @param today                        the current date, used for time-dependent logic
      *
      * @author Illiani
      * @since 0.50.05
      */
     public SkillCheckUtility(final Person person, final String skillName,
           @Nullable List<TargetRollModifier> externalModifiers, final int miscModifier, final boolean useEdge,
-          final boolean includeMarginsOfSuccessText) {
+          final boolean includeMarginsOfSuccessText, boolean isUseAgingEffects, boolean isClanCampaign,
+          LocalDate today) {
         this.person = person;
         this.skillName = skillName;
 
@@ -141,7 +168,7 @@ public SkillCheckUtility(final Person person, final String skillName,
 
         final SkillType skillType = SkillType.getType(skillName);
         isCountUp = skillType.isCountUp();
-        targetNumber = determineTargetNumber(person, skillType, miscModifier);
+        targetNumber = determineTargetNumber(person, skillType, miscModifier, isUseAgingEffects, isClanCampaign, today);
 
         if (externalModifiers != null) {
             for (TargetRollModifier modifier : externalModifiers) {
@@ -152,6 +179,16 @@ public SkillCheckUtility(final Person person, final String skillName,
         performCheck(useEdge, includeMarginsOfSuccessText);
     }
 
+    /**
+     * Use {@link #performQuickSkillCheck(Person, String, List, int, boolean, boolean, LocalDate)} instead
+     */
+    @Deprecated(since = "0.50.07", forRemoval = true)
+    public static boolean performQuickSkillCheck(final Person person, final String skillName,
+          final @Nullable List<TargetRollModifier> externalModifiers, final int miscModifier) {
+        return performQuickSkillCheck(person, skillName, externalModifiers, miscModifier, false, false,
+              LocalDate.of(3151, 1, 1));
+    }
+
     /**
      * Performs a quick and simple skill check for a person based on the specified skill name.
      *
@@ -175,6 +212,9 @@ public SkillCheckUtility(final Person person, final String skillName,
      *                              <li>For non-'count up' skills, this value is added to the target number
      *                                  (i.e., positive values are penalties).</li>
      *                          </ul>
+     * @param isUseAgingEffects if {@code true}, considers aging effects during the check
+     * @param isClanCampaign    if {@code true}, applies rules specific to clan campaigns
+     * @param today             the current date, used for time-dependent logic
      *
      * @return {@code true} if the skill check succeeds, {@code false} otherwise
      *
@@ -184,9 +224,10 @@ public SkillCheckUtility(final Person person, final String skillName,
      * @since 0.50.05
      */
     public static boolean performQuickSkillCheck(final Person person, final String skillName,
-          final @Nullable List<TargetRollModifier> externalModifiers, final int miscModifier) {
+          final @Nullable List<TargetRollModifier> externalModifiers, final int miscModifier,
+          boolean isUseAgingEffects, boolean isClanCampaign, LocalDate today) {
         SkillCheckUtility skillCheck = new SkillCheckUtility(person, skillName, externalModifiers, miscModifier,
-              false, false);
+              false, false, isUseAgingEffects, isClanCampaign, today);
         return skillCheck.isSuccess();
     }
 
@@ -398,6 +439,14 @@ public boolean isUsedEdge() {
         return usedEdge;
     }
 
+    /**
+     * Use {@link #determineTargetNumber(Person, SkillType, int, boolean, boolean, LocalDate)} instead
+     */
+    @Deprecated(since = "0.50.07", forRemoval = true)
+    public static TargetRoll determineTargetNumber(Person person, SkillType skillType, int miscModifier) {
+        return determineTargetNumber(person, skillType, miscModifier, false, false, LocalDate.of(3151, 1, 1));
+    }
+
     /**
      * Determines the target number for a skill check based on the person's attributes, skill type, and whether they are
      * trained in the skill.
@@ -412,11 +461,15 @@ public boolean isUsedEdge() {
      *                     target number. This means negative values are bonuses, positive values are penalties.
      *
      * @return the target number for the skill check
+     * @param isUseAgingEffects if {@code true}, considers aging effects during the check
+     * @param isClanCampaign if {@code true}, applies rules specific to clan campaigns
+     * @param today        the current date, used for time-dependent logic
      *
      * @author Illiani
      * @since 0.50.05
      */
-    public static TargetRoll determineTargetNumber(Person person, SkillType skillType, int miscModifier) {
+    public static TargetRoll determineTargetNumber(Person person, SkillType skillType, int miscModifier,
+          boolean isUseAgingEffects, boolean isClanCampaign, LocalDate today) {
         final String skillName = skillType.getName();
         final Attributes characterAttributes = person.getATOWAttributes();
 
@@ -441,7 +494,7 @@ public static TargetRoll determineTargetNumber(Person person, SkillType skillTyp
             Skill skill = person.getSkill(skillName);
             int skillValue = skill.getFinalSkillValue(person.getOptions(),
                   person.getATOWAttributes(),
-                  person.getReputation());
+                  person.getAdjustedReputation(isUseAgingEffects, isClanCampaign, today, person.getRankNumeric()));
             targetNumber.addModifier(skillValue, skillName);
         }
 
diff --git a/MekHQ/src/mekhq/campaign/stratcon/StratconRulesManager.java b/MekHQ/src/mekhq/campaign/stratcon/StratconRulesManager.java
@@ -1666,7 +1666,8 @@ public static ReinforcementResultsType processReinforcementDeployment(Force forc
         int targetNumber = 9;
         Skill tactics = commander.getSkill(S_TACTICS);
 
-        SkillCheckUtility skillCheckUtility = new SkillCheckUtility(commander, S_TACTICS, null, 0, true, false);
+        SkillCheckUtility skillCheckUtility = new SkillCheckUtility(commander, S_TACTICS, null, 0, true, false,
+              campaign.getCampaignOptions().isUseAgeEffects(), campaign.isClanCampaign(), campaign.getLocalDate());
         campaign.addReport(skillCheckUtility.getResultsText());
 
         if (skillCheckUtility.isSuccess()) {
diff --git a/MekHQ/src/mekhq/gui/dialog/SkillCheckDialog.java b/MekHQ/src/mekhq/gui/dialog/SkillCheckDialog.java
diff --git a/MekHQ/unittests/mekhq/campaign/personnel/skills/SkillCheckUtilityTest.java b/MekHQ/unittests/mekhq/campaign/personnel/skills/SkillCheckUtilityTest.java
