Separate spawners and object types.

[alexmillane]

Summary

Refactors how we handle spawner objects, to separate their specification from object type

Detailed description

• Addresses [Improvement] Separate spawner and object type concepts. #592
• Separate ObjectType and SPAWNER

[greptile-apps]

Greptile Summary

This PR separates the SPAWNER pseudo-type from ObjectType, replacing it with an explicit spawner_cfg: SpawnerCfg | None parameter on Object. GroundPlane and DomeLight migrate to ObjectType.BASE + spawner_cfg, and a new Sphere class is added using the same pattern for ObjectType.RIGID. Previous review concerns (prim_path mis-routing for GroundPlane/DomeLight, confusing AssertionError when object_type is unset) are resolved.

• P1 — Sphere.get_bounding_box() crashes in placement: get_bounding_box() and get_corners() assert self.usd_path is not None; Sphere has usd_path=None. relation_solver and object_placer call get_bounding_box() on every object with a spatial relation, so adding any relation to a Sphere crashes at solve time with an opaque AssertionError.
• P2 — activate_contact_sensors silently dropped for spawner-only objects: _get_spawn_cfg returns self.spawner_cfg unchanged, ignoring the activate_contact_sensors=True flag that _generate_rigid_cfg passes.
• P2 — relations parameter in Object.__init__ is never forwarded and uses a mutable default list.

Confidence Score: 4/5

Safe to merge for existing USD-based objects; the new Sphere class will silently break in any placement scenario with spatial relations.

One P1 finding remains: Sphere.get_bounding_box() will crash with an opaque AssertionError the first time it is used in a placement context, which is the natural use case for an object tagged [object] with type RIGID. Prior P0/P1 concerns from the previous review round are resolved.

isaaclab_arena/assets/object.py (bounding-box/contact-sensor gaps) and isaaclab_arena/assets/object_library.py (Sphere missing bounding-box support).

Important Files Changed

Filename	Overview
isaaclab_arena/assets/object.py	Core refactor: adds spawner_cfg param and splits dispatch into _get_spawn_cfg; bounding-box crash for spawner-only objects (P1), activate_contact_sensors silently ignored (P2), dead relations parameter (P2).
isaaclab_arena/assets/object_base.py	Removes SPAWNER from ObjectType enum and cleans up dispatch; no new issues.
isaaclab_arena/assets/object_library.py	Migrates GroundPlane/DomeLight to ObjectType.BASE + spawner_cfg and adds Sphere; Sphere will crash in placement contexts due to missing bounding-box support.
isaaclab_arena/assets/object_reference.py	Minor adaptation to match ObjectType enum changes; no new issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Object.__init__(usd_path, spawner_cfg, object_type)"] --> B{usd_path AND spawner_cfg None?}
    B -- yes --> C[raise ValueError]
    B -- no --> D{object_type None?}
    D -- yes --> E{usd_path is None?}
    E -- yes --> F[assert / clear error]
    E -- no --> G[detect_object_type usd_path]
    D -- no --> H[use provided object_type]
    G --> H
    H --> I[_init_object_cfg]
    I --> J{object_type?}
    J -- RIGID --> K[_generate_rigid_cfg]
    J -- ARTICULATION --> L[_generate_articulation_cfg]
    J -- BASE --> M[_generate_base_cfg]
    K --> N["_get_spawn_cfg(activate_contact_sensors=True)"]
    L --> N
    M --> O["_get_spawn_cfg()"]
    N --> P{spawner_cfg set?}
    O --> P
    P -- yes --> Q["return spawner_cfg as-is ⚠️ activate_contact_sensors ignored"]
    P -- no --> R["UsdFileCfg(usd_path, scale, activate_contact_sensors)"]

Loading

Comments Outside Diff (1)

1. isaaclab_arena/assets/object.py, line 67-91 (link)

   get_bounding_box() / get_corners() crash for spawner-only objects

   Both methods assert self.usd_path is not None, but Sphere (and any future spawner-only RIGID object) has usd_path=None. relation_solver.py and object_placer.py call get_bounding_box() on every object with a relation. Since Sphere is tagged ["object"] and typed RIGID, users will naturally add it with spatial relations — triggering an opaque AssertionError at solve time rather than a clear API error.

   get_contact_sensor_cfg() (line 104) has the same silent failure: usd_path = usd_path or self.usd_path resolves to None for spawner-only objects, then find_shallowest_rigid_body(None, ...) will crash.

