Fix visualizer initialization

[gavrielstate]

Description

Fixes visualizer initialization regression; always store settings in a dictionary.

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 ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Summary

This PR fixes a visualizer initialization regression in SettingsManager by making _standalone_settings the authoritative source of truth: set() now always writes to the dictionary (and additionally syncs to carb.settings when available), while get() now reads exclusively from the dictionary in both modes.

Key changes:

• set(): removed the if/else branching so _standalone_settings is always populated, regardless of whether carb is active.
• get(): simplified to always return from _standalone_settings, removing the carb delegation path entirely.

Issues found:

• initialize_carb_settings() does not sync pre-existing _standalone_settings values into carb.settings when carb becomes available. Any Omniverse extension or internal Isaac Sim code that reads settings directly from carb.settings (not through SettingsManager.get()) will not see values that were set before SimulationApp launched.
• The updated get() docstring does not document that it no longer reads from carb.settings in Omniverse mode, which can cause silent divergence surprises for carb-native consumers.

Confidence Score: 3/5

• The core fix is correct and addresses the regression, but the missing back-sync from dictionary to carb on initialization is a risk in Omniverse mode.
• The change correctly solves the described regression (visualizer settings lost when set before carb is available). However, initialize_carb_settings() does not propagate already-stored values into carb, meaning any carb-native consumer that was previously relying on SettingsManager to populate carb will see missing values after SimulationApp starts. This is a real gap that could resurface as a new regression in Omniverse deployments.
• source/isaaclab/isaaclab/app/settings_manager.py — specifically the initialize_carb_settings() method and the get() docstring

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/app/settings_manager.py	Fixes visualizer regression by making _standalone_settings the always-written source of truth in set() and the sole read source in get(), but does not sync pre-existing dictionary values to carb when initialize_carb_settings() is called, leaving carb-native consumers with potentially missing data.

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant SM as SettingsManager
    participant D as _standalone_settings (dict)
    participant CS as carb.settings (Omniverse)

    Note over SM: Before initialize_carb_settings()
    C->>SM: set(path, value)
    SM->>D: store value [always]
    Note over CS: carb not yet available — no sync

    C->>SM: get(path)
    SM->>D: lookup
    D-->>SM: value
    SM-->>C: value

    Note over SM: After initialize_carb_settings()
    SM->>CS: connect to carb.settings
    Note over D,CS: ⚠️ Pre-existing dict values NOT synced to carb here

    C->>SM: set(path, value)
    SM->>D: store value [always]
    SM->>CS: sync value (set_bool / set_int / etc.)

    C->>SM: get(path)
    SM->>D: lookup [only dict, carb ignored]
    D-->>SM: value
    SM-->>C: value

    Note over CS: External code writing directly to carb.settings<br/>will NOT be visible via SettingsManager.get()

Loading

Comments Outside Diff (1)

1. source/isaaclab/isaaclab/app/settings_manager.py, line 78-91 (link)

   Pre-existing settings not synced to carb when carb becomes available

   When initialize_carb_settings() is called (after SimulationApp is created), any settings already stored in _standalone_settings (set prior to carb initialization) are not synced to carb.settings. This means any Omniverse extension or Isaac Lab code that reads settings directly from carb.settings (rather than through SettingsManager.get()) will not see values that were set before initialize_carb_settings() was invoked.

   Consider adding a sync loop in initialize_carb_settings() to push pre-existing dictionary values into carb after carb becomes available:

   def initialize_carb_settings(self):
       try:
           import carb
   
           self._carb_settings = carb.settings.get_settings()
           self._use_carb = True
   
           # Sync pre-existing standalone settings into carb now that it's available
           for path, value in self._standalone_settings.items():
               if isinstance(value, bool):
                   self._carb_settings.set_bool(path, value)
               elif isinstance(value, int):
                   self._carb_settings.set_int(path, value)
               elif isinstance(value, float):
                   self._carb_settings.set_float(path, value)
               elif isinstance(value, str):
                   self._carb_settings.set_string(path, value)
               else:
                   self._carb_settings.set(path, value)
       except (ImportError, AttributeError):
           self._use_carb = False

   Without this, the fix is only half-complete: SettingsManager.get() will work correctly (since it always uses the dictionary), but external consumers of carb.settings (Omniverse extensions, Isaac Sim internals) that were relying on the SettingsManager to propagate values into carb will silently get stale/missing data after the app starts.

_{Last reviewed commit: b09af6a}

[greptile-apps]

get() silently diverges from carb.settings in Omniverse mode

The get() method now unconditionally reads only from _standalone_settings and never consults carb.settings. This means that if any external code (Omniverse extensions, carb-native APIs) writes a value directly to carb.settings under an isaaclab path without going through SettingsManager.set(), SettingsManager.get() will return stale or default data.

The docstring should at minimum document this behavioral change so callers are aware that in Omniverse mode the returned value may differ from what carb.settings.get() would return for the same path. Without documentation, this silent divergence can be a hard-to-debug surprise.

[AntoineRichard]

@gavrielstate is this still needed? Or should we close it?

[AntoineRichard]

Hi @gavrielstate — thanks for putting this one up! 🙏

We're doing a cleanup pass over the Isaac Lab PR backlog, which had grown past 400 open pull requests, and we're closing out the ones that have gone quiet so the queue is reviewable again.

Why this PR is being closed: Here is exactly what we found on this PR when we reviewed the backlog:

Opened	2026-03-06 (about 6 months ago)
Last commit on the branch	2026-03-06
Last activity from the author	about 6 months ago
Target branch	develop
Review status	Never reviewed by a maintainer — nobody on the team got to it. Sorry about that.
Merge status	Unknown
Size	1 commit(s), 1 file(s) changed, +8 / -11

It was picked up by the sweep because it has been open for about 6 months. It was then put in the "close" bucket because the author has been silent for about 6 months — which is the signal we used to tell apart pull requests that are still being worked on from ones that have genuinely been set aside.

We deliberately did not close pull requests that were approved and ready to land, or that were small and clearly still fixing a live bug — there were 27 of those, and we are merging them rather than closing them.

No judgement on the change itself — this is purely backlog hygiene.

If this is still wanted, please reopen it or re-submit against develop. 💚

---

🤖 This comment was drafted with AI assistance as part of a maintainer-led sweep of the Isaac Lab pull request backlog. A maintainer is behind this cleanup — but if this closure looks wrong, it may well be, so please push back and we'll take another look.