-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Moves parameter platform_height
to the correct mesh terrain configuration
#3316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug where the platform_height
parameter was incorrectly placed in the MeshPyramidStairsTerrainCfg
class instead of the appropriate base configuration class for mesh terrain objects.
- Removes the misplaced
platform_height
parameter fromMeshPyramidStairsTerrainCfg
- Adds the
platform_height
parameter to the correct location in theMeshRepeatedObjectsTerrainCfg.ObjectCfg
class - Includes various formatting improvements with additional blank lines for consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
source/isaaclab/isaaclab/terrains/trimesh/mesh_terrains_cfg.py | Moves platform_height from wrong class to correct base configuration and adds formatting improvements |
source/isaaclab/isaaclab/terrains/height_field/hf_terrains_cfg.py | Formatting improvements with additional blank lines and fixes a type annotation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Test Results Summary2 446 tests 2 037 ✅ 2h 13m 34s ⏱️ For more details on these failures, see this check. Results for commit 8970553. |
Thanks for fixing this bug #3162! Could it be useful if |
@heleiduan Good point. I was checking the same today why this wasn't caught in the CI. Will change the script to be a test instead :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix. That was a bad transfer from our end.
Merging this for now. Will make a unit test for it once I get more time. |
Description
This PR fixes a bug where the platform_height parameter was incorrectly placed in the MeshPyramidStairsTerrainCfg class instead of the appropriate base configuration class for mesh terrain objects.
platform_height
parameter fromMeshPyramidStairsTerrainCfg
platform_height
parameter to the correct location in theMeshRepeatedObjectsTerrainCfg
classFixes #3162
Regression was introduced in MR #2695
Type of change
Checklist
pre-commit
checks with./isaaclab.sh --format
config/extension.toml
fileCONTRIBUTORS.md
or my name already exists there