Skip to content

Conversation

@Sheikah45
Copy link
Member

@Sheikah45 Sheikah45 commented Dec 30, 2025

Summary by CodeRabbit

  • New Features

    • Added Ahead-Of-Time (AOT) generation to improve startup and runtime performance.
  • Chores

    • Bumped Java runtime to JDK 25 and Gradle wrapper to 9.2.1.
    • Upgraded key build tools and libraries (Lombok, JUnit, plugin updates).
    • Enhanced packaging pipeline with intermediate module linking and improved installer scripting.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 30, 2025

Warning

Rate limit exceeded

@Sheikah45 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 13 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf47f3 and 55b7e55.

📒 Files selected for processing (4)
  • .github/workflows/release.yml
  • .github/workflows/tests.yml
  • buildSrc/build.gradle.kts
  • shared/src/main/java/com/faforever/neroxis/mask/OperationsMask.java
📝 Walkthrough

Walkthrough

This PR removes the Pipeline instance parameter across masks, generators, and related APIs; refactors Pipeline into a thread-scoped/static facility with run/add/isAccepting; updates mask constructors and inflate/deflate signatures to use ints; upgrades toolchain to Java 25/Gradle 9.2.1 and adds jlink/AOT packaging steps and script templates.

Changes

Cohort / File(s) Summary
CI / Toolchain
\.github/workflows/release.yml, \.github/workflows/tests.yml, gradle/wrapper/gradle-wrapper.properties
JDK bumped 21 → 25; Gradle wrapper updated to 9.2.1; release workflow split into jlink → AOT MapGenerator → jpackage steps and packaging script refactor.
BuildSrc & Packaging Templates
buildSrc/build.gradle.kts, buildSrc/src/main/kotlin/...conventions-*.gradle.kts, buildSrc/src/main/resources/templates/unixScriptTemplate.txt, buildSrc/src/main/resources/templates/windowsScriptTemplate.txt
jlink plugin bumped; JVM toolchain -> 25; jlink/jpackage options reworked (zip-9, AOT cache, launcher JVM args); added unix/windows launcher templates.
IDE / Styles
.idea/codeStyles/Project.xml
Project code-style settings adjusted (indent autodetect off, multiline parentheses alignment enabled, removed binary op option).
Pipeline Core
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java
Converted Pipeline from instance-based to ScopedValue-backed static API: added run(...), add(...), isAccepting(), entry lifecycle changes, dependency sequencing, and write hooks.
Mask Base / Core Changes
shared/src/main/java/com/faforever/neroxis/mask/Mask.java, ComparableMask.java, OperationsMask.java, PrimitiveMask.java
Removed Pipeline ctor parameter and per-instance pipeline field; replaced with static Pipeline helpers; removed getMostRecentEntry(); enqueue/getSize/assert logic adapted to Pipeline.isAccepting().
Mask Subclasses (constructors & API)
shared/src/main/java/com/faforever/neroxis/mask/*{BooleanMask,FloatMask,IntegerMask,NormalMask,Vector2Mask,Vector3Mask,Vector4Mask,VectorMask}.java
Dropped Pipeline from public constructors and delegation; BooleanMask.inflate/deflate changed from float → int; VectorMask splitComponentMasks uses generic Mask[] and casts; propagation of constructor signature removals across mask types.
Generators — Terrain, Texture, Prop, Resource, Decal
generator/src/main/java/com/faforever/neroxis/generator/... (e.g., terrain/*, prop/*, resource/*, texture/*, decal/DecalGenerator.java, style/StyleGenerator.java)
All initialize(...) signatures had Pipeline removed and super.initialize calls updated; mask instantiations updated to new constructors; many generators switched numeric literals from float to int; StyleGenerator adapted to collect terrainPipelineEntries/placementPipelineEntries and use Pipeline.run(); new getPipelineHashes() added.
Map Generator / Utilities / Tests
generator/src/main/java/.../GeneratedMapNameEncoder.java, generator/src/test/java/.../MapGeneratorTest.java, shared/src/main/java/.../placement/{HydroPlacer,SpawnPlacer}.java, toolsuite/.../MapInfoTextureExporter.java, utilities/.../TestingGround.java
Base32 encoder builder change; test hash checks simplified; spawn/hydro placement minor numeric/format tweaks; MapInfoTextureExporter shadow inflate changed; TestingGround simplified main.
Gradle & Shared libs
shared/build.gradle.kts, buildSrc/...
Avaje bumped 3.5 → 3.9; formatting/argument listing tweaks in build scripts.

Sequence Diagram(s)

mermaid
sequenceDiagram
autonumber
participant StyleGen as StyleGenerator
participant Pipeline as Pipeline (static)
participant Entry as Pipeline.Entry
participant Mask as Mask/Generators
participant Writer as Entry.write(OutputStream)
Note over StyleGen,Pipeline: High-level pipeline-run flow for style/placement
StyleGen->>Pipeline: Pipeline.run(setup)
activate Pipeline
Pipeline->>Mask: setup adds entries via Pipeline.add(...)
Mask-->>Pipeline: register dependencies (mask -> Entry)
Pipeline->>Entry: create Entry (with future) for each add
Note right of Entry: Entry holds computation future and write()
Entry-->>Pipeline: complete future when work done
Pipeline->>StyleGen: returns immutable List
StyleGen->>Entry: iterate entries -> Entry.write(stream)
deactivate Pipeline

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • Fractal Variety #472: Overlaps on Fractal* terrain generator changes and mask/flattening logic; likely touches similar initialize/mask signatures and related pipeline behavior.

Poem

🐰 Pipelines turned tiny, then made to run wide,
Masks swapped their floats for integer stride,
Java twenty‑five boots the new pack,
Scripts and AOT stitch the launcher back,
A rabbit cheers: small hops, giant stride!

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Update to java 25' accurately and concisely summarizes the main objective of the pull request, which is to upgrade the project from Java 21 to Java 25 across multiple build configurations and dependencies.

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.

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
shared/src/main/java/com/faforever/neroxis/mask/OperationsMask.java (4)

238-248: Pre-existing bug: multiply(BooleanMask, T) calls subtractValueAt instead of multiplyValueAt.

Line 244 incorrectly calls subtractValueAt when it should call multiplyValueAt. This appears to be a pre-existing copy-paste error, not introduced by this PR.

🔎 Proposed fix
     public U multiply(BooleanMask other, T value) {
         assertCompatibleMask(other);
         return enqueue(dependencies -> {
             BooleanMask source = (BooleanMask) dependencies.getFirst();
             apply((x, y) -> {
                 if (source.getPrimitive(x, y)) {
-                    subtractValueAt(x, y, value);
+                    multiplyValueAt(x, y, value);
                 }
             });
         }, other);
     }

258-270: Pre-existing bug: multiply(BooleanMask, U) calls subtractValueAt instead of multiplyValueAt.

Line 266 incorrectly calls subtractValueAt when it should call multiplyValueAt. Same copy-paste issue as the previous method.

🔎 Proposed fix
     public U multiply(BooleanMask other, U values) {
         assertCompatibleMask(other);
         assertCompatibleMask(values);
         return enqueue(dependencies -> {
             BooleanMask source = (BooleanMask) dependencies.get(0);
             U vals = (U) dependencies.get(1);
             apply((x, y) -> {
                 if (source.getPrimitive(x, y)) {
-                    subtractValueAt(x, y, vals.get(x, y));
+                    multiplyValueAt(x, y, vals.get(x, y));
                 }
             });
         }, other, values);
     }

322-332: Pre-existing bug: divide(BooleanMask, T) calls subtractValueAt instead of divideValueAt.

Line 328 incorrectly calls subtractValueAt when it should call divideValueAt.

🔎 Proposed fix
     public U divide(BooleanMask other, T value) {
         assertCompatibleMask(other);
         return enqueue(dependencies -> {
             BooleanMask source = (BooleanMask) dependencies.getFirst();
             apply((x, y) -> {
                 if (source.getPrimitive(x, y)) {
-                    subtractValueAt(x, y, value);
+                    divideValueAt(x, y, value);
                 }
             });
         }, other);
     }

342-354: Pre-existing bug: divide(BooleanMask, U) calls subtractValueAt instead of divideValueAt.

Line 350 incorrectly calls subtractValueAt when it should call divideValueAt.

🔎 Proposed fix
     public U divide(BooleanMask other, U values) {
         assertCompatibleMask(other);
         assertCompatibleMask(values);
         return enqueue(dependencies -> {
             BooleanMask source = (BooleanMask) dependencies.get(0);
             U vals = (U) dependencies.get(1);
             apply((x, y) -> {
                 if (source.getPrimitive(x, y)) {
-                    subtractValueAt(x, y, vals.get(x, y));
+                    divideValueAt(x, y, vals.get(x, y));
                 }
             });
         }, other, values);
     }
🧹 Nitpick comments (5)
.github/workflows/release.yml (1)

60-63: Consider breaking the AOT command into multiple lines for readability.

The AOT cache generation command on line 62 is extremely long (over 200 characters), making it difficult to read and maintain. Consider using YAML's multiline syntax for better readability.

🔎 Suggested refactoring
       - name: Package with Gradle
         run: |
           ./gradlew -x test --stacktrace jlink --parallel -PgeneratorVersion="${{ github.event.inputs.version }}"
-          ./generator/build/image/bin/java -XX:AOTCacheOutput=generator/build/image/bin/neroxis-generator.aot -XX:+UseCompactObjectHeaders -m com.faforever.neroxis.generator/com.faforever.neroxis.generator.MapGenerator --out-path ./GeneratedMaps                                                                                                                                                                                                                       
+          ./generator/build/image/bin/java \
+            -XX:AOTCacheOutput=generator/build/image/bin/neroxis-generator.aot \
+            -XX:+UseCompactObjectHeaders \
+            -m com.faforever.neroxis.generator/com.faforever.neroxis.generator.MapGenerator \
+            --out-path ./GeneratedMaps
           ./gradlew -x test --stacktrace jpackage --parallel -PgeneratorVersion="${{ github.event.inputs.version }}"
buildSrc/src/main/kotlin/com.faforever.neroxis.conventions-application.gradle.kts (1)

28-31: Consider simplifying path resolution.

The nested resolve() calls work correctly, but could be more concise:

🔎 Proposed simplification
-        val templatesDir =
-            rootDir.resolve("buildSrc").resolve("src").resolve("main").resolve("resources").resolve("templates")
+        val templatesDir = rootDir.resolve("buildSrc/src/main/resources/templates")
         unixScriptTemplate = templatesDir.resolve("unixScriptTemplate.txt")
         windowsScriptTemplate = templatesDir.resolve("windowsScriptTemplate.txt")
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)

209-217: Entry.write() method may throw unchecked exception for algorithm availability.

The NoSuchAlgorithmException is wrapped in RuntimeException, which is acceptable since MD5 should always be available. However, consider logging the error or using a more specific exception type for clarity.

shared/src/main/java/com/faforever/neroxis/mask/NormalMask.java (1)

14-20: Constructor ignores symmetrySettings parameter.

The constructor at lines 18-20 accepts a SymmetrySettings parameter but ignores it, always delegating to the constructor that uses Symmetry.NONE. This is misleading for callers who might expect their symmetry settings to be applied.

If NormalMask should always use Symmetry.NONE, consider either:

  1. Removing this constructor overload, or
  2. Adding a comment explaining why symmetry settings are intentionally ignored for normal masks
🔎 Option 1: Remove misleading constructor
 public NormalMask(int size, Long seed, String name) {
     super(size, seed, new SymmetrySettings(Symmetry.NONE), name);
 }

-public NormalMask(int size, Long seed, SymmetrySettings symmetrySettings, String name) {
-    this(size, seed, name);
-}
🔎 Option 2: Document the behavior
+/**
+ * Creates a NormalMask. Note: symmetrySettings is ignored as normal masks
+ * always use {@link Symmetry#NONE}.
+ */
 public NormalMask(int size, Long seed, SymmetrySettings symmetrySettings, String name) {
     this(size, seed, name);
 }
