Fixes behavior of shutdown app on stop in SimulationContext (#254)

This MR fixes the behavior of
`omni.isaac.orbit.sim.SimulationCfg.shutdown_app_on_stop` to not close
the app when the flag is False. Earlier, it would throw an error
(invalid physics handles) when the stop button was pressed in the GUI
and the flag was False.

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

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./orbit.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there

source/extensions/omni.isaac.orbit/config/extension.toml

 [package]
 
 # Note: Semantic Versioning is used: https://semver.org/
-version = "0.9.49"
+version = "0.9.50"
 
 # Description
 title = "ORBIT framework for Robot Learning"

source/extensions/omni.isaac.orbit/docs/CHANGELOG.rst

 Changelog
 ---------
 
+0.9.50 (2023-11-28)
+~~~~~~~~~~~~~~~~~~~
+
+Added
+^^^^^
+
+* Hid the ``STOP`` button in the UI when running standalone Python scripts. This is to prevent
+  users from accidentally clicking the button and stopping the simulation. They should only be able to
+  play and pause the simulation from the UI.
+
+Removed
+^^^^^^^
+
+* Removed :attr:`omni.isaac.orbit.sim.SimulationCfg.shutdown_app_on_stop`. The simulation is always rendering
+  if it is stopped from the UI. The user needs to close the window or press ``Ctrl+C`` to close the simulation.
+
+
 0.9.49 (2023-11-27)
 ~~~~~~~~~~~~~~~~~~~
 

source/extensions/omni.isaac.orbit/omni/isaac/orbit/app.py

@@ -561,6 +561,8 @@ class AppLauncher:
         # add orbit modules back to sys.modules
         for key, value in hacked_modules.items():
             sys.modules[key] = value
+        # hide the stop button in the toolbar
+        self._hide_stop_button()
 
     def _load_extensions(self):
         """Load correct extensions based on AppLauncher's resolved config member variables."""
@@ -644,3 +646,15 @@ class AppLauncher:
                 "/persistent/isaac/asset_root/nvidia",
                 "http://omniverse-content-production.s3-us-west-2.amazonaws.com/Assets",
             )
+
+    def _hide_stop_button(self):
+        """Hide the stop button in the toolbar."""
+        import omni.kit.widget.toolbar
+
+        # grey out the stop button because we don't want to stop the simulation manually in standalone mode
+        toolbar = omni.kit.widget.toolbar.get_instance()
+        play_button_group = toolbar._builtin_tools._play_button_group  # type: ignore
+        if play_button_group is not None:
+            play_button_group._stop_button.visible = False  # type: ignore
+            play_button_group._stop_button.enabled = False  # type: ignore
+            play_button_group._stop_button = None  # type: ignore

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/simulation_cfg.py

@@ -256,13 +256,3 @@ class SimulationCfg:
 
     The material is created at the path: ``{physics_prim_path}/defaultMaterial``.
     """
-
-    shutdown_app_on_stop: bool = True
-    """Enable/disable shutting down the application when the simulation is stopped. Default is True.
-
-    This flag is only used when running the simulation as a standalone application.
-
-    .. note::
-        When the simulation is stopped, the physics handles become invalidated. Thus, in the simplest case,
-        it is better to shutdown the application.
-    """

source/extensions/omni.isaac.orbit/omni/isaac/orbit/sim/simulation_context.py

@@ -193,6 +193,17 @@ class SimulationContext(_SimulationContext):
         # note: we do it once here because it reads the VERSION file from disk and is not expected to change.
         self._isaacsim_version = get_version()
 
+        # add callback to deal the simulation app when simulation is stopped.
+        # this is needed because physics views go invalid once we stop the simulation
+        if not builtins.ISAAC_LAUNCHED_FROM_TERMINAL:
+            timeline_event_stream = omni.timeline.get_timeline_interface().get_timeline_event_stream()
+            self._app_control_on_stop_handle = timeline_event_stream.create_subscription_to_pop_by_type(
+                int(omni.timeline.TimelineEventType.STOP),
+                lambda *args, obj=weakref.proxy(self): obj._app_control_on_stop_callback(*args),
+                order=10,
+            )
+        else:
+            self._app_control_on_stop_handle = None
         # flatten out the simulation dictionary
         sim_params = self.cfg.to_dict()
         if sim_params is not None:
@@ -336,22 +347,6 @@ class SimulationContext(_SimulationContext):
     Operations - Override (standalone)
     """
 
