--[Bugfix] - Scene Reset Redundancy (#2470)

* --modify reset() to not perform redundant object re-placement
After successful scene creation, reset is called, but the objects have all already been placed in their initial positions.

* --fix bindings
Don't expose the boolean in reset to python, should only be consumed from Simulator::reconfigure

Author: jturner65

src/esp/bindings/SimBindings.cpp

@@ -164,7 +164,7 @@ void initSimBindings(py::module& m) {
           R"(Use gfx_replay_manager for replay recording and playback.)")
       .def("seed", &Simulator::seed, "new_seed"_a)
       .def("reconfigure", &Simulator::reconfigure, "configuration"_a)
-      .def("reset", &Simulator::reset)
+      .def("reset", [](Simulator& self) { self.reset(false); })
       .def(
           "close", &Simulator::close, "destroy"_a = true,
           R"(Free all loaded assets and GPU contexts. Use destroy=true except where noted in tutorials/async_rendering.py.)")

src/esp/physics/PhysicsManager.h

@@ -264,16 +264,24 @@ class PhysicsManager : public std::enable_shared_from_this<PhysicsManager> {
 
   /**
    * @brief Reset the simulation and physical world.
-   * Sets the @ref worldTime_ to 0.0, changes the physical state of all objects back to their initial states. Only changes motion_type when scene_instance specified a motion type.
-   */
-  virtual void reset() {
+   * Sets the @ref worldTime_ to 0.0, changes the physical
+   * state of all objects back to their initial states.
+   * Only changes motion_type when scene_instance specified a motion type.
+   * @param calledAfterSceneCreate If this is true, this is being called
+   * directly after a new scene was created and all the objects were placed
+   * appropriately, so bypass object placement reset code.
+   */
+  virtual void reset(bool calledAfterSceneCreate) {
     // reset object states from initial values (e.g. from scene instance)
     worldTime_ = 0.0;
-    for (const auto& bro : existingObjects_) {
-      bro.second->resetStateFromSceneInstanceAttr();
-    }
-    for (const auto& bao : existingArticulatedObjects_) {
-      bao.second->resetStateFromSceneInstanceAttr();
+    if (!calledAfterSceneCreate) {
+      // No need to re-place objects after scene creation
+      for (const auto& bro : existingObjects_) {
+        bro.second->resetStateFromSceneInstanceAttr();
+      }
+      for (const auto& bao : existingArticulatedObjects_) {
+        bao.second->resetStateFromSceneInstanceAttr();
+      }
     }
   }
 

src/esp/sim/Simulator.cpp

@@ -426,9 +426,9 @@ bool Simulator::createSceneInstance(const std::string& activeSceneName) {
       success = instanceArticulatedObjectsForSceneAttributes(
           curSceneInstanceAttributes_);
       if (success) {
-        // TODO : reset may eventually have all the scene instantiation code so
-        // that scenes can be reset
-        reset();
+        // Pass true so that the object/AO placement code is bypassed in
+        // physicsManager_->reset
+        reset(true);
       }
     }
   }
@@ -670,20 +670,20 @@ bool Simulator::instanceArticulatedObjectsForSceneAttributes(
   return true;
 }  // Simulator::instanceArticulatedObjectsForSceneAttributes
 
-void Simulator::reset() {
+void Simulator::reset(bool calledAfterSceneCreate) {
   if (physicsManager_ != nullptr) {
     // Note: resets time to 0 and all existing objects set back to initial
     // states. Does not add back deleted objects or delete added objects. Does
     // not break ManagedObject pointers.
-    physicsManager_->reset();
+    physicsManager_->reset(calledAfterSceneCreate);
   }
 
   for (auto& agent : agents_) {
     agent->reset();
   }
   getActiveSceneGraph().getRootNode().computeCumulativeBB();
   resourceManager_->setLightSetup(gfx::getDefaultLights());
-}  // Simulator::reset()
+}  // Simulator::reset
 
 metadata::attributes::SceneInstanceAttributes::ptr
 Simulator::buildCurrentStateSceneAttributes() const {

src/esp/sim/Simulator.h

@@ -75,8 +75,11 @@ class Simulator {
    * Does not invalidate existing ManagedObject wrappers.
    * Does not add or remove object instances.
    * Only changes motion_type when scene_instance specified a motion type.
+   * @param calledAfterSceneCreate Whether this reset is being called after a
+   * new scene has been created in reconfigure. If so we con't want to
+   * redundantly re-place the newly-placed object positions.
    */
-  void reset();
+  void reset(bool calledAfterSceneCreate = false);
 
   void seed(uint32_t newSeed);
 

src/tests/PhysicsTest.cpp

@@ -224,7 +224,7 @@ void PhysicsTest::testJoinCompound() {
         objectTemplate->setJoinCollisionMeshes(true);
       }
       objectAttributesManager->registerObject(objectTemplate);
-      physicsManager_->reset();
+      physicsManager_->reset(false);
 
       // add and simulate objects
       int num_objects = 7;
@@ -302,7 +302,7 @@ void PhysicsTest::testCollisionBoundingBox() {
         objectTemplate->setBoundingBoxCollisions(true);
       }
       objectAttributesManager->registerObject(objectTemplate);
-      physicsManager_->reset();
+      physicsManager_->reset(false);
 
       auto objectWrapper = makeObjectGetWrapper(
           objectFile, &sceneManager_->getSceneGraph(sceneID_).getDrawables());
@@ -612,7 +612,7 @@ void PhysicsTest::testMotionTypes() {
 
       // reset the scene
       rigidObjectManager_->removeAllObjects();
-      physicsManager_->reset();  // time=0
+      physicsManager_->reset(false);  // time=0
     }
   }
 }  // PhysicsTest::testMotionTypes