shared/src/main/java/com/faforever/neroxis/mask/VectorMask.java (1)

635-653: Type change reduces compile-time safety but is functionally correct.

The change from FloatMask[] to Mask<?, ?>[] at line 644 loses some type information, requiring the explicit cast at line 647. Since the components array passed to enqueue is guaranteed to be FloatMask[], the cast is safe at runtime. The class-level @SuppressWarnings("unchecked") already covers this pattern.

Consider retaining the stronger typing if possible:

🔎 Optional: Retain FloatMask[] typing
         enqueue(dependencies -> {
-            Mask<?, ?>[] sources = dependencies.subList(0, dimension).toArray(Mask[]::new);
+            FloatMask[] sources = dependencies.subList(0, dimension).toArray(FloatMask[]::new);
             apply((x, y) -> {
                 for (int i = 0; i < dimension; ++i) {
-                    ((FloatMask) sources[i]).setPrimitive(x, y, get(x, y).get(i));
+                    sources[i].setPrimitive(x, y, get(x, y).get(i));
                 }
             });
         }, components);
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 61aca49 and 7e725ca.

📒 Files selected for processing (78)
  • .github/workflows/release.yml
  • .github/workflows/tests.yml
  • .idea/codeStyles/Project.xml
  • buildSrc/build.gradle.kts
  • buildSrc/src/main/kotlin/com.faforever.neroxis.conventions-application.gradle.kts
  • buildSrc/src/main/kotlin/com.faforever.neroxis.conventions-java.gradle.kts
  • buildSrc/src/main/resources/templates/unixScriptTemplate.txt
  • buildSrc/src/main/resources/templates/windowsScriptTemplate.txt
  • generator/src/main/java/com/faforever/neroxis/generator/GeneratedMapNameEncoder.java
  • generator/src/main/java/com/faforever/neroxis/generator/decal/DecalGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/BasicPropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/BoulderFieldPropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/EnemyCivPropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/LargeBattlePropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/NavyWrecksPropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/NeutralCivPropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/PropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/RockFieldPropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/prop/SmallBattlePropGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/resource/HighMexLandLowMexWaterResourceGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/resource/ResourceGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/style/StyleGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/BasicLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/BasicTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/BigIslandsTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/CenterLakeLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/CenterLakeTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/DropPlateauLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/DropPlateauTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FloodedTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalLandLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNoiseLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalPlateauLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalUpsideDownLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/LandBridgeTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/LittleMountainLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/LittleMountainTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/MountainRangeLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/MountainRangeTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/OneIslandTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedPlateauLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedPlateauTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/RiversAndOceansTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/RiversTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/SmallIslandsTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/SpawnFirstTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/SpawnLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/TerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/ValleyLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/ValleyTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/texture/LegacyTextureGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/texture/PbrTextureGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/texture/TextureGenerator.java
  • generator/src/test/java/com/faforever/neroxis/generator/MapGeneratorTest.java
  • gradle/wrapper/gradle-wrapper.properties
  • shared/build.gradle.kts
  • shared/src/main/java/com/faforever/neroxis/lua/LuaParserVisitorImpl.java
  • shared/src/main/java/com/faforever/neroxis/map/placement/HydroPlacer.java
  • shared/src/main/java/com/faforever/neroxis/map/placement/SpawnPlacer.java
  • shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/ComparableMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/FloatMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/IntegerMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/Mask.java
  • shared/src/main/java/com/faforever/neroxis/mask/NormalMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/OperationsMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/PrimitiveMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/Vector2Mask.java
  • shared/src/main/java/com/faforever/neroxis/mask/Vector3Mask.java
  • shared/src/main/java/com/faforever/neroxis/mask/Vector4Mask.java
  • shared/src/main/java/com/faforever/neroxis/mask/VectorMask.java
  • shared/src/main/java/com/faforever/neroxis/util/Pipeline.java
  • toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java
  • utilities/src/main/java/com/faforever/neroxis/utilities/TestingGround.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-27T19:00:16.091Z
