-
-
Notifications
You must be signed in to change notification settings - Fork 11
Fractal Variety #472
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
base: develop
Are you sure you want to change the base?
Fractal Variety #472
Conversation
WalkthroughAdds fractal-driven terrain configuration (new records), replaces flooded-multilevel variants with four fractal variants, adds fractal terrain/style generators, updates resource and mex placement logic, extends mask utilities for slope-based flattening and blur, and introduces water-area handling and spawn-mask/ramp pipelines. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Orchestrator as Generator orchestrator
participant Style as StyleGenerator (Fractal*)
participant Terrain as Fractal*LastTerrainGenerator
participant Core as FractalNoiseLastTerrainGenerator
participant Mask as MapMaskMethods
participant Mex as MexPlacer
Orchestrator->>Style: request style
Style->>Terrain: instantiate & initialize(map, seed, params, symmetry, pipeline)
Terrain->>Terrain: assign FractalParams (with FractalFlattenParams[])
Terrain->>Core: super.initialize(...)
Core->>Core: init rampNoise & optional random waterMask
Core->>Core: for each FractalFlattenParams -> create ramp layer & flatten band
Core->>Core: blurRamps()
Core->>Mask: flattenHeightBand(noiseMap, min,max,destMin,destMax,slope,blur)
Mask-->>Core: flattened bands
Core->>Core: setupSpawnMaskPipeline()
Core->>Mex: placeMexes(... spacing, spawnRadius, remainingRadius ...)
Mex-->>Orchestrator: placed mexes
Core-->>Orchestrator: finalized terrain and spawn data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
...or/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.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.
Actionable comments posted: 3
🧹 Nitpick comments (3)
shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java (1)
153-157: Consider documenting or validating the slope ≤ 0 behavior.When
slope <= 0, the entire height band is set todestinationMaxHeight(a constant value). While this is documented in the javadoc, it might indicate a configuration error. Consider adding a comment explaining the use case or adding validation if this is unexpected behavior.If slope ≤ 0 is a valid use case, add an inline comment:
if (slope <= 0) { + // When slope is non-positive, flatten entire band to max height return destinationMaxHeight; } else {Alternatively, if slope ≤ 0 indicates a configuration error, consider logging a warning or throwing an exception.
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java (1)
14-14: Use consistent float literal notation.The waterHeight parameter uses
2instead of2for2.0f. While Java auto-casts this correctly, use2ffor consistency with other terrain generators.Apply this diff:
- fractalParams = new FractalParams(2, false, 2, 1.2f, 3, 3, new FractalFlattenParams[]{ + fractalParams = new FractalParams(2f, false, 2, 1.2f, 3, 3, new FractalFlattenParams[]{generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java (1)
108-117: Consider renaming for clarity.The method name
blurRamps()suggests it modifies therampsmask, but it actually applies progressive blurring to theheightmapusing the ramps as a mask region. The logic is correct, but the name might be clearer asblurHeightmapAtRamps()orapplyRampBlurring().This is a minor naming consideration to improve code readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
generator/src/main/java/com/faforever/neroxis/generator/FractalFlattenParams.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/FractalParams.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/MapStyle.java(2 hunks)generator/src/main/java/com/faforever/neroxis/generator/ResourceStyle.java(2 hunks)generator/src/main/java/com/faforever/neroxis/generator/TerrainStyle.java(2 hunks)generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java(3 hunks)generator/src/main/java/com/faforever/neroxis/generator/style/FractalLandStyleGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/style/FractalNavyStyleGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/style/FractalPlateauStyleGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/style/FractalUpsideDownStyleGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/style/RiversAndOceansStyleGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FloodedMultiLevelLastTerrainGenerator.java(0 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java(7 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java(7 hunks)shared/src/main/java/com/faforever/neroxis/map/placement/MexPlacer.java(3 hunks)shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java(1 hunks)
💤 Files with no reviewable changes (1)
- generator/src/main/java/com/faforever/neroxis/generator/terrain/FloodedMultiLevelLastTerrainGenerator.java
🧰 Additional context used
🧬 Code graph analysis (12)
generator/src/main/java/com/faforever/neroxis/generator/style/FractalNavyStyleGenerator.java (9)
generator/src/main/java/com/faforever/neroxis/generator/prop/BasicPropGenerator.java (1)
BasicPropGenerator(12-81)generator/src/main/java/com/faforever/neroxis/generator/prop/BoulderFieldPropGenerator.java (1)
BoulderFieldPropGenerator(12-74)generator/src/main/java/com/faforever/neroxis/generator/prop/HighReclaimPropGenerator.java (1)
HighReclaimPropGenerator(6-37)generator/src/main/java/com/faforever/neroxis/generator/prop/LargeBattlePropGenerator.java (1)
LargeBattlePropGenerator(15-66)generator/src/main/java/com/faforever/neroxis/generator/prop/NavyWrecksPropGenerator.java (1)
NavyWrecksPropGenerator(15-72)generator/src/main/java/com/faforever/neroxis/generator/prop/RockFieldPropGenerator.java (1)
RockFieldPropGenerator(12-54)generator/src/main/java/com/faforever/neroxis/generator/prop/SmallBattlePropGenerator.java (1)
SmallBattlePropGenerator(15-62)generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java (1)
HighMexLandLowMexWaterResourceGenerator(11-53)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java (1)
FractalNavyLastTerrainGenerator(10-35)
generator/src/main/java/com/faforever/neroxis/generator/style/FractalPlateauStyleGenerator.java (9)
generator/src/main/java/com/faforever/neroxis/generator/prop/BasicPropGenerator.java (1)
BasicPropGenerator(12-81)generator/src/main/java/com/faforever/neroxis/generator/prop/BoulderFieldPropGenerator.java (1)
BoulderFieldPropGenerator(12-74)generator/src/main/java/com/faforever/neroxis/generator/prop/EnemyCivPropGenerator.java (1)
EnemyCivPropGenerator(17-80)generator/src/main/java/com/faforever/neroxis/generator/prop/HighReclaimPropGenerator.java (1)
HighReclaimPropGenerator(6-37)generator/src/main/java/com/faforever/neroxis/generator/prop/LargeBattlePropGenerator.java (1)
LargeBattlePropGenerator(15-66)generator/src/main/java/com/faforever/neroxis/generator/prop/NeutralCivPropGenerator.java (1)
NeutralCivPropGenerator(17-81)generator/src/main/java/com/faforever/neroxis/generator/prop/RockFieldPropGenerator.java (1)
RockFieldPropGenerator(12-54)generator/src/main/java/com/faforever/neroxis/generator/prop/SmallBattlePropGenerator.java (1)
SmallBattlePropGenerator(15-62)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java (1)
FractalPlateauLastTerrainGenerator(10-23)
generator/src/main/java/com/faforever/neroxis/generator/style/RiversAndOceansStyleGenerator.java (1)
generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java (1)
HighMexLandLowMexWaterResourceGenerator(11-53)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java (1)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
Pipeline(24-212)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java (1)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
Pipeline(24-212)
generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java (1)
shared/src/main/java/com/faforever/neroxis/util/DebugUtil.java (1)
DebugUtil(6-124)
generator/src/main/java/com/faforever/neroxis/generator/style/FractalLandStyleGenerator.java (1)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java (1)
FractalLandLastTerrainGenerator(10-24)
generator/src/main/java/com/faforever/neroxis/generator/style/FractalUpsideDownStyleGenerator.java (1)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java (1)
FractalUpsideDownLastTerrainGenerator(10-22)
generator/src/main/java/com/faforever/neroxis/generator/TerrainStyle.java (4)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java (1)
FractalLandLastTerrainGenerator(10-24)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java (1)
FractalNavyLastTerrainGenerator(10-35)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java (1)
FractalPlateauLastTerrainGenerator(10-23)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java (1)
FractalUpsideDownLastTerrainGenerator(10-22)
generator/src/main/java/com/faforever/neroxis/generator/ResourceStyle.java (2)
generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java (1)
HighMexLandLowMexWaterResourceGenerator(11-53)generator/src/main/java/com/faforever/neroxis/generator/resource/WaterMexResourceGenerator.java (1)
WaterMexResourceGenerator(3-12)
generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java (1)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
Pipeline(24-212)
generator/src/main/java/com/faforever/neroxis/generator/MapStyle.java (4)
generator/src/main/java/com/faforever/neroxis/generator/style/FractalLandStyleGenerator.java (1)
FractalLandStyleGenerator(17-35)generator/src/main/java/com/faforever/neroxis/generator/style/FractalNavyStyleGenerator.java (1)
FractalNavyStyleGenerator(18-40)generator/src/main/java/com/faforever/neroxis/generator/style/FractalPlateauStyleGenerator.java (1)
FractalPlateauStyleGenerator(17-35)generator/src/main/java/com/faforever/neroxis/generator/style/FractalUpsideDownStyleGenerator.java (1)
FractalUpsideDownStyleGenerator(17-35)
🔇 Additional comments (18)
generator/src/main/java/com/faforever/neroxis/generator/style/FractalUpsideDownStyleGenerator.java (2)
14-21: LGTM! Class rename and terrain generator update align with PR objectives.The transition from
FloodedMultiLevelStyleGeneratortoFractalUpsideDownStyleGeneratoris consistent with the PR's goal of introducing fractal map varieties. The newFractalUpsideDownLastTerrainGeneratoris properly configured with fractal parameters (as seen in the relevant code snippets).
24-34: The removal of custom resource generation is correct and follows the established pattern in the codebase. Most Fractal style generators (Land, Plateau, and UpsideDown) use the parent class defaultBasicResourceGenerator, while onlyFractalNavyStyleGeneratoroverrides with a specializedHighMexLandLowMexWaterResourceGenerator. This is appropriate design—the default resource generator is well-defined in the parent class and suitable for these variants.Likely an incorrect or invalid review comment.
generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java (1)
130-138: Ensure symmetry divisor is non-zeroIn the HOUR_GLASS branch we divide (and derive radii) from
symmetrySettings.teamSymmetry().getNumSymPoints()several times. If we ever hit this path with team symmetry set toNONE/no symmetry, that accessor returns 0 today, which means the blur radius (Line 138) will throw anArithmeticExceptionand the earlier radius calculations will go toInfinity. Please add a guard (e.g.,int symPoints = Math.max(1, ...)) or otherwise confirm that this block is unreachable when the divisor would be zero.generator/src/main/java/com/faforever/neroxis/generator/FractalFlattenParams.java (1)
3-13: LGTM! Clean immutable parameter record.The record provides a clear, type-safe way to pass fractal flattening configuration.
generator/src/main/java/com/faforever/neroxis/generator/style/FractalLandStyleGenerator.java (2)
14-14: LGTM! Consistent renaming to FractalLand variant.The class, import, and terrain generator have been consistently renamed to reflect the FractalLand style variant.
Also applies to: 17-17, 20-20
27-27: Verify the 5x weight increase for BoulderFieldPropGenerator.The weight increased from 0.1f to 0.5f, which is a significant change. Ensure this aligns with gameplay balance expectations for the Fractal Land style.
generator/src/main/java/com/faforever/neroxis/generator/style/RiversAndOceansStyleGenerator.java (1)
16-16: LGTM! Resource generator swap is consistent.The import and instantiation have been updated consistently to use HighMexLandLowMexWaterResourceGenerator.
Also applies to: 24-24
generator/src/main/java/com/faforever/neroxis/generator/FractalParams.java (1)
3-11: LGTM! Well-structured configuration record.The record effectively encapsulates fractal generation parameters with nested FractalFlattenParams array for detailed control.
generator/src/main/java/com/faforever/neroxis/generator/ResourceStyle.java (1)
4-4: LGTM! Standard enum extension.The new HI_MEX_LAND_LOW_MEX_WATER resource style has been added correctly with appropriate import and supplier.
Also applies to: 17-18
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java (1)
10-22: LGTM! Clean fractal plateau configuration.The terrain generator correctly overrides initialize to set fractal-specific parameters before delegating to the superclass.
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java (1)
10-23: LGTM! Clean fractal land configuration.The terrain generator correctly overrides initialize to set fractal-specific parameters with five height bands before delegating to the superclass.
generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java (1)
25-28: Double-check reduced spawn exclusion radii for high-mex navy maps.Passing
spawnMexRadiusandremainingMexRadiusas 12 means we now carve out substantially less space around each spawn before dropping the remaining mexes; previously the default path kept 24/48, which prevented extra mex from landing inside the opening build area. On dense navy layouts that still have land spawns, this could let the “remaining” mex pool creep right up to—or even inside—the initial base footprint. Please confirm (e.g., by generating a few 10 km navy maps) that we still avoid spawning extra mex within the starting base ring.generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java (6)
89-106: Approve ramp generation logic.The refactored ramp generation logic correctly iterates over
fractalFlattenParamsto create height-band-based ramps. The approach of outlining layers and subtracting noise-based masks is sound.Note: This assumes the
fractalParamsinitialization issue is resolved.
132-137: Approve parameterized height flattening.The refactored height band flattening correctly uses configurable
fractalFlattenParamsinstead of hardcoded values, enabling the fractal variety system. This is a good improvement for flexibility.Note: This assumes the
fractalParamsinitialization issue is resolved.
167-174: Approve circular clipping for odd symmetries.The circular terrain clipping logic correctly handles odd-symmetry maps (3, 5, 6, 7, etc. symmetry points). The implementation properly:
- Creates a circular mask at the map center
- Inverts it to select the outer region
- Sets the outer region to water height
- Blurs the edge for smooth transition
This is essential for "pie-shaped" maps with odd symmetries.
179-190: Approve spawn mask pipeline.The new
setupSpawnMaskPipeline()method correctly builds the spawn mask by:
- Iterating over spawnable fractal flatten params
- Adding corresponding height bands with appropriate deflation
- Subtracting unbuildable areas
- Applying final deflation
The logic is clear and correctly implements the fractal-driven spawning system.
Note: This assumes the
fractalParamsinitialization issue is resolved.
192-201: Approve team separation logic.The
getTeamSeparation()method implements sensible team spacing:
- Returns 0 for single-team scenarios (no separation needed)
- Uses configurable
fractalParams.teamSeparation()for 2-team games- Scales dynamically for 3+ teams with a reasonable 256-unit cap
The logic is sound and appropriately handles different team configurations.
Note: This assumes the
fractalParamsinitialization issue is resolved.
23-23: No defect: Template Method pattern is working correctly.The
fractalParamsfield intentionally follows a Template Method pattern where the base class declares it for use in inherited methods, and all subclasses are required to initialize it in their overriddeninitialize()methods before the base class methods execute. Verification confirms all existing subclasses (FractalLandLastTerrainGenerator,FractalNavyLastTerrainGenerator,FractalUpsideDownLastTerrainGenerator,FractalPlateauLastTerrainGenerator) properly set this field first, so there is no NullPointerException risk.Consider adding
abstractmodifier toFractalNoiseLastTerrainGeneratorif it's not intended to be directly instantiated, or add a defensive null-check in line 34 to guard against future subclass misuse.
...main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java(1 hunks)shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java (2)
137-152: JavaDoc improvements look good.The documentation now comprehensively describes all parameters and the return value, addressing the previous review feedback. The slope behavior is well-explained with clear examples.
Minor formatting note: Line 152 has extra trailing spaces before the closing
*/.
185-196: LGTM! The remapping logic is sound.The normalization, clamping, curve application via
StrictMath.pow, and final mapping to the destination range are all correctly implemented. The method is only called whenslope > 0(guarded at line 164), so negative slope issues are avoided.
| @AllArgsConstructor | ||
| public enum MapStyle { | ||
| BASIC(BasicStyleGenerator::new, 1), | ||
| BASIC(BasicStyleGenerator::new, 2f), |
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.
So I feel this needs a bit of a re-balance.
I'm not sure what that balance should be...
But I guess this is what I think.
I felt the BasicStyleGenerator should be bumped to 2... This is arguably the best generator for ladder, in my opinion..
And bump the FLOODED one... It seems good to me, it's better than the fractal navy...
And I put the frcatal one's as 0.25 for ladder..
Maybe the FRACTAL_NAVY should be 0.1 for ladder? it's not super consist...
But it does make some good navy maps (some great navy maps maybe), but some maybe fringe.
(I guess every terrain generator can have some funky seeds)
| @Override | ||
| public void placeResources() { | ||
| DebugUtil.timedRun("com.faforever.neroxis.map.generator", "generateResources", () -> { | ||
| mexPlacer.placeMexes(getMexCount(), resourceMask.getFinalMask(), waterResourceMask.getFinalMask(), 12, 12, 12); |
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.
So yeah, I have had a dificult time trying to get Navy to work well.
The problems stem from a low amount of land to place mexes...
I've found I needed to tweak the mex spacing, so the more mexes are allowed closer together... And it allows more mex desity on the land areas availble, fixing many problems.
...or/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java (1)
137-152: Minor formatting: extra spaces before closing Javadoc tag.Line 152 has extra spaces before the closing
*/. While not functionally significant, standard Javadoc formatting uses a single space.Apply this diff for consistency:
- * @return the modified FloatMask */ + * @return the modified FloatMask + */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java (2)
153-155: LGTM!The expanded method signature correctly adds the new parameters for slope-based remapping functionality.
156-175: Method body logic is correct, pending fix to helper method.The edge-case handling at line 162 correctly checks
minHeight >= maxHeight, and the blur logic at line 172 properly uses theblurAmountparameter. However, the functionality depends on fixing the critical bug in theremapWithSlopehelper method (see separate comment on lines 177-196).
| int mapSize = map.getSize(); | ||
|
|
||
| int MAX_OCTAVES = 8; | ||
| int MAX_OCTAVES = 7; |
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.
So this parameter is a big change actually.
It affects all maps 10K or larger.
I was finding the the 8th octave here kinda pushes the maps to be like 1 big mountain area.
But limiting it to 7 mean's you might have 2 or 3 mountains, or just more detailed terrain (hard to describe)
But limit to 7 octaves actually works best I feel.
8 might be OK for 20K maps, but 7 is defintely better for 10K maps.
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.
| int spawnCount = generatorParameters.spawnCount(); | ||
|
|
||
| // 4 mexes per player, and about 24 mexes per 256 chunk of the map multiplied by resource density | ||
| return (spawnCount * 4) + StrictMath.round( (mapSize / 256f) * 24f * resourceDensity); |
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 struggled with the default implementation of this function.
So for "fractal navy" and "rivers and oceans" I made this much simpler calculation that works well for these navy maps.
And it's good for land aswell (Although one would need choose custom settings to try it)
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.
This could get pretty sparse with two players right with it only being 8 mexes total maybe we want to add a constant for when resource density is low?
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've updated the code a bit for this. To make it more readable.
The mex count is mainly based on the map size. So 1v1 and 4v4 and 8v8 will have very similar mex count on same map...
However the mex count felt a bit low with lots of players, so I added 4 mexes per player so bump the count a bit.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java(7 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java(1 hunks)generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java
🧰 Additional context used
🧬 Code graph analysis (2)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java (1)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
Pipeline(24-212)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java (1)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
Pipeline(24-212)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java (1)
14-22: Land fractal profile wired cleanly.Assigning
fractalParamsahead ofsuper.initialize(...)keeps the superclass logic deterministic for the land tiering you’re targeting, and the band definitions align with the legacy terrain steps.generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java (1)
14-21: Plateau parameterization looks on point.The flatter mid-band and late ramp flags fit the plateau intent while keeping the superclass setup untouched—nice separation of concerns.
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java (1)
14-34: Good small-vs-large navy tuning.Branching on map size to widen spawnable land on sub-512 maps should address the low-mex complaints without disturbing larger navy layouts.
.../src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java (1)
92-105: Consider reusing rampNoise across iterations.The
rampNoisemask is recreated (resized and regenerated) in each loop iteration. While this ensures independent noise for each ramp layer, it could be optimized by creating the noise once outside the loop if deterministic noise per layer isn't required.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java (8)
3-4: LGTM! Clean addition of fractal configuration.The new imports and field declarations appropriately support the fractal-based terrain generation configuration.
Also applies to: 13-13, 21-21, 23-23
34-40: LGTM! Random water mask selection is now correct.The switch statement correctly uses
random.nextInt(3)which produces 0, 1, or 2, making all three water mask options reachable. This resolves the issue flagged in the previous review.
43-43: LGTM! Good parameterization of terrain generation.Replacing hardcoded values with
fractalParamsconfiguration improves flexibility and maintainability.Also applies to: 45-45, 52-52, 63-63
108-117: LGTM! Effective progressive blurring for smooth ramp transitions.The multi-pass blur with increasing inflation creates smooth height transitions around ramps.
132-137: LGTM! Parameterized height band flattening.The loop-based approach allows flexible terrain flattening based on configuration, replacing hardcoded height bands.
167-174: LGTM! Appropriate circular clipping for odd symmetry.The circular boundary handling correctly clips pie-shaped maps to prevent terrain extending beyond the playable area, with smooth transitions via blur.
179-191: LGTM! Well-structured spawn mask pipeline.The spawn mask construction correctly identifies spawnable regions from configured height bands and applies appropriate constraints (unbuildable subtraction, center clearing, deflation).
193-202: No critical risk identified in current code.All five
FractalParamsinstantiations across the codebase assignteamSeparationvalues of 2 or 3. While the record field has no validation constraints and technically allows 0, all current code paths are safe. The division at line 198 will not throw anArithmeticExceptionin production.Consider this: if defensive validation is desired for future maintenance, add a
@Positiveannotation or validation to theFractalParamsrecord, or add a guard at line 198. However, the current code has no breaking issue.
| int spawnCount = generatorParameters.spawnCount(); | ||
|
|
||
| // 4 mexes per player, and about 24 mexes per 256 chunk of the map multiplied by resource density | ||
| return (spawnCount * 4) + StrictMath.round( (mapSize / 256f) * 24f * resourceDensity); |
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.
This could get pretty sparse with two players right with it only being 8 mexes total maybe we want to add a constant for when resource density is low?
generator/src/main/java/com/faforever/neroxis/generator/FractalParams.java
Outdated
Show resolved
Hide resolved
| heightmap.add(heightMapNoise); | ||
| } | ||
|
|
||
| if (symmetrySettings.spawnSymmetry().getNumSymPoints() == 3 || symmetrySettings.spawnSymmetry().getNumSymPoints() >= 5) { |
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.
you can just use the isPerfectSymmetry function here I believe
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.
Yep, it can, `spawnSymmtrey seems to do the job. Tested and working.
I remember writing that and I was unsure if it needed Terrain / Spawn / Team symmetry , etc....
So yep I put this in, and forgot to come back and check it etc.
I also removed the addWhiteNoise() stuff just above....
It was inherited from the BasicTerrainGenerator...
But i doesn't seem add much for these Fractal ones.
It was OK for the MutliLevel stuff, but maybe not so much now.




Adds 4 new varietyies of the Fractal Style of map generation.
And removes the FloodedMutliLayer terrain generator. (Re-incarnated as Fractal Navy)
So some previews of these new map generators:

Fractal Land: (Same as original) 15K, 8 player:
Fractal Plateau: 15K, 8 player

Fractal Navy: 15K, 8 player

Fractal Upside Down:

11/11/2025:



And, as is Tradition with these kinds of PR's....
We have a map of the day...
This one pushing the boundries with 9 player phantom (Fractal Navy) map:
And another one, a 20K Fractal navy........
And 5 random 15K 3v3 navy maps: (I seem to be focusing on navy a lot)
12/11/2025:

Was testing a bit more.... I was finding that on phantom land maps the spawn points can be too close to the centre.
So I have subtracted a circle in the centre on the map.... much better.
A new map of the day...
This is a 15k 3v3 on Fractal Land.... Same as what we have today, except the MAX_OCTIVE in the noise map is 7 instead of 8:
13/11/2015:
More generating today, everything is stable... looking for a map of the day.
And decided to write some general thoughts:
I guess this fractal stuff is the result of much experimentation, with lots of strange maps along the way.
It certainly fit the philosophy of map diversity, and trying to bring something new and different to the table.
Is it Good or is it Bad? Well probably both, as with all mapgens.... But hopefully mostly OK, with something good.
And my thoughts on each new terrain type:
Fractal Land: Well it's the basis, playable, nice looking preview. (I thought this would turn out to be a T1 spam map style, but maybe it's more turtle, I'm not sure)
Fractal Plateau: Well I think this is new and different.
Fractal Navy: Kinda hard to make navy work... I almost give up on navy... but persisted, and finally made it more reliable, and I think it generates intersting maps now.
Fractal Upside Down: The most experimentation... the most strange... complexity reduced to simplicity. but possibly fun to play.
What I think is the best: probably Land or the Plateau, but trying the navy will be interesting.

And I might do a map of the day......
Featuring a 15K 3v3 of Fractal Upside Down.
Summary by CodeRabbit
New Features
Improvements
Removed