Skip to content

Conversation

@clivepaterson
Copy link
Contributor

@clivepaterson clivepaterson commented Nov 4, 2025

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:
neroxis_map_generator_snapshot_576ctubliunzq_bagaeey_preview

Fractal Plateau: 15K, 8 player
image

Fractal Navy: 15K, 8 player
image

Fractal Upside Down:
image

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:
image
And another one, a 20K Fractal navy........
image
And 5 random 15K 3v3 navy maps: (I seem to be focusing on navy a lot)
image

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:
image

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.
image

Summary by CodeRabbit

  • New Features

    • Four new fractal map style variants with matching terrain, prop and resource options.
    • New navy/plateau/upside-down themed layouts and a high-land/low-water resource distribution.
  • Improvements

    • Multi-band height flattening with slope-based remapping and configurable blur.
    • Better spawn and mex placement with density-aware scaling and adjustable radii.
    • Water-area aware terrain shaping, expanded ramp/blurring, circular spawn clipping, and refined team separation.
  • Removed

    • Legacy flooded multi-level terrain variant.

@coderabbitai
Copy link

coderabbitai bot commented Nov 4, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
New Parameter Records
\generator/src/main/java/com/faforever/neroxis/generator/FractalFlattenParams.java`, `generator/src/main/java/com/faforever/neroxis/generator/FractalParams.java``
Added immutable Java records carrying fractal flatten and overall fractal-generation configuration fields.
Map / Style / Resource Enums
\generator/src/main/java/com/faforever/neroxis/generator/MapStyle.java`, `generator/src/main/java/com/faforever/neroxis/generator/TerrainStyle.java`, `generator/src/main/java/com/faforever/neroxis/generator/ResourceStyle.java``
Removed flooded-multilevel entries, adjusted weights for existing styles, and added four FRACTAL_* style variants plus HI_MEX_LAND_LOW_MEX_WATER resource constant.
Style Generators
\generator/src/main/java/com/faforever/neroxis/generator/style/Fractal.java`, `generator/src/main/java/com/faforever/neroxis/generator/style/RiversAndOceansStyleGenerator.java``
Added/renamed fractal style generators (land/plateau/navy/upside-down), rebalanced prop weights, and switched RiversAndOceans to use HighMexLandLowMexWaterResourceGenerator.
Terrain Generators (new)
\generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java`, `.../FractalNavyLastTerrainGenerator.java`, `.../FractalPlateauLastTerrainGenerator.java`, `.../FractalUpsideDownLastTerrainGenerator.java``
Added terrain generators that set specific FractalParams (arrays of FractalFlattenParams) in initialize(...) then delegate to FractalNoiseLastTerrainGenerator.
Fractal Noise Core Refactor
\generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java``
Added fractalParams field; parameterized ramp and flattening loops driven by FractalFlattenParams[]; introduced blurRamps(), setupSpawnMaskPipeline(), random water-mask option, circular clipping for odd symmetry, and team separation logic; updated initialize(...) signature.
Removed Flooded Multi-level
\generator/src/main/java/com/faforever/neroxis/generator/terrain/FloodedMultiLevelLastTerrainGenerator.java``
Deleted the FloodedMultiLevelLastTerrainGenerator class and its specialized terrain/spawn/team-spacing logic.
Multi-level Water Area Support
\generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java`​`
Added WaterMasks enum plus waterArea/waterAreaBlur and addWaterAreasToNoiseMap(...); integrated water-area-aware noise adjustments and reduced MAX_OCTAVES.
Resource Generator rename & logic
\generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java`​`
Renamed from RiversAndOceansMexResourceGenerator; added resourceDensity randomization, timed placeResources() flow, higher water-mask threshold, and revised getMexCount() calculation.
Mex Placement API
\shared/src/main/java/com/faforever/neroxis/map/placement/MexPlacer.java`​`
Added overloaded placeMexes(...) with explicit mexSpacing, spawnMexRadius, and remainingMexRadius; original delegates to the new overload; adjusted spacing/radii calculations.
Mask Utilities Update
\shared/src/main/java/com/faforever/neroxis/mask/MapMaskMethods.java`​`
Expanded flattenHeightBand signature to accept destinationMinHeight, destinationMaxHeight, slope, and blurAmount; added remapWithSlope(...) and slope-based remapping plus variable blur semantics.

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~70 minutes

  • Focus review on:
    • FractalNoiseLastTerrainGenerator: new multi-layer control flow, ramp/blur/spawn pipeline, symmetry clipping, and team separation.
    • MultiLevelLastTerrainGenerator: water-area mask logic and integration with noise.
    • HighMexLandLowMexWaterResourceGenerator: resourceDensity effects, placeResources(), and mex-count math.
    • MapMaskMethods.flattenHeightBand/remapWithSlope: numeric mapping, slope behavior, and blur semantics.
    • Enum and generator bindings (MapStyle/TerrainStyle/ResourceStyle): ensure new variants and weights map to correct generator classes.