Learnt from: clivepaterson
Repo: FAForever/Neroxis-Map-Generator PR: 0
File: :0-0
Timestamp: 2025-12-27T19:00:16.091Z
Learning: In the Neroxis Map Generator codebase, StrictMath should always be used instead of Math to ensure deterministic behavior across different JVM implementations for procedural map generation.

Applied to files:

  • generator/src/main/java/com/faforever/neroxis/generator/terrain/FractalNavyLastTerrainGenerator.java
📚 Learning: 2025-12-27T17:01:57.927Z
Learnt from: clivepaterson
Repo: FAForever/Neroxis-Map-Generator PR: 477
File: shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java:817-819
Timestamp: 2025-12-27T17:01:57.927Z
Learning: In the Neroxis Map Generator codebase: X symmetry means the top half of the map mirrors the bottom half (requires a horizontal line at halfY), and Z symmetry means the left half mirrors the right half (requires a vertical line at halfX).

Applied to files:

  • generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java
  • shared/src/main/java/com/faforever/neroxis/mask/Vector3Mask.java
  • shared/src/main/java/com/faforever/neroxis/mask/ComparableMask.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/SmallIslandsTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/DropPlateauTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/BigIslandsTerrainGenerator.java
  • shared/src/main/java/com/faforever/neroxis/mask/Vector4Mask.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/RiversAndOceansTerrainGenerator.java
  • shared/src/main/java/com/faforever/neroxis/mask/VectorMask.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/RiversTerrainGenerator.java
  • shared/src/main/java/com/faforever/neroxis/mask/Vector2Mask.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/DropPlateauLastTerrainGenerator.java
  • generator/src/main/java/com/faforever/neroxis/generator/terrain/OneIslandTerrainGenerator.java
  • shared/src/main/java/com/faforever/neroxis/mask/IntegerMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/FloatMask.java
  • shared/src/main/java/com/faforever/neroxis/mask/NormalMask.java
