-
Notifications
You must be signed in to change notification settings - Fork 183
StratCon Advanced Terrain #3693
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
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #3693 +/- ##
============================================
- Coverage 10.65% 10.64% -0.02%
- Complexity 5472 5473 +1
============================================
Files 830 834 +4
Lines 113498 113677 +179
Branches 17154 17174 +20
============================================
+ Hits 12097 12099 +2
- Misses 100194 100372 +178
+ Partials 1207 1206 -1
☔ View full report in Codecov by Sentry. |
Oh wow this sounds awesome! |
So is the terrain thing in the nightly now or would have to use just the artifact build? |
Artifact build until the pull request is merged. It's currently missing multiple biome definitions so for a lot of biomes you'll just see the default terrain mix. |
All right, this baby's ready for review; code-wise at least. Still waiting on some art assets and map presets, but those don't require code review. |
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.
Well, some of this is very specific and hard to read, but no complaints from my side (FWIW). I have some suggestions.
@@ -356,9 +397,17 @@ private boolean drawHexes(Graphics2D g2D, DrawHexType drawHexType) { | |||
} | |||
} | |||
|
|||
// here we draw the coordinate labels | |||
if (drawHexType == DrawHexType.Hex) { | |||
g2D.setColor(Color.GREEN); |
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.
If you want, it should be possible to replace 402-410 with
new StringDrawer(currentCoords.toBTString()) .at(graphHex.xpoints[0] + xRadius / 2, graphHex.ypoints[0] + ((int) (g2D.getFontMetrics().getHeight() / 1.25))) .centerX() .color(Color.GREEN) .font(g2D.getFont().deriveFont(Font.BOLD, g2D.getFont().getSize())) .draw(g2D);
I had to guess for the x center, but the coords probably look better when x-centered. Also I struggled a bit with the font, as it is not set anywhere. I dont know what the font size is then, I'd guess 12. It could simplify things if the font size were set, e.g. at yRadius/3
float fontSize = yRadius/3;
new StringDrawer(currentCoords.toBTString())
.at(graphHex.xpoints[0] + xRadius / 2, graphHex.ypoints[0] + (int)(fontSize*1.2))
.center()
.color(Color.GREEN)
.font(g2D.getFont().deriveFont(Font.BOLD))
.fontSize(fontSize)
.draw(g2D);
The font and fontSize could be set outside of the loop, making the call simpler.
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.
The font is "whatever is the default"; you do bring up a good point about only setting it once. It looks pretty centered to me for the moment. I can revisit it if people report display issues with it.
Fix #3669 (not really, but I want it to auto-close when this is done)
When completed, this pull request will give StratCon the capability to generate a substantially greater variety of terrain. A few highlights: