Skip to content
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

Remove User Preferences Handling from BatchXPDialog #5550

Merged
merged 1 commit into from
Dec 29, 2024

Conversation

IllianiCBT
Copy link
Collaborator

@IllianiCBT IllianiCBT commented Dec 28, 2024

Removed the entire setUserPreferences method, along with associated imports.

Storing preferences caused a lot of fragility with this system. It was very easy for a user to accidentally brick their Mass Training dialog by moving between different campaigns if those campaigns had different skill set ups. For example, if Campaign A had Piloting/Mek cap out at 8, while B had it cap out at 10. Moving from B to A could prevent the Mass Training dialog being used entirely if the user had their target level set to 9 or 10.

User preferences, in this instance, just stored the last known configuration for the Mass Training dialog. Given how few options there are - and the fact that it doesn't appear to have stored configurations on a per-skill basis - I opted to remove preferences entirely.

Fixes #5542

Removed the entire `setUserPreferences` method, as it was no longer in use, along with associated imports. This cleanup reduces unnecessary code complexity and ensures better maintainability.
@IllianiCBT IllianiCBT added Bug Severity: Medium Issues described as medium severity as per the new issue form labels Dec 28, 2024
@IllianiCBT IllianiCBT self-assigned this Dec 28, 2024
@codecov-commenter
Copy link

codecov-commenter commented Dec 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.02%. Comparing base (2af7e1a) to head (449266e).
Report is 18 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #5550      +/-   ##
============================================
- Coverage     10.03%   10.02%   -0.02%     
+ Complexity     6053     6036      -17     
============================================
  Files          1073     1073              
  Lines        141198   141173      -25     
  Branches      20565    20566       +1     
============================================
- Hits          14175    14151      -24     
+ Misses       125662   125660       -2     
- Partials       1361     1362       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IllianiCBT IllianiCBT merged commit aad5c67 into MegaMek:master Dec 29, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Severity: Medium Issues described as medium severity as per the new issue form
Projects
None yet
Development

Successfully merging this pull request may close these issues.

12/26 Nightly / 0.50.02 SNAPSHOT - Mass Training dialogue won't allow adjustment of Target Skill Level field.
2 participants