-    def reset(self, soft: bool = False):
-        # need to load all "physics" information from the USD file
-        # FIXME: Remove this for Isaac Sim 2023.1 release if it will be fixed in the core.
-        if not soft:
-            omni.physx.acquire_physx_interface().force_load_physics_from_usd()
-        # play the simulation
-        super().reset(soft=soft)
-        # add callback to shutdown the simulation when it is stopped.
-        # we do this because physics views go invalid once we stop the simulation. So there is
-        # no point in keeping the simulation running.
-        if self.cfg.shutdown_app_on_stop and not self.timeline_callback_exists("shutdown_app_on_stop"):
-            self.add_timeline_callback(
-                "shutdown_app_on_stop",
-                lambda *args, obj=weakref.proxy(self): obj._shutdown_app_on_stop_callback(*args),
-            )
-
     def step(self, render: bool = True):
         """Steps the physics simulation with the pre-defined time-step.
 
@@ -431,7 +426,7 @@ class SimulationContext(_SimulationContext):
         await super().reset_async(soft=soft)
 
     """
-    Initialization - Override.
+    Initialization/Destruction - Override.
     """
 
     def _init_stage(self, *args, **kwargs) -> Usd.Stage:
@@ -452,6 +447,16 @@ class SimulationContext(_SimulationContext):
         # return the stage
         return self.stage
 
+    @classmethod
+    def clear_instance(cls):
+        # clear the callback
+        if cls._instance is not None:
+            if cls._instance._app_control_on_stop_handle is not None:
+                cls._instance._app_control_on_stop_handle.unsubscribe()
+                cls._instance._app_control_on_stop_handle = None
+        # call parent to clear the instance
+        super().clear_instance()
+
     """
     Helper Functions
     """
@@ -526,15 +531,32 @@ class SimulationContext(_SimulationContext):
     Callbacks.
     """
 
-    def _shutdown_app_on_stop_callback(self, event: carb.events.IEvent):
-        """Callback to shutdown the app when the simulation is stopped.
+    def _app_control_on_stop_callback(self, event: carb.events.IEvent):
+        """Callback to deal with the app when the simulation is stopped.
+
+        Once the simulation is stopped, the physics handles go invalid. After that, it is not possible to
+        resume the simulation from the last state. This leaves the app in an inconsistent state, where
+        two possible actions can be taken:
+
+        1. **Keep the app rendering**: In this case, the simulation is kept running and the app is not shutdown.
+           However, the physics is not updated and the script cannot be resumed from the last state. The
+           user has to manually close the app to stop the simulation.
+        2. **Shutdown the app**: This is the default behavior. In this case, the app is shutdown and
+           the simulation is stopped.
 
