Improvement: When Selecting Change Profession Categorize Professions Based on SubType #6915
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6915 +/- ##
============================================
- Coverage 11.71% 11.71% -0.01%
- Complexity 6668 6670 +2
============================================
Files 1100 1101 +1
Lines 141383 141414 +31
Branches 21891 21901 +10
============================================
Hits 16568 16568
- Misses 123115 123144 +29
- Partials 1700 1702 +2 ☔ 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 refines the categorization of professions by introducing a PersonnelRoleSubType to separate roles into Combat, Support, and Civilian, thereby cleaning up the assignment logic for personnel with many professions. It updates the popup menus in the GUI to reflect these categories, refactors the PersonnelRole enum to use the new subtype concept, and adjusts resource strings accordingly.
- Updated enum constructors and methods in PersonnelRole for subtype handling
- Added categorized submenu construction for primary and secondary roles in PersonnelTableMouseAdapter
- Introduced PersonnelRoleSubType enum and updated resource property labels
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| MekHQ/src/mekhq/gui/adapter/PersonnelTableMouseAdapter.java | Revised popup menu creation with categorized submenus based on role subtype |
| MekHQ/src/mekhq/campaign/personnel/enums/PersonnelRoleSubType.java | New enum to represent role subtypes with internationalized labels |
| MekHQ/src/mekhq/campaign/personnel/enums/PersonnelRole.java | Updated enum to use PersonnelRoleSubType and refactored role boolean methods accordingly |
| MekHQ/resources/mekhq/resources/PersonnelRoleSubType.properties | Added labels for the new role subtypes |
| MekHQ/resources/mekhq/resources/GUI.properties | Updated strings to reference “Profession” instead of “Role” along with subtype labels |
Comments suppressed due to low confidence (1)
MekHQ/src/mekhq/campaign/personnel/enums/PersonnelRole.java:569
- [nitpick] The change in isSupport() now includes civilian roles, but the Javadoc still references 'support' in a general manner. Consider updating the documentation to clearly explain that civilian roles are treated as support in this context.
public boolean isSupport() { return isSubType(PersonnelRoleSubType.SUPPORT) || isSubType(PersonnelRoleSubType.CIVILIAN);
| if (menuSupportPrimary.getItemCount() > 0) { | ||
| menu.add(menuSupportPrimary); | ||
| } | ||
| if (menuCivilianPrimary.getItemCount() > 0) { |
There was a problem hiding this comment.
The logic for creating and adding categorized submenus for both primary and secondary roles is duplicated. Consider refactoring this logic into a helper method to streamline future updates.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
As we need to return three different, unique, lists to make this fit with legacy code it would be a pain to do.
In the land of wishes and fairies I'd have the time to completely rewrite and modernize these context menus, but that is not today.
Requires #6914
This change makes assignment cleaner for personnel who qualify for a large number of professions and is prep work for an upcoming feature.