📚 Learning: 2025-12-27T17:01:49.688Z
Learnt from: clivepaterson
Repo: FAForever/Neroxis-Map-Generator PR: 477
File: shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java:817-819
Timestamp: 2025-12-27T17:01:49.688Z
Learning: In shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java, define/clarify symmetry handling: - X symmetry means the top half of the map mirrors the bottom half, achieved by a horizontal reflection across the line y = halfY. - Z symmetry means the left half mirrors the right half, achieved by a vertical reflection across the line x = halfX. Ensure the implementation computes halfX and halfY, applies the appropriate reflection to affected cells, and that tests cover both symmetry cases with boundary checks.

Applied to files:

  • shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java
📚 Learning: 2025-12-19T12:43:23.824Z
Learnt from: clivepaterson
Repo: FAForever/Neroxis-Map-Generator PR: 477
File: generator/src/main/java/com/faforever/neroxis/generator/terrain/MultiLevelLastTerrainGenerator.java:153-156
Timestamp: 2025-12-19T12:43:23.824Z
Learning: The fillRect method in the Mask class has the signature: fillRect(int x, int y, int width, int height, T value), where x and y are the starting coordinates, and width and height are the dimensions (not ending coordinates).

Applied to files:

  • shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java
🧬 Code graph analysis (3)
shared/src/main/java/com/faforever/neroxis/mask/Mask.java (1)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
  • Pipeline (27-223)
shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java (1)
shared/src/main/java/com/faforever/neroxis/util/SymmetryUtil.java (1)
  • SymmetryUtil (13-335)
generator/src/main/java/com/faforever/neroxis/generator/style/StyleGenerator.java (3)
shared/src/main/java/com/faforever/neroxis/map/placement/SpawnPlacer.java (1)
  • SpawnPlacer (17-134)
shared/src/main/java/com/faforever/neroxis/util/DebugUtil.java (1)
  • DebugUtil (6-124)
shared/src/main/java/com/faforever/neroxis/util/Pipeline.java (1)
  • Pipeline (27-223)
🪛 actionlint (1.7.9)
.github/workflows/tests.yml

20-20: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

.github/workflows/release.yml

50-50: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

⏰ 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). (3)
  • GitHub Check: Package windows-latest
  • GitHub Check: Package macOS-latest
  • GitHub Check: build

Comment on lines +18 to 20
val lombokVersion = "1.18.42"
compileOnly("org.projectlombok:lombok:$lombokVersion")
annotationProcessor("org.projectlombok:lombok:$lombokVersion")
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Lombok 1.18.42 Java 25 support compatibility

💡 Result:

Short answer: Lombok 1.18.42 adds JDK 25 compatibility (release dated Sep 20, 2025), but there are still active/closed issues reporting JDK 25-related breakages — test your code (and consider newer Lombok releases if problems appear). [1][2][3][4]

Sources:

  • 1.18.42 release entry. [1]
  • Feature/compatibility discussion for JDK 25. [2]
  • Reported "Unable to compile using java 25" issue against 1.18.42. [3]
  • Reported "val can be used only once with JDK25" bug (1.18.42). [4]

Test Lombok 1.18.42 thoroughly with JDK 25 before merging.

While Lombok 1.18.42 adds JDK 25 compatibility support, there are known issues reported including compilation failures and val usage restrictions with JDK 25. Test your codebase with this version, and consider upgrading to a newer Lombok release if you encounter compatibility problems.

🤖 Prompt for AI Agents
In buildSrc/src/main/kotlin/com.faforever.neroxis.conventions-java.gradle.kts
around lines 18 to 20, you've set lombokVersion to "1.18.42" which may have JDK
25 compatibility issues; run the entire project build and test suite using JDK
25 to verify compilation, annotation processing and 'val' usage; if you
encounter failures, either bump lombokVersion to a newer release that fixes JDK
25 issues or revert to the previous known-good Lombok version, update the
version string accordingly and re-run tests, and document the chosen version in
the file header or project README for future reference.

Comment on lines 32 to 36
this.slope = terrainGenerator.getSlope();
this.passableLand = new BooleanMask(1, random.nextLong(), symmetrySettings, "passableLand", pipeline);
this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "passableLand", pipeline);
this.passableLand = new BooleanMask(1, random.nextLong(), symmetrySettings, "passableLand");
this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "passableLand");
passableLand.init(terrainGenerator.getPassableLand());
slope.init(terrainGenerator.getSlope());
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Fix incorrect mask name and redundant assignment.

There are two issues here:

  1. Line 32 assigns this.slope = terrainGenerator.getSlope(), but line 34 immediately overwrites it with a new FloatMask
  2. Line 34 creates a FloatMask with the name "passableLand" when it should be "slope"

This appears to be a copy-paste error where the mask name wasn't updated.

🔎 Proposed fix
-        this.slope = terrainGenerator.getSlope();
         this.passableLand = new BooleanMask(1, random.nextLong(), symmetrySettings, "passableLand");
-        this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "passableLand");
+        this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "slope");
         passableLand.init(terrainGenerator.getPassableLand());
         slope.init(terrainGenerator.getSlope());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.slope = terrainGenerator.getSlope();
