fix: forward CLI args to AppLauncher in state machine scripts (#5572)#5654
fix: forward CLI args to AppLauncher in state machine scripts (#5572)#5654dparikh79 wants to merge 1 commit into
Conversation
The three state machine scripts under scripts/environments/state_machine/ register the full AppLauncher CLI surface via AppLauncher.add_app_launcher_args(parser) and parse it into args_cli, but then instantiate AppLauncher with only the headless flag: app_launcher = AppLauncher(headless=args_cli.headless) This drops every other parsed AppLauncher argument. The most visible symptom (reported in isaac-sim#5572) is that --viz kit on Isaac Lab 3.0 beta is silently ignored: the process stays alive, GPU goes into compute+graphics mode, but no Kit / Isaac Sim window opens. The same gap also drops --kit-args, --device, --enable_cameras, --livestream and others. Every other Isaac Lab 3 script that calls add_app_launcher_args passes the full args namespace through (e.g. scripts/tutorials/00_sim/*.py uses AppLauncher(args_cli)). Align the three state machine scripts (lift_cube_sm.py, open_cabinet_sm.py, lift_teddy_bear.py) to that pattern so --viz and the other registered flags reach the launcher. Fixes isaac-sim#5572 Signed-off-by: Dhruvil <dhruvilparikh79@gmail.com>
Greptile SummaryThis PR fixes a bug in the three state machine scripts under
Confidence Score: 5/5Three identical one-line corrections to script-level launcher invocations — no library code, no API surface, no data path touched. The change is minimal and mechanical: it removes a single keyword filter and passes the already-parsed argparse namespace directly to No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant argparse
participant AppLauncher
participant SimApp
User->>argparse: parse_args() (--headless, --viz, --device, --livestream, ...)
argparse-->>User: args_cli (full namespace)
Note over User,AppLauncher: Before fix
User->>AppLauncher: "AppLauncher(headless=args_cli.headless)"
Note right of AppLauncher: --viz, --device, --livestream silently dropped
Note over User,AppLauncher: After fix
User->>AppLauncher: AppLauncher(args_cli)
AppLauncher->>SimApp: Launch with full CLI config
SimApp-->>AppLauncher: simulation_app
AppLauncher-->>User: app_launcher.app
Reviews (1): Last reviewed commit: "fix: forward CLI args to AppLauncher in ..." | Re-trigger Greptile |
There was a problem hiding this comment.
Code Review Summary
This PR correctly fixes issue #5572 by aligning the three state machine scripts with the canonical AppLauncher initialization pattern used throughout Isaac Lab.
✅ What Works Well
-
Minimal, targeted fix: The change is exactly one line per file, replacing
AppLauncher(headless=args_cli.headless)withAppLauncher(args_cli). This matches the established pattern in tutorial scripts. -
Backward compatibility preserved: Since
AppLauncherreadsheadlessfrom the namespace object, existing callers using only--headlesssee no behavior change. -
Root cause addressed: The original code registered AppLauncher CLI arguments via
add_app_launcher_args(parser)but then ignored them during instantiation. This fix ensures all parsed arguments (--viz,--device,--kit-args,--enable_cameras,--livestream, etc.) are properly forwarded. -
Good PR documentation: The description includes pattern verification with
grepexamples, clear reproduction steps, and links to the issue.
📋 Observations
-
No test changes: Understandable given these are example scripts rather than core library code. The manual verification steps in the PR description are appropriate.
-
Consistent with codebase patterns: Verified that
scripts/tutorials/00_sim/and other directories useAppLauncher(args_cli)as the standard pattern.
Verdict
LGTM ✅ — Clean, minimal bug fix that properly aligns these scripts with established Isaac Lab conventions. The change is low-risk and directly addresses the reported issue.
Summary
Fixes #5572.
The three state machine scripts under
scripts/environments/state_machine/register the full AppLauncher CLI surface viaAppLauncher.add_app_launcher_args(parser)and parse it intoargs_cli, but then instantiate AppLauncher with only theheadlessflag:This drops every other parsed AppLauncher argument. The most visible symptom (the one @agundogdu-bdai reported in #5572) is that
--viz kiton Isaac Lab 3.0 beta is silently ignored: the process stays alive, GPU goes into compute+graphics mode, but no Kit / Isaac Sim window opens. The same gap also drops--kit-args,--device,--enable_cameras,--livestream, and any other registered AppLauncher flag.Every other Isaac Lab 3 script that calls
add_app_launcher_argspasses the fullargs_clinamespace through. Aligning the three state machine scripts to that pattern fixes the reported--viz kitregression and the silent drop of the other flags. Credit to @agundogdu-bdai for the root-cause analysis and the proposed one-line fix in the issue body; this PR applies that fix and extends it to the two sibling state machine scripts that have the same bug.What changed
Three one-line changes, identical across the three scripts:
Files:
scripts/environments/state_machine/lift_cube_sm.pyscripts/environments/state_machine/open_cabinet_sm.pyscripts/environments/state_machine/lift_teddy_bear.pyNo breaking change: callers using only
--headlesssee no behavior difference becauseargs_cli.headlessis still respected by the AppLauncher when reading the full namespace. Callers using--viz,--kit-args,--device, etc., now have those flags reach the launcher as the parser already advertised.Pattern verification
Sample of in-tree precedent (the canonical pattern for Isaac Lab 3 scripts):
How to verify
@agundogdu-bdai's repro from the issue:
Before: process stays alive, no Kit window opens.
After: Kit visualizer opens as expected, matching
scripts/tutorials/00_sim/create_empty.py --viz kit.Checklist
-s)AI Assistance Disclosure
This PR was drafted with AI assistance (Claude Code). I read the issue, verified the issue author's analysis by tracing all 7 sites where AppLauncher is instantiated under
scripts/, confirmed the tutorials canonical pattern, and applied the same one-line fix the author proposed to the two sibling state machine scripts that have the same bug. The fix is the minimum change to align these three scripts with the rest of the codebase.