_{Reviews (3): Last reviewed commit: "Revert gh config inclusion." | Re-trigger Greptile}

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR cleanly separates the SPAWNER concept from ObjectType, addressing issue #592. Objects like GroundPlane, Sphere, and DomeLight now use their correct semantic types (BASE, RIGID) while passing a custom spawner_cfg for spawn configuration. The design is sound, but there's a critical prim path regression for global objects and a missing type annotation worth addressing.

Design Assessment

Design is sound. Decoupling the spawning mechanism from the object type is the right call — it eliminates the ObjectType.SPAWNER hack where objects had to pretend to be a different type just because they used a custom spawn function. The new _get_spawn_cfg() dispatch method is clean and minimal.

Findings

🔴 Critical: Prim path regression for GroundPlane and DomeLight — isaaclab_arena/assets/object.py:180

_generate_base_cfg() hardcodes the prim path to "{ENV_REGEX_NS}/" + self.name, but GroundPlane and DomeLight set global prim paths (/World/GroundPlane and /World/Light respectively) that should NOT be scoped per-environment.

Previously, these objects used _generate_spawner_cfg() which correctly used self.prim_path. Now that they route through _generate_base_cfg(), their custom prim paths are silently ignored.

Impact: GroundPlane and DomeLight will be spawned at {ENV_REGEX_NS}/ground_plane and {ENV_REGEX_NS}/light instead of /World/GroundPlane and /World/Light — duplicating global scene elements per environment and likely breaking rendering/physics.

Suggested fix: Use self.prim_path in _generate_base_cfg():

        object_cfg = AssetBaseCfg(
            prim_path=self.prim_path,
            spawn=self._get_spawn_cfg(),
            **self.asset_cfg_addon,
        )

Note: This changes behavior for existing BASE objects that rely on the {ENV_REGEX_NS}/ prefix. If that's intentional, the fix should be conditional — e.g., only use self.prim_path when it's explicitly set by the caller (not the default).

🟡 Warning: spawner_cfg parameter lacks type annotation — isaaclab_arena/assets/object.py:36

The new spawner_cfg parameter has no type hint:

spawner_cfg=None,

The codebase follows a consistent pattern of typing all constructor parameters. The spawner config accepts various Isaac Sim spawn config types.

Suggested fix:

        spawner_cfg: sim_utils.SpawnerCfg | None = None,

(Or the appropriate base type from isaaclab.sim.spawners if SpawnerCfg isn't the correct parent.)

🟡 Warning: detect_object_type will crash if spawner_cfg is provided without object_type — isaaclab_arena/assets/object.py:46

When creating an Object with spawner_cfg and no usd_path, if object_type is omitted, the code calls detect_object_type(usd_path=None) which asserts that at least one of usd_path or stage is provided. This produces an unhelpful AssertionError instead of guiding the user.

Currently all library objects set object_type explicitly, so this doesn't bite today — but it's a trap for future callers.

Suggested fix: Add a guard before detection:

if object_type is None:
    if usd_path is not None:
        object_type = detect_object_type(usd_path=usd_path)
    else:
        raise ValueError(
            f"Object '{name}' with spawner_cfg must specify object_type explicitly"
        )

🔵 Suggestion: Docker volume mount is unrelated to this refactor — docker/run_docker.sh:135

The .config/gh volume mount is a separate concern from the spawner/object-type refactoring. Consider splitting this into its own PR to keep the refactoring commit focused.

Test Coverage

• Bug fix PR: N/A — this is a refactoring PR
• New code: No new tests included
• Gaps: The refactoring changes runtime behavior for GroundPlane, Sphere, and DomeLight. Given the prim path regression identified above, at minimum a smoke test verifying that these objects produce the expected config (correct prim_path, correct spawn type, correct object_type) would prevent regressions. Existing integration tests may catch the prim path issue at runtime, but explicit unit tests for config generation would be more reliable.

CI Status

⏳ Pre-commit check is pending.

Verdict

Significant concerns

The separation of ObjectType and spawner config is well-designed and addresses a real architectural problem. However, the prim path regression for GroundPlane and DomeLight (routed through _generate_base_cfg which ignores self.prim_path) is a correctness issue that will break these global scene objects. This should be fixed before merging.

[viiik-inside]

Looks great