this.passableLand = new BooleanMask(1, random.nextLong(), symmetrySettings, "passableLand", pipeline);
this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "passableLand", pipeline);
this.passableLand = new BooleanMask(1, random.nextLong(), symmetrySettings, "passableLand");
this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "passableLand");
passableLand.init(terrainGenerator.getPassableLand());
slope.init(terrainGenerator.getSlope());
this.passableLand = new BooleanMask(1, random.nextLong(), symmetrySettings, "passableLand");
this.slope = new FloatMask(1, random.nextLong(), symmetrySettings, "slope");
passableLand.init(terrainGenerator.getPassableLand());
slope.init(terrainGenerator.getSlope());
🤖 Prompt for AI Agents
In
generator/src/main/java/com/faforever/neroxis/generator/decal/DecalGenerator.java
around lines 32 to 36, there is a redundant assignment to this.slope and a wrong
mask name: remove the early assignment this.slope = terrainGenerator.getSlope()
(it is immediately overwritten), instantiate the FloatMask with the correct name
"slope" instead of "passableLand", and keep the BooleanMask for passableLand
named "passableLand"; then ensure
passableLand.init(terrainGenerator.getPassableLand()) and
slope.init(terrainGenerator.getSlope()) remain as the initializations.

Comment on lines 62 to +81
public void initialize(SCMap map, long seed, GeneratorParameters generatorParameters,
SymmetrySettings symmetrySettings, Pipeline pipeline) {
super.initialize(map, seed, generatorParameters, symmetrySettings, pipeline);

spawnLandMask = new BooleanMask(map.getSize() + 1, random.nextLong(), symmetrySettings, "spawnLandMask",
pipeline);
spawnPlateauMask = new BooleanMask(map.getSize() + 1, random.nextLong(), symmetrySettings, "spawnPlateauMask",
pipeline);
land = new BooleanMask(1, random.nextLong(), symmetrySettings, "land", pipeline);
mountains = new BooleanMask(1, random.nextLong(), symmetrySettings, "mountains", pipeline);
plateaus = new BooleanMask(1, random.nextLong(), symmetrySettings, "plateaus", pipeline);
ramps = new BooleanMask(1, random.nextLong(), symmetrySettings, "ramps", pipeline);
hills = new BooleanMask(1, random.nextLong(), symmetrySettings, "hills", pipeline);
valleys = new BooleanMask(1, random.nextLong(), symmetrySettings, "valleys", pipeline);
connections = new BooleanMask(1, random.nextLong(), symmetrySettings, "connections", pipeline);
heightmapValleys = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapValleys", pipeline);
heightmapHills = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapHills", pipeline);
heightmapPlateaus = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapPlateaus", pipeline);
heightmapMountains = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapMountains", pipeline);
heightmapLand = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapLand", pipeline);
heightmapOcean = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapOcean", pipeline);
heightMapNoise = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapNoise", pipeline);
SymmetrySettings symmetrySettings) {
super.initialize(map, seed, generatorParameters, symmetrySettings);

spawnLandMask = new BooleanMask(map.getSize() + 1, random.nextLong(), symmetrySettings, "spawnLandMask");
spawnPlateauMask = new BooleanMask(map.getSize() + 1, random.nextLong(), symmetrySettings, "spawnPlateauMask");
land = new BooleanMask(1, random.nextLong(), symmetrySettings, "land");
mountains = new BooleanMask(1, random.nextLong(), symmetrySettings, "mountains");
plateaus = new BooleanMask(1, random.nextLong(), symmetrySettings, "plateaus");
ramps = new BooleanMask(1, random.nextLong(), symmetrySettings, "ramps");
hills = new BooleanMask(1, random.nextLong(), symmetrySettings, "hills");
valleys = new BooleanMask(1, random.nextLong(), symmetrySettings, "valleys");
connections = new BooleanMask(1, random.nextLong(), symmetrySettings, "connections");
heightmapValleys = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapValleys");
heightmapHills = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapHills");
heightmapPlateaus = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapPlateaus");
heightmapMountains = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapMountains");
heightmapLand = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapLand");
heightmapOcean = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapOcean");
heightMapNoise = new FloatMask(1, random.nextLong(), symmetrySettings, "heightmapNoise");
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

LGTM! Comprehensive Pipeline removal.

The initialize method signature and all Mask constructors (BooleanMask and FloatMask) have been correctly updated to remove Pipeline dependency. This is a clean refactoring across 14 mask instances.

🤖 Prompt for AI Agents
In
generator/src/main/java/com/faforever/neroxis/generator/terrain/BasicTerrainGenerator.java
around lines 62 to 81, the initialize method and all BooleanMask/FloatMask
constructor calls were refactored to remove the Pipeline dependency and now use
the new signatures; there is no issue to fix — keep the current changes as-is
(approved).

protected void plateausSetup() {
int mapSize = map.getSize();
float maxStepSize = mapSize / 128f;
int maxStepSize = mapSize / 128;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find pathInCenterBounds method signature and its usage
rg -n 'pathInCenterBounds' --type java -A5 -B2

Repository: FAForever/Neroxis-Map-Generator

Length of output: 12166


🏁 Script executed:

#!/bin/bash
# Also check the full PathedPlateauTerrainGenerator.java file around the flagged lines
wc -l generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedPlateauTerrainGenerator.java

Repository: FAForever/Neroxis-Map-Generator

Length of output: 176


🏁 Script executed:

#!/bin/bash
# Look at the full context of lines around 9 and 17
head -50 generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedPlateauTerrainGenerator.java

Repository: FAForever/Neroxis-Map-Generator

Length of output: 931


🏁 Script executed:

#!/bin/bash
# Check if there are minimum map size constraints in the codebase
rg -n "getSize|mapSize.*=" generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedPlateauTerrainGenerator.java

Repository: FAForever/Neroxis-Map-Generator

Length of output: 113


🏁 Script executed:

#!/bin/bash
# Check inflate method signature to understand its parameter type
rg -n "inflate.*{" --type java -A3 | grep -A3 "float\|int"

Repository: FAForever/Neroxis-Map-Generator

Length of output: 158


🏁 Script executed:

#!/bin/bash
# Search for inflate method signature properly
rg -n 'inflate' --type java | grep 'public\|inflate(' | head -20

Repository: FAForever/Neroxis-Map-Generator

Length of output: 3254


🏁 Script executed:

#!/bin/bash
# Find the inflate method definition in BooleanMask or MapMask classes
fd -e java -path '*/mask/*' | xargs grep -l 'inflate' | head -5

Repository: FAForever/Neroxis-Map-Generator

Length of output: 243


🏁 Script executed:

#!/bin/bash
# Find the BooleanMask class and search for inflate method definition
find . -name "BooleanMask.java" -o -name "*Mask.java" | xargs grep -l "public.*inflate"

Repository: FAForever/Neroxis-Map-Generator

Length of output: 141


🏁 Script executed:

#!/bin/bash
# Search for inflate method definition in the mask directory
rg 'public.*inflate' --type java -A2

Repository: FAForever/Neroxis-Map-Generator

Length of output: 390


🏁 Script executed:

#!/bin/bash
# Check if there are any minimum map size validations in the codebase
rg -n "mapSize.*<|mapSize.*>|MIN.*MAP|minimum.*map" --type java generator/src/main/java/com/faforever/neroxis/generator/ | head -20

Repository: FAForever/Neroxis-Map-Generator

Length of output: 972


🏁 Script executed:

#!/antml:thinking
Let me verify if there are any minimum map size constraints that would prevent the truncation issue from occurring in practice.

Repository: FAForever/Neroxis-Map-Generator

Length of output: 118


🏁 Script executed:

#!/bin/bash
# Look for typical map sizes used in the codebase
rg -n "getSize\|mapSize.*[0-9]" --type java generator/src/main/java/com/faforever/neroxis/generator/ | grep -E "[0-9]{3,}" | head -20

Repository: FAForever/Neroxis-Map-Generator

Length of output: 57


🏁 Script executed:

#!/bin/bash
# Confirm the learned context about StrictMath usage in this codebase
rg -n 'StrictMath' --type java generator/src/main/java/com/faforever/neroxis/generator/ | head -10

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1922


Integer division truncates step size for small maps.

Changed from float maxStepSize = mapSize / 128f to int maxStepSize = mapSize / 128. Integer division loses precision before conversion to float—for maps smaller than 128 pixels, this produces 0 instead of a fractional value. Similarly, line 17 uses inflate(mapSize / 256), truncating values below 256. Given the emphasis on StrictMath for deterministic procedural generation, verify that pathInCenterBounds handles zero or reduced step sizes gracefully.

🤖 Prompt for AI Agents
In
generator/src/main/java/com/faforever/neroxis/generator/terrain/PathedPlateauTerrainGenerator.java
around line 9, the change from float to int causes integer division and
truncation (mapSize / 128 -> 0 for small maps) and similarly for the inflate
call at line 17; restore fractional precision by using floating-point division
(e.g., use float or double: mapSize / 128f or (double) mapSize / 128) for
maxStepSize and the inflate argument, and if downstream code expects integers,
explicitly round or clamp values (e.g., Math.max(1, Math.round(...)) or preserve
as float if supported); also review pathInCenterBounds usage to ensure it
handles zero or very small step sizes—if it cannot, clamp to a minimum positive
step (e.g., 1 or a small fraction) before calling.

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.2-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-9.2.1-bin.zip
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Gradle 9.2.1 Java 25 compatibility support requirements

💡 Result:

Short answer — Gradle 9.2.1 supports Java 25, but you must meet the usual JVM/toolchain requirements and be aware of some known edge cases:

  • Required JVM to run Gradle: any JVM between Java 17 and Java 25 (Gradle 9.2.1 is tested to run on up to Java 25). Use a supported JDK for the Gradle daemon or configure a toolchain for build tasks. [1]
  • Java 25 support introduced in Gradle 9.1.0 — so Java 25 is supported for running Gradle and as a toolchain in 9.2.1 (Gradle ≥ 9.1.0). [2]
  • If you use the Tooling API / native JNI clients you may need to enable native access at JVM startup per the release notes (see JEP/JNI notes). Also some plugins (e.g., Kotlin/Android toolchain integrations) have reported issues; check plugin compatibility if you see “incompatible Java 25” errors. [2][3]
  • Practical checklist:
    • Run Gradle with a JVM in the 17–25 range (set GRADLE_JAVA_HOME / wrapper JVM). [1]
    • Configure Java toolchains in build if you need to compile/test with Java 25 while running Gradle on a different JVM. [1]
    • Verify third‑party plugin compatibility (Kotlin, Android Gradle Plugin, IDE tooling) and update them if needed. [2][3]

Sources: Gradle compatibility matrix and docs (Java 17–25, Java‑25 support added in 9.1.0) and reported plugin/IDE issues. [1] [2] [3]

References
[1] Gradle Compatibility Matrix (Java versions supported).
[2] Gradle 9.1.0 release notes (introduces Java 25 support).
[3] gradle/gradle GitHub issue reporting Kotlin/IDE problems when using Java 25.


Verify Gradle 9.2.1 Java 25 compatibility and plugin support before upgrade.

Gradle 9.2.1 supports Java 25 (support introduced in 9.1.0) and can run on JVM versions 17–25. However, verify that your project's plugins (Kotlin, Android, IDE tooling) are compatible with Java 25, as some plugins have reported issues. Also ensure the Gradle JVM is configured correctly (via GRADLE_JAVA_HOME or toolchain) and, if using the Tooling API or JNI, enable native access as required by Java 25.

🤖 Prompt for AI Agents
In gradle/wrapper/gradle-wrapper.properties around line 3, the upgrade to Gradle
9.2.1 may require verification of Java 25 compatibility and plugin support;
confirm that all project plugins (Kotlin, Android Gradle Plugin, IDE tooling and
any third‑party plugins) support Java 25 and Gradle 9.2.1, update plugin
versions if needed, ensure the build uses the correct Gradle JVM by setting
GRADLE_JAVA_HOME or configuring the toolchain in gradle.properties/build.gradle,
and if your build uses the Tooling API or JNI enable the required native access
flags for Java 25; run local and CI builds/tests and fix any plugin or
configuration issues before merging.

Comment on lines +91 to +93
.inflate(teamSeparation / spawnMask.getSymmetrySettings()
.spawnSymmetry()
.getNumSymPoints()))
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Integer division may cause incorrect spawn placement.

The change from float to int division will truncate the result, which can significantly affect the inflate operation:

  • Before: teamSeparation / numSymPoints produced a float (e.g., 32 / 4 = 8.0, 2 / 4 = 0.5)
  • After: Integer division truncates (e.g., 32 / 4 = 8, 2 / 4 = 0)

When teamSeparation < numSymPoints, this produces 0 instead of a fractional value, which may cause:

  1. No inflation of symmetry lines (inflate(0))
  2. Incorrect spawn separation boundaries
  3. Potential spawn placement failures or clustering
🔎 Proposed fix

Either restore float division or use explicit rounding:

-                                     .inflate(teamSeparation / spawnMask.getSymmetrySettings()
-                                                                        .spawnSymmetry()
-                                                                        .getNumSymPoints()))
+                                     .inflate((int) Math.ceil((float) teamSeparation / spawnMask.getSymmetrySettings()
+                                                                        .spawnSymmetry()
+                                                                        .getNumSymPoints())))

