Feature: Adds rough slope terrain #4468
Conversation
Greptile OverviewGreptile SummaryThis PR introduces rough slope terrain generation by combining pyramid sloped terrain with random uniform noise. However, there is a critical bug in the platform clipping logic at Key Changes
Critical IssueThe platform preservation logic uses Additional Concerns
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Decorator as height_field_to_mesh
participant RST as rough_slope_terrain
participant PST as pyramid_sloped_terrain.__wrapped__
participant RUT as random_uniform_terrain.__wrapped__
User->>Decorator: Call rough_slope_terrain(difficulty, cfg)
Decorator->>Decorator: Modify cfg.size for border
Decorator->>RST: Call unwrapped function
RST->>PST: Get sloped terrain heights
PST-->>RST: Return np.int16 array (terrain1)
RST->>RUT: Get random noise heights
RUT-->>RST: Return np.int16 array (terrain2)
RST->>RST: Combine: hf = terrain1 + terrain2
RST->>RST: Calculate platform position (x_pf, y_pf)
RST->>RST: Get platform height: z_pf = hf[x_pf, y_pf]
RST->>RST: ⚠️ BUG: Clip entire terrain to [min(0, z_pf), max(0, z_pf)]
RST-->>Decorator: Return flattened height field
Decorator->>Decorator: Convert height field to mesh
Decorator-->>User: Return mesh and origin
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| x_pf = width_pixels // 2 - platform_width | ||
| y_pf = length_pixels // 2 - platform_width | ||
| z_pf = hf[x_pf, y_pf] | ||
| hf = np.clip(hf, min(0, z_pf), max(0, z_pf)) |
There was a problem hiding this comment.
This clips the entire terrain to a flat range, eliminating both the slope and roughness. The logic should only flatten the platform region, not the whole terrain.
The correct approach: calculate platform boundaries and only set hf[center_region] = z_pf, similar to how pyramid_sloped_terrain builds the platform before clipping.
There was a problem hiding this comment.
For the pyramid slope terrain, this clip is ok. Though the whole nparray is clipped. It is same with the pyramid_sloped_terrain method.
| class HfRoughSlopeTerrainCfg(HfRandomUniformTerrainCfg, HfPyramidSlopedTerrainCfg): | ||
| "rough slope cfg, combine the cfg of random terrain and smooth slope terrain" |
There was a problem hiding this comment.
Missing proper docstring formatting - should use triple quotes and follow the same style as other config classes (e.g., HfInvertedPyramidSlopedTerrainCfg on lines 82-89)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| class HfInvertedRoughSlopeTerrainCfg(HfRandomUniformTerrainCfg, HfInvertedPyramidSlopedTerrainCfg): | ||
| "rough slope cfg, combine the cfg of random terrain and smooth slope terrain" |
There was a problem hiding this comment.
Missing proper docstring formatting - should use triple quotes and follow the same style as other config classes
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
|
||
|
|
||
| @height_field_to_mesh | ||
| def rough_slope_terrain(difficulty: float, cfg: hf_terrains_cfg.HfRoughSlopeTerrainCfg) -> np.ndarray: |
There was a problem hiding this comment.
Missing docstring. Other terrain functions have comprehensive docstrings explaining parameters, return values, and behavior (see pyramid_sloped_terrain at line 83 or random_uniform_terrain at line 21).
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @height_field_to_mesh | ||
| def rough_slope_terrain(difficulty: float, cfg: hf_terrains_cfg.HfRoughSlopeTerrainCfg) -> np.ndarray: | ||
| # combine the two methods | ||
| terrain1 = pyramid_sloped_terrain.__wrapped__(difficulty, cfg) |
There was a problem hiding this comment.
Ensure both wrapped functions return arrays of matching dimensions before addition - test with various cfg.size and cfg.border_width values
Mayankm96
force-pushed
the
main
branch
2 times, most recently
from
2ef7fc8 to
f3061a4
Compare
1 participant
Description
Adds the rough slope terrain and its cfg, including both inverted and not inverted version. I combine the already defined
pyramid_sloped_terrainandrandom_uniform_terrainmethod and their config to keep code simple.Type of change
Screenshots
The rough slope terrains and the inverted version generated:
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there