-        This is used only when running the simulation in a standalone python script. This is because
-        the physics views go invalid once we stop the simulation. So there is no point in keeping the
-        simulation running.
+        Note:
+            This callback is used only when running the simulation in a standalone python script. In an extension,
+            it is expected that the user handles the extension shutdown.
         """
         # check if the simulation is stopped
         if event.type == int(omni.timeline.TimelineEventType.STOP):
+            # keep running the simulator when configured to not shutdown the app
+            self.app.print_and_log(
+                "Simulation is stopped. The app will keep running with physics disabled."
+                " Press Ctrl+C or close the window to exit the app."
+            )
+            while self.app.is_running():
+                self.render()
             # make sure that any replicator workflows finish rendering/writing
             if not builtins.ISAAC_LAUNCHED_FROM_TERMINAL:
                 try:
@@ -559,5 +581,3 @@ class SimulationContext(_SimulationContext):
             self.app.shutdown()
             # disabled on linux to avoid a crash
             carb.get_framework().unload_all_plugins()
-            # exit the application
-            print("Exiting the application complete.")

source/extensions/omni.isaac.orbit/test/assets/test_articulation.py

@@ -43,7 +43,7 @@ class TestArticulation(unittest.TestCase):
         # Simulation time-step
         self.dt = 0.005
         # Load kit helper
-        sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0", shutdown_app_on_stop=False)
+        sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0")
         self.sim = sim_utils.SimulationContext(sim_cfg)
 
     def tearDown(self):

source/extensions/omni.isaac.orbit/test/assets/test_rigid_object.py

@@ -42,7 +42,7 @@ class TestRigidObject(unittest.TestCase):
         # Simulation time-step
         self.dt = 0.01
         # Load kit helper
-        sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0", shutdown_app_on_stop=False)
+        sim_cfg = sim_utils.SimulationCfg(dt=self.dt, device="cuda:0")
         self.sim = sim_utils.SimulationContext(sim_cfg)
 
     def tearDown(self):

source/extensions/omni.isaac.orbit/test/controllers/test_differential_ik.py

@@ -40,7 +40,7 @@ class TestDifferentialIKController(unittest.TestCase):
         # Constants
         self.num_envs = 128
         # Load kit helper
-        sim_cfg = sim_utils.SimulationCfg(dt=0.01, shutdown_app_on_stop=False)
+        sim_cfg = sim_utils.SimulationCfg(dt=0.01)
         self.sim = sim_utils.SimulationContext(sim_cfg)
 
         # Create a ground plane

source/extensions/omni.isaac.orbit_tasks/test/test_environments.py

@@ -62,8 +62,6 @@ class TestEnvironments(unittest.TestCase):
             omni.usd.get_context().new_stage()
             # parse configuration
             env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
-            # note: we don't want to shutdown the app on stop during the tests since we reload the stage
-            env_cfg.sim.shutdown_app_on_stop = False
 
             # create environment
             env: RLTaskEnv = gym.make(task_name, cfg=env_cfg)

source/extensions/omni.isaac.orbit_tasks/test/test_record_video.py

@@ -69,8 +69,6 @@ class TestRecordVideoWrapper(unittest.TestCase):
 
             # parse configuration
             env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
-            # note: we don't want to shutdown the app on stop during the tests since we reload the stage
-            env_cfg.sim.shutdown_app_on_stop = False
 
             # create environment
             env: RLTaskEnv = gym.make(task_name, cfg=env_cfg, render_mode="rgb_array")

source/extensions/omni.isaac.orbit_tasks/test/wrappers/test_rl_games_wrapper.py

@@ -65,8 +65,6 @@ class TestRlGamesVecEnvWrapper(unittest.TestCase):
             omni.usd.get_context().new_stage()
             # parse configuration
             env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
-            # note: we don't want to shutdown the app on stop during the tests since we reload the stage
-            env_cfg.sim.shutdown_app_on_stop = False
 
             # create environment
             env = gym.make(task_name, cfg=env_cfg)

source/extensions/omni.isaac.orbit_tasks/test/wrappers/test_rsl_rl_wrapper.py

@@ -65,8 +65,6 @@ class TestRslRlVecEnvWrapper(unittest.TestCase):
             omni.usd.get_context().new_stage()
             # parse configuration
             env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
-            # note: we don't want to shutdown the app on stop during the tests since we reload the stage
-            env_cfg.sim.shutdown_app_on_stop = False
 
             # create environment
             env = gym.make(task_name, cfg=env_cfg)

source/extensions/omni.isaac.orbit_tasks/test/wrappers/test_sb3_wrapper.py

@@ -66,8 +66,6 @@ class TestStableBaselines3VecEnvWrapper(unittest.TestCase):
             omni.usd.get_context().new_stage()
             # parse configuration
             env_cfg: RLTaskEnvCfg = parse_env_cfg(task_name, use_gpu=self.use_gpu, num_envs=self.num_envs)
-            # note: we don't want to shutdown the app on stop during the tests since we reload the stage
-            env_cfg.sim.shutdown_app_on_stop = False
 
             # create environment
             env = gym.make(task_name, cfg=env_cfg)

source/standalone/demo/play_ik_control.py

@@ -54,7 +54,7 @@ def main():
     """Main function."""
 
     # Load kit helper
-    sim_cfg = sim_utils.SimulationCfg(dt=0.01, shutdown_app_on_stop=False)
+    sim_cfg = sim_utils.SimulationCfg(dt=0.01)
     sim = sim_utils.SimulationContext(sim_cfg)
     # Set main camera
     sim.set_camera_view([2.5, 2.5, 2.5], [0.0, 0.0, 0.0])