Or verify that inflate() now accepts int and handles small values appropriately.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.inflate(teamSeparation / spawnMask.getSymmetrySettings()
.spawnSymmetry()
.getNumSymPoints()))
.inflate((int) Math.ceil((float) teamSeparation / spawnMask.getSymmetrySettings()
.spawnSymmetry()
.getNumSymPoints())))
🤖 Prompt for AI Agents
In shared/src/main/java/com/faforever/neroxis/map/placement/SpawnPlacer.java
around lines 91-93, integer division is being used for the inflate argument
which truncates fractional values (e.g. 2/4 -> 0) and can cause no inflation and
incorrect spawn placement; change the expression to perform floating-point
division (cast one operand to float/double) so fractional results are preserved,
or explicitly compute a safe non-zero inflate value (e.g. Math.max(minValue,
(float)teamSeparation / numSymPoints) or similar) and ensure inflate receives
the correct type expected by the method.

BooleanMask realWater = realLand.copy().invert();
BooleanMask shadowsMask = heightMap
.copyAsShadowMask(map.getBiome().lightingSettings().sunDirection()).inflate(0.5f);
.copyAsShadowMask(map.getBiome().lightingSettings().sunDirection()).inflate(1);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java | sed -n '40,60p'

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1476


🏁 Script executed:

