Add outline to unit count and damage counts #3488
Conversation
| } | ||
| } | ||
|
|
||
| private static void drawOutlinedText(final Graphics2D graphics, final String s, final int x, final int y, |
There was a problem hiding this comment.
This method is the core change. Everything else is pretty boiler plate.
Codecov Report
@@ Coverage Diff @@
## master #3488 +/- ##
============================================
- Coverage 21.96% 21.81% -0.16%
- Complexity 5920 5952 +32
============================================
Files 836 836
Lines 72017 73224 +1207
Branches 11596 11955 +359
============================================
+ Hits 15821 15976 +155
- Misses 54107 55161 +1054
+ Partials 2089 2087 -2
Continue to review full report at Codecov.
|
|
Looks good Red. Well done as usual. |
There was a problem hiding this comment.
CodeChanges look good, however I'm seeing a lot of pretty similar code being repeated an repeated again.
I'm not sure if there's a good way to solve this though.
In any case I'll try to review your current PRs in detail tomorrow (getting pretty late for me here)
|
@RoiEXLab Yeah, a lot of it is setter/getter/reset properties boiler plate code. Probably could create some sort of utility to try to simplify them a bit but didn't really spend time digging into them just followed the existing pattern as the main change updating how counts are draw and the properties were kind of just a customization add-on for outlines. |
|
|
||
| public static void resetPropertyUnitHitDamageOutline() { | ||
| final Preferences pref = Preferences.userNodeForPackage(MapImage.class); | ||
| pref.remove(PROPERTY_UNIT_HIT_DAMAGE_OUTLINE_STRING); |
There was a problem hiding this comment.
I think it really wouldn't hurt to just create a static helper method for all of those reset methods that accepts a string it then removes.
This helper could then be tested in a simple unit test and would increase coverage a little bit
There was a problem hiding this comment.
Yeah, added a reset helper method.
| public static Color getPropertyUnitHitDamageColor() { | ||
| if (propertyUnitHitDamageColor == null) { | ||
| final Preferences pref = Preferences.userNodeForPackage(MapImage.class); | ||
| propertyUnitHitDamageColor = | ||
| new Color(pref.getInt(PROPERTY_UNIT_HIT_DAMAGE_COLOR_STRING, Color.black.getRGB())); | ||
| new Color(pref.getInt(PROPERTY_UNIT_HIT_DAMAGE_COLOR_STRING, UNIT_HIT_DAMAGE_COLOR_DEFAULT.getRGB())); | ||
| } | ||
| return propertyUnitHitDamageColor; | ||
| } | ||
|
|
There was a problem hiding this comment.
Similarly, because the setters and getters vary too much, you could just create a getPrefs method that simply returns a Preferences object which can be chained with a set or get call directly.
Makes the code shorter, and again allows those methods to be a little bit more tested
There was a problem hiding this comment.
Not really sure much can be done for get/set as the variation is too high without doing significant refactoring or only covering a portion of them. Only thing is literally creating a 1 line helper method for Preferences.userNodeForPackage(MapImage.class). Which maybe makes each method a bit shorter but adds indirection. Not sure that's even worth it.
Addresses: https://forums.triplea-game.org/topic/917/inception-a-dream-within-a-dream-unit-stack-numbers-display
Before:
After:
Font Properties:
