-
Notifications
You must be signed in to change notification settings - Fork 176
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
add tools to manage SP/VP; track intensity visibility #2725
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2725 +/- ##
============================================
- Coverage 10.60% 10.60% -0.01%
- Complexity 3853 3856 +3
============================================
Files 717 718 +1
Lines 99774 99850 +76
Branches 16404 16413 +9
============================================
+ Hits 10585 10589 +4
- Misses 87812 87885 +73
+ Partials 1377 1376 -1
Continue to review full report at Codecov.
|
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.
I'm not the biggest fan of how this is implemented, but think it in general looks okay for an alpha dialog subject to future change. Just a few comments first.
@@ -56,6 +60,8 @@ | |||
private JScrollPane expandedObjectivePanel; | |||
private boolean objectivesCollapsed = true; | |||
|
|||
CampaignManagementDialog cmd; |
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.
I can see no good reason to have this always initialized and not just recreate it as required.
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.
On the other hand, I don't see a reason to keep creating/destroying a bunch of buttons and whatnot.
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.
That's how we handle it in MekHQ, and... permanent memory vs clearable memory regularly matters when using a weaker computer.
Co-authored-by: Justin Bowen <[email protected]>
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.
Looks good for an initial setup, albeit with the concerns voiced previously.
Adds some tools for users to manage SP/VP, including GM mode tools; shows track scenario odds in GM mode.