git log --oneline -20 toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java

Repository: FAForever/Neroxis-Map-Generator

Length of output: 102


🏁 Script executed:

git diff HEAD~5 toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java | head -100

Repository: FAForever/Neroxis-Map-Generator

Length of output: 261


🏁 Script executed:

find . -name "*.java" -type f | xargs grep -l "class BooleanMask" | head -5

Repository: FAForever/Neroxis-Map-Generator

Length of output: 141


🏁 Script executed:

rg "def inflate|inflate\(" --type java -A 2 | head -80

Repository: FAForever/Neroxis-Map-Generator

Length of output: 8969


🏁 Script executed:

rg "\.inflate\(" --type java | grep -E "(0\.5|1|inflate)" | head -20

Repository: FAForever/Neroxis-Map-Generator

Length of output: 3397


🏁 Script executed:

cat -n shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java | head -100

Repository: FAForever/Neroxis-Map-Generator

Length of output: 4636


🏁 Script executed:

rg "inflate" shared/src/main/java/com/faforever/neroxis/mask/BooleanMask.java -A 5

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1172


🏁 Script executed:

git log --all --oneline -- "*MapInfoTextureExporter*" | head -10

Repository: FAForever/Neroxis-Map-Generator

Length of output: 150


🏁 Script executed:

git show 7e725ca:toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java | sed -n '40,60p'

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1329


🏁 Script executed:

git show fa04454:toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java | sed -n '40,60p'

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1347


The shadow mask inflation change from 0.5f to 1 effectively enables shadow spread that was previously disabled.

The old code inflate(0.5f) was compiled as inflate(0) due to implicit float-to-int truncation in the inflate(int radius) method signature, meaning no inflation occurred. The new code inflate(1) enables actual 1-pixel inflation. This is a behavioral change that will affect shadow rendering in the generated map info texture. Confirm this is intentional.

🤖 Prompt for AI Agents
In
toolsuite/src/main/java/com/faforever/neroxis/toolsuite/MapInfoTextureExporter.java
around line 48, the call .copyAsShadowMask(...).inflate(1) changes behavior by
enabling a 1-pixel shadow spread that previously did not occur (inflate(0.5f)
was truncated to 0); decide whether this was intentional and either revert to
the previous no-inflate behavior by calling inflate(0) (or keep the prior
literal 0.5f but change the API to accept floats), or if the spread is desired
keep inflate(1) and add a comment explaining the deliberate change; update the
code accordingly and add a short comment stating the intended shadow-inflation
behavior.

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

♻️ Duplicate comments (1)
.github/workflows/release.yml (1)

48-54: Update actions/checkout from v2 to v4.

actions/checkout@v2 is outdated. The actions/setup-java@v3 issue was already flagged in a previous review.

🔎 Proposed fix
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: Set up JDK 25
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e725ca and 9bf47f3.

📒 Files selected for processing (1)
  • .github/workflows/release.yml
🧰 Additional context used
🪛 actionlint (1.7.9)
.github/workflows/release.yml

19-19: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


50-50: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

⏰ 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

