Merge pull request #5027 from AndrewMeadows/isentropic

cleanup around sending all "terse update" data whenever some of it changes

libraries/entities/src/EntityItem.cpp

@@ -944,31 +944,38 @@ void EntityItem::getAllTerseUpdateProperties(EntityItemProperties& properties) c
 bool EntityItem::setProperties(const EntityItemProperties& properties) {
     bool somethingChanged = false;
 
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(position, updatePosition); // this will call recalculate collision shape if needed
+    // these affect TerseUpdate properties
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(dimensions, updateDimensions); // NOTE: radius is obsolete
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(position, updatePosition);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(rotation, updateRotation);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(density, updateDensity);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(velocity, updateVelocity);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(gravity, updateGravity);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(angularVelocity, updateAngularVelocity);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(acceleration, setAcceleration);
+
+    // these (along with "position" above) affect tree structure
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(dimensions, updateDimensions);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(registrationPoint, setRegistrationPoint);
+
+    // these (along with all properties above) affect the simulation
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(density, updateDensity);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(gravity, updateGravity);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(damping, updateDamping);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(angularDamping, updateAngularDamping);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(restitution, updateRestitution);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(friction, updateFriction);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(ignoreForCollisions, updateIgnoreForCollisions);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(collisionsWillMove, updateCollisionsWillMove);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(created, updateCreated);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(lifetime, updateLifetime);
+    SET_ENTITY_PROPERTY_FROM_PROPERTIES(simulatorID, updateSimulatorID);
+
+    // non-simulation properties below
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(script, setScript);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(collisionSoundURL, setCollisionSoundURL);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(registrationPoint, setRegistrationPoint);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(angularVelocity, updateAngularVelocity);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(angularDamping, updateAngularDamping);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(glowLevel, setGlowLevel);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(localRenderAlpha, setLocalRenderAlpha);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(visible, setVisible);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(ignoreForCollisions, updateIgnoreForCollisions);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(collisionsWillMove, updateCollisionsWillMove);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(locked, setLocked);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(userData, setUserData);
-    SET_ENTITY_PROPERTY_FROM_PROPERTIES(simulatorID, updateSimulatorID);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(marketplaceID, setMarketplaceID);
     SET_ENTITY_PROPERTY_FROM_PROPERTIES(name, setName);
 

libraries/entities/src/EntityScriptingInterface.cpp

@@ -128,37 +128,45 @@ EntityItemProperties EntityScriptingInterface::getEntityProperties(QUuid identit
     return results;
 }
 
-QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties& properties) {
+QUuid EntityScriptingInterface::editEntity(QUuid id, EntityItemProperties properties) {
     EntityItemID entityID(id);
     // If we have a local entity tree set, then also update it.
     if (_entityTree) {
         _entityTree->lockForWrite();
-        _entityTree->updateEntity(entityID, properties);
+        bool updatedEntity = _entityTree->updateEntity(entityID, properties);
         _entityTree->unlock();
-    }
 
-    // make sure the properties has a type, so that the encode can know which properties to include
+        if (updatedEntity) {
-    if (properties.getType() == EntityTypes::Unknown) {
+            _entityTree->lockForRead();
             EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID);
             if (entity) {
-            // we need to change the outgoing properties, so we make a copy, modify, and send.
+                // make sure the properties has a type, so that the encode can know which properties to include
-            EntityItemProperties modifiedProperties = properties;
+                properties.setType(entity->getType());
-            entity->setLastBroadcast(usecTimestampNow());
+                if (properties.hasTerseUpdateChanges()) {
-            modifiedProperties.setType(entity->getType());
-            if (modifiedProperties.hasTerseUpdateChanges()) {
-                // we make a bid for (or assert) our simulation ownership
                     auto nodeList = DependencyManager::get<NodeList>();
                     const QUuid myNodeID = nodeList->getSessionUUID();
-                modifiedProperties.setSimulatorID(myNodeID);
     
                     if (entity->getSimulatorID() == myNodeID) {
-                    // we think we already own simulation, so make sure we send ALL TerseUpdate properties
+                        // we think we already own the simulation, so make sure to send ALL TerseUpdate properties
-                    entity->getAllTerseUpdateProperties(modifiedProperties);
+                        entity->getAllTerseUpdateProperties(properties);
+                        // TODO: if we knew that ONLY TerseUpdate properties have changed in properties AND the object 
+                        // is dynamic AND it is active in the physics simulation then we could chose to NOT queue an update 
+                        // and instead let the physics simulation decide when to send a terse update.  This would remove
+                        // the "slide-no-rotate" glitch (and typical a double-update) that we see during the "poke rolling
+                        // balls" test.  However, even if we solve this problem we still need to provide a "slerp the visible 
+                        // proxy toward the true physical position" feature to hide the final glitches in the remote watcher's
+                        // simulation.
                     }
+                    // we make a bid for (or assert existing) simulation ownership
+                    properties.setSimulatorID(myNodeID);
                 }
-            queueEntityMessage(PacketTypeEntityEdit, entityID, modifiedProperties);
+                entity->setLastBroadcast(usecTimestampNow());
+            }
+            _entityTree->unlock();
+            queueEntityMessage(PacketTypeEntityEdit, entityID, properties);
             return id;
         }
+        return QUuid();
     }
 
     queueEntityMessage(PacketTypeEntityEdit, entityID, properties);

libraries/entities/src/EntityScriptingInterface.h

@@ -79,7 +79,7 @@ public slots:
 
     /// edits a model updating only the included properties, will return the identified EntityItemID in case of
     /// successful edit, if the input entityID is for an unknown model this function will have no effect
-    Q_INVOKABLE QUuid editEntity(QUuid entityID, const EntityItemProperties& properties);
+    Q_INVOKABLE QUuid editEntity(QUuid entityID, EntityItemProperties properties);
 
     /// deletes a model
     Q_INVOKABLE void deleteEntity(QUuid entityID);

libraries/physics/src/EntityMotionState.cpp

@@ -50,24 +50,17 @@ EntityMotionState::~EntityMotionState() {
     assert(!_entity);
 }
 
-void EntityMotionState::updateServerPhysicsVariables(uint32_t flags) {
+void EntityMotionState::updateServerPhysicsVariables() {
-    if (flags & EntityItem::DIRTY_POSITION) {
     _serverPosition = _entity->getPosition();
-    }
-    if (flags & EntityItem::DIRTY_ROTATION) {
     _serverRotation = _entity->getRotation();
-    }
-    if (flags & EntityItem::DIRTY_LINEAR_VELOCITY) {
     _serverVelocity = _entity->getVelocity();
-    }
-    if (flags & EntityItem::DIRTY_ANGULAR_VELOCITY) {
     _serverAngularVelocity = _entity->getAngularVelocity();
-    }
+    _serverAcceleration = _entity->getAcceleration();
 }
 
 // virtual
 void EntityMotionState::handleEasyChanges(uint32_t flags) {
-    updateServerPhysicsVariables(flags);
+    updateServerPhysicsVariables();
     ObjectMotionState::handleEasyChanges(flags);
     if (flags & EntityItem::DIRTY_SIMULATOR_ID) {
         _loopsWithoutOwner = 0;
@@ -95,7 +88,7 @@ void EntityMotionState::handleEasyChanges(uint32_t flags) {
 
 // virtual
 void EntityMotionState::handleHardAndEasyChanges(uint32_t flags, PhysicsEngine* engine) {
-    updateServerPhysicsVariables(flags);
+    updateServerPhysicsVariables();
     ObjectMotionState::handleHardAndEasyChanges(flags, engine);
 }
 

libraries/physics/src/EntityMotionState.h

@@ -28,7 +28,7 @@ public:
     EntityMotionState(btCollisionShape* shape, EntityItemPointer item);
     virtual ~EntityMotionState();
 
-    void updateServerPhysicsVariables(uint32_t flags);
+    void updateServerPhysicsVariables();
     virtual void handleEasyChanges(uint32_t flags);
     virtual void handleHardAndEasyChanges(uint32_t flags, PhysicsEngine* engine);