8000 Added Skill Deprecation Tool For Managing Deprecated Skills; Deprecated the 'Scrounge' Skill by IllianiBird · Pull Request #6430 · MegaMek/mekhq · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 4 commits into from
Mar 26, 2025

Conversation

IllianiBird
Copy link
Collaborator
  • Introduced SkillDeprecationTool to handle deprecated skills for personnel in campaigns.
  • Added functionality for identifying deprecated skills, providing XP refunds, and managing skill removal.
  • Implemented Immersive Dialogs to alert user to skill deprecation.
  • Deprecated the "Scrounge" skill.

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.

image

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 the DEPRECATED_SKILLS list. Users loading campaigns containing characters with a deprecated skill are then prompted to refund any xp spent on the deprecated skill.

image

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.

…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.
- Marked `SCROUNGE` column in `PersonnelTableModelColumn` as `@Deprecated` since version `0.50.05`.
- Added deprecation warning to indicate future removal of the column.
@IllianiBird IllianiBird added UX User experience Utility A new utility function or developer tool labels Mar 26, 2025
@IllianiBird IllianiBird self-assigned this Mar 26, 2025
Copy link
Collaborator Author

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

Potential uncaught 'java.lang.NumberFormatException'.
);
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

Expression always evaluates to the same value.
} 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

Potential uncaught 'java.lang.NumberFormatException'.
@@ -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

The parameter 'version' is never used.
Copy link
codecov bot commented Mar 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 11.27%. Comparing base (35b967b) to head (d4a5af3).
Report is 20 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

# Conflicts:
#	MekHQ/resources/mekhq/resources/CampaignOptionsDialog.properties
@IllianiBird IllianiBird merged commit 4debaef into MegaMek:master Mar 26, 2025
3 checks passed
@IllianiBird IllianiBird deleted the deprecatedScrounge branch March 31, 2025 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Utility A new utility function or developer tool UX User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0