Comment on lines 17 to 23
- uses: actions/checkout@v2
- name: Set up JDK 21
- name: Set up JDK 25
uses: actions/setup-java@v3
with:
distribution: "temurin"
java-version: 21
java-version: 25
cache: 'gradle'
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Update outdated GitHub Actions in the build job.

  • actions/checkout@v2 is outdated; current major version is v4.
  • actions/setup-java@v3 is outdated; current version is v4.x (static analysis confirms this).
🔎 Proposed fix
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
       - name: Set up JDK 25
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: "temurin"
           java-version: 25
           cache: 'gradle'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- uses: actions/checkout@v2
- name: Set up JDK 21
- name: Set up JDK 25
uses: actions/setup-java@v3
with:
distribution: "temurin"
java-version: 21
java-version: 25
cache: 'gradle'
- uses: actions/checkout@v4
- name: Set up JDK 25
uses: actions/setup-java@v4
with:
distribution: "temurin"
java-version: 25
cache: 'gradle'
🧰 Tools
🪛 actionlint (1.7.9)

17-17: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)


19-19: the runner of "actions/setup-java@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue

(action)

🤖 Prompt for AI Agents
.github/workflows/release.yml around lines 17 to 23: the workflow uses outdated
action versions (actions/checkout@v2 and actions/setup-java@v3); update
actions/checkout to the current major (v4) and actions/setup-java to v4.x, keep
the existing inputs (distribution: "temurin", java-version: 25, cache: 'gradle')
intact, and run/validate the workflow after updating to ensure no syntax or
input changes are required by the new major versions.

Comment on lines +60 to +63
run: |
./gradlew -x test --stacktrace jlink --parallel -PgeneratorVersion="${{ github.event.inputs.version }}"
./generator/build/image/bin/java -XX:AOTCacheOutput=generator/build/image/bin/neroxis-generator.aot -XX:+UseCompactObjectHeaders -m com.faforever.neroxis.generator/com.faforever.neroxis.generator.MapGenerator --out-path ./GeneratedMaps
./gradlew -x test --stacktrace jpackage --parallel -PgeneratorVersion="${{ github.event.inputs.version }}"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -name "release.yml" -path "*/.github/workflows/*" | head -5

Repository: FAForever/Neroxis-Map-Generator

Length of output: 106


🏁 Script executed:

cat -n ./.github/workflows/release.yml | sed -n '50,75p'

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1468


🏁 Script executed:

cat -n ./.github/workflows/release.yml | sed -n '1,50p'

Repository: FAForever/Neroxis-Map-Generator

Length of output: 1776


🌐 Web query:

jlink output Windows java.exe executable extension GitHub Actions

💡 Result:

Short answer: jlink produces a platform-specific runtime image that includes the java launcher with the normal OS filename — on Windows that is java.exe in the image's bin\ directory. A Windows-targeted jlink image will contain bin\java.exe; a Linux-targeted image will contain bin/java (no .exe) and will not run on Windows. When using GitHub Actions build matrix, generate the Windows jlink image on a Windows runner (or use Windows JDK jmods when cross-building) and invoke the runtime via /bin/java.exe (or add that bin dir to PATH). Ensure you run the Windows image on a Windows runner — Linux/macOS runners will not execute .exe binaries. [1][2][3]

Sources

  • jlink runtime image description (shows bin/java and platform-specific images). [1]
  • Microsoft doc: jlink image contains ./bin/java; Windows images use java.exe. [2]
  • Example GitHub Actions + jlink (JReleaser) showing Windows target images and Actions configuration. [3]

🌐 Web query:

GitHub Actions Windows shell bash git bash java.exe extension resolution

💡 Result:

Short answer: On Windows runners GitHub Actions’ shell: bash runs the Git-for-Windows bash (bash.exe). That bash still relies on Windows PATH/PATHEXT behavior and the MSYS/MSYS2 emulation layer — so executable lookup can differ from native cmd/powershell. If a program isn’t found when you call its bare name (java) but exists as java.exe, call java.exe or use the full path or fix PATH so bash can see the Windows location. You can also switch the step to a Windows-native shell (pwsh or cmd) where PATHEXT resolution behaves like normal Windows. Examples and links below.

Actionable options

  • Call java.exe explicitly: run: java.exe -version.
  • Use full Windows path: run: "/c/Program Files/Java/jdk.../bin/java.exe" -version (or quoted Windows path).
  • Ensure the Java bin directory is on PATH (export PATH="/c/Program Files/Java/…/bin:$PATH") in bash.
  • Use shell: pwsh or shell: cmd if you need native Windows command resolution.
  • Or set a custom shell to a different bash (Cygwin/MSYS2) if required. (docs.github.com)

If you want, tell me which runner image and workflow step (example run block) and I’ll give the exact syntax to fix it.

Citations:


🏁 Script executed:

cat -n ./.github/workflows/release.yml | sed -n '59,64p'

Repository: FAForever/Neroxis-Map-Generator

Length of output: 858


AOT generation will fail on Windows runners due to missing .exe extension.

When jlink runs on Windows, it produces java.exe, not java. The AOT command at line 62 uses ./generator/build/image/bin/java, which Git Bash on Windows runners cannot resolve. Use ./generator/build/image/bin/java.exe or implement platform-agnostic logic (e.g., shell conditional or a Gradle task to handle the invocation).

The workflow matrix includes windows-latest, so this will cause builds to fail on Windows.

🤖 Prompt for AI Agents
.github/workflows/release.yml around lines 60 to 63: the workflow invokes the
jlink-generated Java binary as ./generator/build/image/bin/java which fails on
Windows where the runtime is java.exe; update the step to use a platform-aware
invocation — either call ./generator/build/image/bin/java.exe when runner.os ==
'Windows' (use a conditional job/step), or implement a small cross-platform
wrapper/Gradle task that resolves the correct binary name and runs the AOT
command so Linux/macOS use ./generator/build/image/bin/java and Windows uses
./generator/build/image/bin/java.exe.

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