Skip to content

Fix visualizer initialization #4844

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.

Sign up for GitHub

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

Closed
gavrielstate wants to merge 1 commit into from
Closed

Fix visualizer initialization #4844

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions source/isaaclab/isaaclab/app/settings_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,18 @@ def initialize_carb_settings(self):
def set(self, path: str, value: Any) -> None:
"""Set a setting value at the given path.

Always stores in our dictionary for consistent behavior. Also syncs to carb.settings
when available for compatibility with Omniverse.

Args:
path: The settings path (e.g., "/isaaclab/render/offscreen")
value: The value to set
"""
# Always store in our dictionary (source of truth)
self._standalone_settings[path] = value

# Also sync to carb.settings when available
if self._use_carb and self._carb_settings is not None:
# Delegate to carb.settings
if isinstance(value, bool):
self._carb_settings.set_bool(path, value)
elif isinstance(value, int):
Expand All @@ -110,9 +116,6 @@ def set(self, path: str, value: Any) -> None:
else:
# For other types, try generic set
self._carb_settings.set(path, value)
else:
# Standalone mode - use dictionary
self._standalone_settings[path] = value

def get(self, path: str, default: Any = None) -> Any:
"""Get a setting value at the given path.
Expand All @@ -124,13 +127,7 @@ def get(self, path: str, default: Any = None) -> Any:
Returns:
The value at the path, or default if not found
"""
if self._use_carb and self._carb_settings is not None:
# Delegate to carb.settings
value = self._carb_settings.get(path)
return value if value is not None else default
else:
# Standalone mode - use dictionary
return self._standalone_settings.get(path, default)
return self._standalone_settings.get(path, default)
Comment on lines 120 to +130

@greptile-apps greptile-apps Bot Mar 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


def set_bool(self, path: str, value: bool) -> None:
"""Set a boolean setting value.
Expand Down
Loading