-
Notifications
You must be signed in to change notification settings - Fork 294
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
Fix 6593: counterbattery icon hidden in double blind #6607
Fix 6593: counterbattery icon hidden in double blind #6607
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6607 +/- ##
============================================
- Coverage 29.11% 29.09% -0.02%
+ Complexity 15295 15277 -18
============================================
Files 2837 2837
Lines 279718 279737 +19
Branches 49286 49291 +5
============================================
- Hits 81429 81403 -26
- Misses 192820 192878 +58
+ Partials 5469 5456 -13 ☔ View full report in Codecov by Sentry. |
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 minor nitpick to ensure what is being compared is actually being compared.
Ideal world these conditions would be read as (I hope) as intended to do the math first THEM compare but it could be read as either.
if (GUIP.getUnitDisplayEnabled() && GUIP.getUnitDisplayLocaton() == 0) { | ||
// Move arrow inward if either side overlaps with the Unit Display | ||
Rectangle udRectangle = clientgui.getUnitDisplayDialog().getBounds(); | ||
if ((myRectangle.x + myRectangle.width + 5 > udRectangle.x && myRectangle.x <= udRectangle.x + udRectangle.width) || |
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.
Add some parenthesis to prevent ambiguity.
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.
Sorry, I popped out to a show.
I was on the fence about adding more parentheses: on the one hand, unambiguous; on the other, longer and more difficult to read.
Luckily the operator precedence is with us this time:
https://pages.cs.wisc.edu/~willb/cs302/java-operator-precedence.pdf
But in the future I'll add the parentheses.
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.
Checked with Co-Pilot and IDEA AI no issues detected
Fixes two issues:
Testing:
Close #6593