-
Notifications
You must be signed in to change notification settings - Fork 188
Added Skill Deprecation Tool For Managing Deprecated Skills; Deprecated the 'Scrounge' Skill #6430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ed the 'Scrounge' Skill - Introduced `SkillDeprecationTool` to handle deprecated skills for personnel in campaigns. - Added functionality for identifying deprecated skills, providing XP refunds, and managing skill removal. - Implemented dialogs with both in-character and out-of-character messaging for user interaction. - Added initial support for "Scrounge" skill as a deprecated skill example.
… for the deprecated skills list.
- Marked `SCROUNGE` column in `PersonnelTableModelColumn` as `@Deprecated` since version `0.50.05`. - Added deprecation warning to indicate future removal of the column.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When reviewing this is the only class you really need to pay attention to. The reset are just bog standard deprecations or method calls.
} else if (xn.equalsIgnoreCase("shipSearchType")) { | ||
retVal.setShipSearchType(Integer.parseInt(wn.getTextContent())); | ||
campaign.setShipSearchType(Integer.parseInt(wn.getTextContent())); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
); | ||
campaign.setAutoResolveBehaviorSettings(firstNonNull(BehaviorSettingsFactory.getInstance() | ||
.getBehavior(wn.getTextContent()), | ||
BehaviorSettingsFactory.getInstance().DEFAULT_BEHAVIOR)); |
Check warning
Code scanning / CodeQL
Expression always evaluates to the same value Warning
} else if (xn.equalsIgnoreCase("temporaryPrisonerCapacity")) { | ||
retVal.setTemporaryPrisonerCapacity(Integer.parseInt(wn.getTextContent().trim())); | ||
campaign.setTemporaryPrisonerCapacity(Integer.parseInt(wn.getTextContent().trim())); |
Check notice
Code scanning / CodeQL
Missing catch of NumberFormatException Note
@@ -2454,8 +2495,8 @@ | |||
MHQXMLUtility.writeSimpleXMLCloseTag(pw, --indent, "person"); | |||
} | |||
|
|||
public static Person generateInstanceFromXML(Node wn, Campaign c, Version version) { | |||
Person retVal = new Person(c); | |||
public static Person generateInstanceFromXML(Node wn, Campaign campaign, Version version) { |
Check notice
Code scanning / CodeQL
Useless parameter Note
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6430 +/- ##
============================================
- Coverage 11.29% 11.27% -0.02%
+ Complexity 6285 6280 -5
============================================
Files 1071 1072 +1
Lines 136316 136363 +47
Branches 21084 21089 +5
============================================
- Hits 15393 15377 -16
- Misses 119385 119453 +68
+ Partials 1538 1533 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# Conflicts: # MekHQ/resources/mekhq/resources/CampaignOptionsDialog.properties
SkillDeprecationTool
to handle deprecated skills for personnel in campaigns.Dev Notes
Around 50.02 we removed the Clan Tech Knowledge SPA. This was in response to official errata. At the time I noted that the way we handled the removal wasn't great. As SPAs are expensive, and that was potentially a lot of XP we just poof'd away. Not a great user experience.
Following a poll on Discord (see above) we decided to officially deprecate the 'Scrounge' skill. Not wanting to repeat the mistakes of the past I have created the
SkillDeprecationTool
to handle skill deprecations. Now, whenever we want to Deprecate a skill we just add it to theDEPRECATED_SKILLS
list. Users loading campaigns containing characters with a deprecated skill are then prompted to refund any xp spent on the deprecated skill.Originally, the intent was to refund Scrounge when the skill was removed, however that created an order of operations issue. How can we refund a skill that has been completely removed? In short, we can't. At least not easily.