-
Notifications
You must be signed in to change notification settings - Fork 291
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
Issue 4259: Fixing Dropship Heat-by-Arc for Players and Princess #6276
Issue 4259: Fixing Dropship Heat-by-Arc for Players and Princess #6276
Conversation
…grounded dropships to use individual weapons with heat-by-arc
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6276 +/- ##
============================================
- Coverage 28.99% 28.95% -0.05%
- Complexity 13983 14016 +33
============================================
Files 2652 2672 +20
Lines 268306 269228 +922
Branches 47768 47910 +142
============================================
+ Hits 77803 77949 +146
- Misses 186620 187379 +759
- Partials 3883 3900 +17 ☔ View full report in Codecov by Sentry. |
…grounded dropships to use individual weapons with heat-by-arc
…all-weapons' into issue-4259-princess-doesnt-fire-all-weapons
…event dereferences variable error
…rrentHeatBuildup for bays should use getHeatByBay
This is ready for review. I would appreciate a code review for this as it's more than just a trivial change. Thanks! |
megamek/src/megamek/client/ui/swing/unitDisplay/WeaponPanel.java
Outdated
Show resolved
Hide resolved
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.
A couple of questions but overall looks good. I appreciate the code clean-up and safety checks you've added as well.
…sure non-large craft with bays will work
…prove Readability of statement
Thank you @Sleet01! I made some changes at your suggestion. |
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.
LGTM!
Fixes #4259
It works, Princess now fires as many weapons as she can when using the default Dropship heat rules, without going over the heat limit of dropships.
The original bug wasn't present (only using Aft weapons & the NPE), but Princess & grounded dropships were still doing funny stuff.
Bugs resolved: