Merge pull request #7844 from IllianiBird/attributeRandomizationImprovements

Improvement: Improved Randomization of Traits & Attribute Scores

Author: HammerGS

MekHQ/resources/mekhq/resources/CampaignOptionsDialog.properties

@@ -1495,23 +1495,28 @@ lblUseAttributes.tooltip=If checked, characters will be generated with <i>A Time
   <br><b>Warning:</b> If enabled (or disabled) mid-campaign, you need to navigate to the personnel tab, and (while in\
   \ GM Mode) you should use the right-click menu to reset attribute scores for all personnel.
 lblRandomizeAttributes.text=Random Attributes
-lblRandomizeAttributes.tooltip=If checked, a d6 is rolled for each of the character's ATOW Attributes.\
+lblRandomizeAttributes.tooltip=If checked, a 2d6 is rolled for each of the character's ATOW Attributes.\
   <br>\
-  <br>On a roll of a 6 the Attribute is increased by 1. On a roll of a 1 the Attribute is decreased\
-  \ by 1.
+  <br>- 2: score reduced by 2\
+  <br>- 3-5: score reduced by 1\
+  <br>- 6-8: score stays the same\
+  <br>- 9-11: score increased by 1\
+  <br>- 12: score increased by 2
 lblDisplayAllAttributes.text=Display All Attributes <span style="color:#C344C3;">\u2605</span>
 lblDisplayAllAttributes.tooltip=If checked, a character's profile will display all of their ATOW Attributes. \
   Otherwise, only those attributes with a skill check modifier will be displayed. Even if this option is disabled, \
   all attributes can be seen by selecting the 'attributes' filter in the personnel table.
 lblRandomizeTraits.text=Randomize Traits
-lblRandomizeTraits.tooltip=If checked, a newly created character's Connections, Wealth, Reputation, Unlucky, and \
-  Bloodmark scores are randomized.\
-  <br>\
-  <br>- For <b>Connections</b> there is a 1-in-6 chance the character has rank 1.\
-  <br>- For <b>Wealth</b> and <b>Reputation</b> there is a 1-in-6 chance the character has rank 1 and 1-in-6 they have \
-  rank -1.\
-  <br>- For <b>Unlucky</b> there is a 1-in-20 chance the character has rank 1\
-  <br>- For <b>Bloodmark</b> there is a 1-in-100 chance the character has rank 1. Increased to 1-in-50 for pirates.
+lblRandomizeTraits.tooltip=If checked, a 2d6 is rolled for each of the character's ATOW Traits (within the confines \
+  of that Trait's minimum and maximum values).\
+  <br>\
+  <br>- 2: score reduced by 2\
+  <br>- 3-5: score reduced by 1\
+  <br>- 6-8: score stays the same\
+  <br>- 9-11: score increased by 1\
+  <br>- 12: score increased by 2\
+  <br>- For <b>Bloodmark</b> there is a 99% chance of no change. Increased to 11% for pirates. <b>Unlucky</b> is \
+  reduced to 5%.
 lblAllowMonthlyReinvestment.text=Allow Monthly Reinvestment of Wealth
 lblAllowMonthlyReinvestment.tooltip=Each month the campaign commander will use their Wealth trait to\
   \ reinvest money back into the campaign.\

MekHQ/src/mekhq/campaign/personnel/generator/DefaultSkillGenerator.java

@@ -32,9 +32,13 @@
  */
 package mekhq.campaign.personnel.generator;
 
+import static megamek.codeUtilities.MathUtility.clamp;
 import static megamek.common.compute.Compute.d6;
 import static megamek.common.compute.Compute.randomInt;
+import static mekhq.campaign.personnel.Person.*;
 import static mekhq.campaign.personnel.skills.Attributes.DEFAULT_ATTRIBUTE_SCORE;
+import static mekhq.campaign.personnel.skills.Attributes.MAXIMUM_ATTRIBUTE_SCORE;
+import static mekhq.campaign.personnel.skills.Attributes.MINIMUM_ATTRIBUTE_SCORE;
 import static mekhq.campaign.personnel.skills.InfantryGunnerySkills.INFANTRY_GUNNERY_SKILLS;
 import static mekhq.campaign.personnel.skills.SkillDeprecationTool.DEPRECATED_SKILLS;
 import static mekhq.campaign.personnel.skills.SkillType.EXP_ELITE;
@@ -55,6 +59,7 @@
 import mekhq.campaign.personnel.Person;
 import mekhq.campaign.personnel.enums.PersonnelRole;
 import mekhq.campaign.personnel.enums.Phenotype;
+import mekhq.campaign.personnel.skills.Attributes;
 import mekhq.campaign.personnel.skills.RandomSkillPreferences;
 import mekhq.campaign.personnel.skills.SkillType;
 import mekhq.campaign.personnel.skills.Skills;
@@ -183,6 +188,31 @@ private static void generateCommandUtilitySkills(Person person, int expLvl,
         }
     }
 
+    /**
+     * Generates and assigns attribute scores for the specified person based on their profession, phenotype, and
+     * randomization settings.
+     *
+     * <p>This method performs the following steps:</p>
+     * <ol>
+     *     <li><b>Reset:</b> All attributes are reset to {@link Attributes#DEFAULT_ATTRIBUTE_SCORE}</li>
+     *     <li><b>Early Exit:</b> If attributes are disabled via {@link RandomSkillPreferences#isUseAttributes()},
+     *         the method returns immediately</li>
+     *     <li><b>Base Assignment:</b> Attribute scores are calculated by combining:
+     *         <ul>
+     *             <li>Profession-based modifiers from {@link PersonnelRole#getAttributeModifier(SkillAttribute)}</li>
+     *             <li>Phenotype-based modifiers from {@link Phenotype#getAttributeModifier(SkillAttribute)}</li>
+     *         </ul>
+     *     </li>
+     *     <li><b>Randomization:</b> If enabled via {@link RandomSkillPreferences#isRandomizeAttributes()}, each
+     *     attribute receives an additional random adjustment using {@link #performTraitRoll()}, which produces
+     *     values ranging from -2 to +2</li>
+     * </ol>
+     *
+     * <p>All final attribute scores are clamped within the valid range defined by
+     * {@link Attributes#MINIMUM_ATTRIBUTE_SCORE} and {@link Attributes#MAXIMUM_ATTRIBUTE_SCORE}.</p>
+     *
+     * @param person the {@link Person} whose attributes will be generated and assigned
+     */
     @Override
     public void generateAttributes(Person person) {
         RandomSkillPreferences skillPreferences = getSkillPreferences();
@@ -216,79 +246,127 @@ public void generateAttributes(Person person) {
             person.setAttributeScore(attribute, baseAttributeScore + attributeModifier);
 
             // Attribute randomization
-            int roll = d6();
             if (randomizeAttributes) {
-                if (roll == 1) {
-                    person.changeAttributeScore(attribute, -1);
-                } else if (roll == 6) {
-                    person.changeAttributeScore(attribute, 1);
+                int delta = clamp(performTraitRoll(), MINIMUM_ATTRIBUTE_SCORE, MAXIMUM_ATTRIBUTE_SCORE);
+                if (delta != 0) {
+                    person.changeAttributeScore(attribute, delta);
                 }
             }
         }
     }
 
     /**
-     * Generates traits for the specified person based on random or pre-determined criteria.
+     * Generates traits for the specified person based on random rolls.
      *
-     * <p>When randomization is enabled, this method calculates and assigns specific traits such as connections,
-     * reputation, wealth, and bad luck using random rolls. Each trait has its own set of rules for adjustment.</p>
+     * <p>When randomization is enabled via {@link RandomSkillPreferences#isRandomizeTraits()}, this method
+     * assigns the following traits using 2d6-based rolls that produce values ranging from -2 to +2:</p>
      *
-     * @param person The person whose traits will be updated. Traits are adjusted based on random rolls when
-     *               randomization is enabled.
+     * <ul>
+     *     <li><b>Connections</b>: Social network strength (clamped to valid range)</li>
+     *     <li><b>Reputation</b>: Public standing and renown (clamped to valid range)</li>
+     *     <li><b>Wealth</b>: Personal financial resources (clamped to valid range)</li>
+     *     <li><b>Unlucky</b>: Degree of bad fortune (clamped to valid range)</li>
+     *     <li><b>Bloodmark</b>: Clan honor debt (assigned only on rare occasions)</li>
+     * </ul>
+     *
+     * <p><b>Bloodmark Assignment:</b></p>
+     * <ul>
+     *     <li>Pirates: ~11.11% chance of receiving a bloodmark</li>
+     *     <li>Non-pirates: ~1.11% chance of receiving a bloodmark</li>
+     *     <li>Severity is determined by {@link #performBloodmarkRoll()}, producing values 0-2</li>
+     * </ul>
+     *
+     * <p>If trait randomization is disabled, no traits are modified.</p>
+     *
+     * @param person the {@link Person} whose traits will be generated and assigned
+     *
+     * @see #performTraitRoll()
+     * @see #performBloodmarkRoll()
      */
     @Override
     public void generateTraits(Person person) {
         if (!getSkillPreferences().isRandomizeTraits()) {
             return;
         }
 
-        // Connections
-        if (d6() == 6) {
-            person.setConnections(1);
-        } else {
-            person.setConnections(0);
-        }
-
-        // Reputation
-        int roll = d6();
-        if (roll == 6 || roll == 1) {
-            person.setReputation(roll == 6 ? 1 : -1);
-        } else {
-            person.setReputation(0);
-        }
+        person.setConnections(clamp(performTraitRoll(), MINIMUM_CONNECTIONS, MAXIMUM_CONNECTIONS));
+        person.setReputation(clamp(performTraitRoll(), MINIMUM_REPUTATION, MAXIMUM_REPUTATION));
+        person.setWealth(clamp(performTraitRoll(), MINIMUM_WEALTH, MAXIMUM_WEALTH));
+        person.setExtraIncomeFromTraitLevel(clamp(performTraitRoll(), MINIMUM_EXTRA_INCOME, MAXIMUM_EXTRA_INCOME));
 
-        // Wealth
-        roll = d6();
-        if (roll == 6 || roll == 1) {
-            person.setWealth(roll == 6 ? 1 : -1);
-        } else {
-            person.setWealth(0);
+        int baseUnluckyDiceSize = 5;
+        int unluckyRoll = randomInt(baseUnluckyDiceSize);
+        if (unluckyRoll == 0) { // 5% chance of positive value
+            person.setUnlucky(clamp(performTraitRoll(), MINIMUM_UNLUCKY, MAXIMUM_UNLUCKY));
         }
-
-        // Extra Income
-        roll = d6();
-        if (roll == 6 || roll == 1) {
-            person.setExtraIncomeFromTraitLevel(roll == 6 ? 1 : -1);
-        } else {
-            person.setExtraIncomeFromTraitLevel(0);
+        // We want the chance of a Bloodmark to be low as it can be quite disruptive
+        int baseBloodmarkDiceSize = person.getOriginFaction().isPirate() ? 5 : 50;
+        // pirates = approx 11.11% chance of a bloodmark
+        // non-pirates = approx 1.11% chance of a bloodmark
+        int bloodmarkRoll = randomInt(baseBloodmarkDiceSize);
+        if (bloodmarkRoll == 0) {
+            person.setBloodmark(clamp(performBloodmarkRoll(), MINIMUM_BLOODMARK, MAXIMUM_BLOODMARK));
         }
+    }
 
-        // Unlucky
-        roll = randomInt(20);
-        if (roll == 0) {
-            person.setUnlucky(1);
-        } else {
-            person.setUnlucky(0);
-        }
+    /**
+     * Performs a 2d6 roll to determine a trait modifier value.
+     *
+     * <p>This method rolls two six-sided dice and converts the result into a trait modifier
+     * using the following distribution:</p>
+     * <ul>
+     *     <li><b>2</b>: returns {@code -2} (exceptional negative trait)</li>
+     *     <li><b>3-5</b>: returns {@code -1} (below average trait)</li>
+     *     <li><b>6-8</b>: returns {@code 0} (average trait)</li>
+     *     <li><b>9-11</b>: returns {@code 1} (above average trait)</li>
+     *     <li><b>12</b>: returns {@code 2} (exceptional positive trait)</li>
+     * </ul>
+     *
+     * <p>This creates a bell curve distribution centered on average (0), with exceptional results being rare.</p>
+     *
+     * @return a trait modifier value ranging from {@code -2} to {@code 2}
+     *
+     * @author Illiani
+     * @since 0.50.10
+     */
+    private static int performTraitRoll() {
+        int roll = d6(2);
+        return switch (roll) {
+            case 2 -> -2;
+            case 3, 4, 5 -> -1;
+            case 9, 10, 11 -> 1;
+            case 12 -> 2;
+            default -> 0;
+        };
+    }
 
-        // Bloodmark
-        // We want the chance of a Bloodmark to be low as it can be quite disruptive
-        roll = randomInt(person.getOriginFaction().isPirate() ? 50 : 100);
-        if (roll == 0) {
-            person.setBloodmark(1);
-        } else {
-            person.setBloodmark(0);
-        }
+    /**
+     * Performs a 2d6 roll to determine a bloodmark severity value.
+     *
+     * <p>This method rolls two six-sided dice and converts the result into a bloodmark value
+     * using the following distribution:</p>
+     *
+     * <ul>
+     *     <li><b>2 or 12</b>: returns {@code 2} (~5.56% chance) - severe bloodmark</li>
+     *     <li><b>3-5 or 9-11</b>: returns {@code 1} (~50% chance) - moderate bloodmark</li>
+     *     <li><b>6-8</b>: returns {@code 0} (~44.44% chance) - no bloodmark assigned</li>
+     * </ul>
+     *
+     * <p>This creates a bell curve distribution where most results produce a moderate bloodmark, with severe
+     * bloodmarks being rare and no bloodmark being moderately common.</p>
+     *
+     * @return a bloodmark severity value: {@code 0} (none), {@code 1} (moderate), or {@code 2} (severe)
+     *
+     * @author Illiani
+     * @since 0.50.10
+     */
+    private static int performBloodmarkRoll() {
+        int roll = d6(2);
+        return switch (roll) {
+            case 2, 12 -> 2;
+            case 3, 4, 5, 9, 10, 11 -> 1;
+            default -> 0;
+        };
     }
 
     /**