[dfloreaa]

Description

Since #4470, a function decorated with height_field_to_mesh must return the height field together with the terrain origin, and the decorator unpacks the result directly:

z_gen, origin = func(difficulty, cfg)

A custom generator written against the previous contract returns only the height field, so terrain generation fails inside the decorator with

ValueError: too many values to unpack (expected 2)

The error names neither the generator nor the contract change, and it comes after the decorator has already shrunk cfg.size for the generator without restoring it. We hit this with downstream height-field generators when moving a project from release/3.0.0-beta2 to main.

This turns a failure to unpack the generator's result into a TypeError that names the generator and the expected (height_field, origin) return. Any two-item return that unpacked before, such as a list, still works. The decorator now restores cfg.size in a finally block, so the size is also restored when the generator or the border assignment raises. The decorator's docstring now lists both errors it raises in a Raises: section. Generators that already return a tuple are unchanged, which covers all six built-in height-field terrains.

release/3.0.0 does not contain #4470, so no backport is needed.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with uv run isaaclab --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have validated the changed behavior with focused coverage and applied the test-audit criteria to any test changes
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump package versions — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

Testing

• Added test_height_field_generator_without_origin_raises_type_error to test/terrains/test_terrain_generator.py: a decorated generator that returns only the height field must raise the TypeError and leave cfg.size unchanged. It fails on main with the ValueError above and passes with the fix.
• The whole test_terrain_generator.py passes locally (28 passed).
• uv run isaaclab --format passes, including the changelog-fragment check.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The PR gives custom height-field generators a clearer error when they omit the origin.

• Height-field generators get a clear error when they omit the terrain origin.

_{Reviews (1) · Last reviewed commit: "Raise a clear error for height-field gen..." · Reviewed by Greptile}

[isaaclab-review-bot]

Isaac Lab Review Bot

Needs attention: Design and architecture (1), Compatibility and deprecation (1), Test quality (1). Other reviewed categories look good.

• PR description: The body matches the guard, documentation, changelog, and regression test, but “non-breaking” is inaccurate because the tuple-only check rejects previously accepted two-item iterables; disclose that narrowing.
• Design and architecture: The new exceptional restore duplicates success-path cleanup and still leaks temporary cfg.size when the generator or border assignment raises; restore it once with try/finally.
• Scope and over-engineering: No over-engineering issues.
• API: API changes look good.
• Compatibility and deprecation: Breaking changes: the tuple-only guard rejects two-item lists and other iterable returns previously accepted by unpacking; preserve iterable unpacking or provide a complete deprecation bridge.
• Implementation: Implementation looks good.
• Style consistency: Style consistency looks good.
• Test quality: The regression test covers the error and restored size, but its unused pixel calculations repeat production logic; return any array directly and remove the extra import.

Minor fixes needed. Posted 3 inline findings.

Automated review; human maintainers own approval decisions.

[AntoineRichard]

Thanks for the fix @dfloreaa LGTM!

[AntoineRichard]

run-ci