Improvement: Introduced Profession SubTypes for Easier Categorization #6914
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6914 +/- ##
=========================================
Coverage 11.71% 11.71%
- Complexity 6666 6670 +4
=========================================
Files 1101 1101
Lines 141414 141414
Branches 21901 21901
=========================================
+ Hits 16564 16568 +4
+ Misses 123146 123145 -1
+ Partials 1704 1701 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the profession categorization by introducing a new enum, PersonnelRoleSubType, to replace the previous boolean combat flag and additional checks. It updates constructors and role methods in PersonnelRole and adds corresponding localized labels in the resources.
- Introduces the PersonnelRoleSubType enum with three entries: COMBAT, SUPPORT, and CIVILIAN.
- Updates the PersonnelRole enum constructors, isCombat, isSupport, and isCivilian methods to use the new enum.
- Adds a properties file for localization labels for the new subtypes.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| MekHQ/src/mekhq/campaign/personnel/enums/PersonnelRoleSubType.java | New enum definition to replace boolean flags for role categorization. |
| MekHQ/src/mekhq/campaign/personnel/enums/PersonnelRole.java | Updated role evaluations and constructors to use PersonnelRoleSubType instead of booleans. |
| MekHQ/resources/mekhq/resources/PersonnelRoleSubType.properties | Provides localized labels for the new subtypes. |
| @@ -554,7 +569,7 @@ public boolean isVesselCrewMember() { | |||
| * otherwise. | |||
| */ | |||
| public boolean isSupport() { | |||
| return isSupport(false); | |||
| return isSubType(PersonnelRoleSubType.SUPPORT) || isSubType(PersonnelRoleSubType.CIVILIAN); | |||
There was a problem hiding this comment.
[nitpick] Consider updating the javadoc comment for isSupport to make clear that the method returns true for roles with either SUPPORT or CIVILIAN subtypes.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
The failure occurred in the PersonnelRoleTest.testIsSupport() unit test. The logs indicate that an AssertionFailedError occurred at PersonnelRoleTest.java:560, but I was unable to retrieve the PersonnelRoleTest.java file for further analysis. However, I can provide guidance based on the context available. Analysis The testIsSupport() method failed, likely due to an assertion mismatch. The workflow checks out specific branches from related repositories (MegaMek and MegaMekLab), falling back to master if the branch does not exist. |
Previously we were using a boolean to determine whether a profession was Combat or Not Combat; and then doing some additional checks to determine if the profession is Not Combat and Also Civilian.
Now we just use an enum with three entries: Combat, Support, and Civilian.
This is prep work for a minor GUI adjustment coming separately.