Poem

🐰 I twitch my whiskers at bands of height,

Ramps rolled, masks blurred in fractal light,
Mexes tucked where spawn-rings gleam,
New lands fold into a fractal dream,
I hop — the map is stitched just right.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 2.70% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Fractal Variety' is vague and generic, using a non-descriptive term that doesn't convey specific information about the changeset beyond referencing 'Fractal' generally. Consider a more specific title that describes the main change, such as 'Add four fractal map generation variants' or 'Introduce Fractal Land, Plateau, Navy, and Upside Down styles'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@clivepaterson clivepaterson marked this pull request as ready for review November 10, 2025 13:16
Copy link

@coderabbitai coderabbitai bot left a 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 to destinationMaxHeight (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 2 instead of 2f or 2.0f. While Java auto-casts this correctly, use 2f for 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 the ramps mask, but it actually applies progressive blurring to the heightmap using the ramps as a mask region. The logic is correct, but the name might be clearer as blurHeightmapAtRamps() or applyRampBlurring().

This is a minor naming consideration to improve code readability.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c223394 and cfde0b3.

📒 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 FloodedMultiLevelStyleGenerator to FractalUpsideDownStyleGenerator is consistent with the PR's goal of introducing fractal map varieties. The new FractalUpsideDownLastTerrainGenerator is 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 default BasicResourceGenerator, while only FractalNavyStyleGenerator overrides with a specialized HighMexLandLowMexWaterResourceGenerator. 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-zero

In 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 to NONE/no symmetry, that accessor returns 0 today, which means the blur radius (Line 138) will throw an ArithmeticException and the earlier radius calculations will go to Infinity. 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 spawnMexRadius and remainingMexRadius as 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 fractalFlattenParams to create height-band-based ramps. The approach of outlining layers and subtracting noise-based masks is sound.

Note: This assumes the fractalParams initialization issue is resolved.


132-137: Approve parameterized height flattening.

The refactored height band flattening correctly uses configurable fractalFlattenParams instead of hardcoded values, enabling the fractal variety system. This is a good improvement for flexibility.

Note: This assumes the fractalParams initialization 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:

  1. Iterating over spawnable fractal flatten params
  2. Adding corresponding height bands with appropriate deflation
  3. Subtracting unbuildable areas
  4. Applying final deflation

The logic is clear and correctly implements the fractal-driven spawning system.

Note: This assumes the fractalParams initialization 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 fractalParams initialization issue is resolved.


23-23: No defect: Template Method pattern is working correctly.

The fractalParams field 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 overridden initialize() 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 abstract modifier to FractalNoiseLastTerrainGenerator if it's not intended to be directly instantiated, or add a defensive null-check in line 34 to guard against future subclass misuse.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between cfde0b3 and 4cfffaa.

📒 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 when slope > 0 (guarded at line 164), so negative slope issues are avoided.

@AllArgsConstructor
public enum MapStyle {
BASIC(BasicStyleGenerator::new, 1),
BASIC(BasicStyleGenerator::new, 2f),
Copy link
Contributor Author

@clivepaterson clivepaterson Nov 10, 2025

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);
Copy link
Contributor Author

@clivepaterson clivepaterson Nov 10, 2025

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.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4cfffaa and 2c99d68.

📒 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 the blurAmount parameter. However, the functionality depends on fixing the critical bug in the remapWithSlope helper method (see separate comment on lines 177-196).

int mapSize = map.getSize();

int MAX_OCTAVES = 8;
int MAX_OCTAVES = 7;
Copy link
Contributor Author

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.

Copy link
Contributor Author

@clivepaterson clivepaterson Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So maybe a preview image for this change shows the result better:
20K Land maps with max octive of 8:
neroxis_map_generator_snapshot_3rkv4g22tla42_ayiaeea_preview
image

20K Land maps with max octive of 7:
image
image

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);
Copy link
Contributor Author

@clivepaterson clivepaterson Nov 11, 2025

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)

Copy link
Member

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?

Copy link
Contributor Author

@clivepaterson clivepaterson Nov 18, 2025

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.

Copy link

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2a48630 and 321efc1.

📒 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 fractalParams ahead of super.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.

Copy link

@coderabbitai coderabbitai bot left a 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 rampNoise mask 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

📥 Commits

Reviewing files that changed from the base of the PR and between 321efc1 and ed10ec2.

📒 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 fractalParams configuration 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 FractalParams instantiations across the codebase assign teamSeparation values 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 an ArithmeticException in production.

Consider this: if defensive validation is desired for future maintenance, add a @Positive annotation or validation to the FractalParams record, 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);
Copy link
Member

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?

heightmap.add(heightMapNoise);
}

if (symmetrySettings.spawnSymmetry().getNumSymPoints() == 3 || symmetrySettings.spawnSymmetry().getNumSymPoints() >= 5) {
Copy link
Member

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

Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants