From 2400f5c00004f6397eeabf23b457de8ea3a416b6 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 1 Jun 2015 09:02:59 -0700 Subject: [PATCH 1/4] update ALL parameters of remote physics simulation --- libraries/physics/src/EntityMotionState.cpp | 23 +++++++-------------- libraries/physics/src/EntityMotionState.h | 2 +- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 9a24aabb34..15be1da3ed 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -49,24 +49,17 @@ EntityMotionState::~EntityMotionState() { assert(!_entity); } -void EntityMotionState::updateServerPhysicsVariables(uint32_t flags) { - 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(); - } +void EntityMotionState::updateServerPhysicsVariables() { + _serverPosition = _entity->getPosition(); + _serverRotation = _entity->getRotation(); + _serverVelocity = _entity->getVelocity(); + _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; @@ -94,7 +87,7 @@ void EntityMotionState::handleEasyChanges(uint32_t flags) { // virtual void EntityMotionState::handleHardAndEasyChanges(uint32_t flags, PhysicsEngine* engine) { - updateServerPhysicsVariables(flags); + updateServerPhysicsVariables(); ObjectMotionState::handleHardAndEasyChanges(flags, engine); } diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 65279dc01a..2be6d0490e 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/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); From fa491a5c4f013c0278d03199f5ff2339cb09919e Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 1 Jun 2015 13:59:56 -0700 Subject: [PATCH 2/4] fix theoretical crash bug in editEntity() --- .../entities/src/EntityScriptingInterface.cpp | 43 ++++++++++--------- .../entities/src/EntityScriptingInterface.h | 2 +- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index d684fbf2fc..36cc8dc07a 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -129,37 +129,38 @@ 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 (properties.getType() == EntityTypes::Unknown) { - EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID); - if (entity) { - // we need to change the outgoing properties, so we make a copy, modify, and send. - EntityItemProperties modifiedProperties = properties; - entity->setLastBroadcast(usecTimestampNow()); - modifiedProperties.setType(entity->getType()); - if (modifiedProperties.hasTerseUpdateChanges()) { - // we make a bid for (or assert) our simulation ownership - auto nodeList = DependencyManager::get(); - 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 - entity->getAllTerseUpdateProperties(modifiedProperties); + if (updatedEntity) { + _entityTree->lockForRead(); + EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID); + if (entity) { + // make sure the properties has a type, so that the encode can know which properties to include + properties.setType(entity->getType()); + if (properties.hasTerseUpdateChanges()) { + // we make a bid for (or assert) our simulation ownership + auto nodeList = DependencyManager::get(); + const QUuid myNodeID = nodeList->getSessionUUID(); + properties.setSimulatorID(myNodeID); + + if (entity->getSimulatorID() == myNodeID) { + // we think we already own the simulation, so make sure to send ALL TerseUpdate properties + entity->getAllTerseUpdateProperties(properties); + } } + entity->setLastBroadcast(usecTimestampNow()); } - queueEntityMessage(PacketTypeEntityEdit, entityID, modifiedProperties); + _entityTree->unlock(); + queueEntityMessage(PacketTypeEntityEdit, entityID, properties); return id; } + return QUuid(); } queueEntityMessage(PacketTypeEntityEdit, entityID, properties); diff --git a/libraries/entities/src/EntityScriptingInterface.h b/libraries/entities/src/EntityScriptingInterface.h index 6c2dc06579..0e2b27baad 100644 --- a/libraries/entities/src/EntityScriptingInterface.h +++ b/libraries/entities/src/EntityScriptingInterface.h @@ -78,7 +78,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); From cc9ae81c7572397b1ac817f8a5578cfa16dc8bd0 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 1 Jun 2015 14:47:14 -0700 Subject: [PATCH 3/4] group Entity property settings together --- libraries/entities/src/EntityItem.cpp | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index cc951ac16b..d5dca268be 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -943,30 +943,37 @@ 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 - SET_ENTITY_PROPERTY_FROM_PROPERTIES(dimensions, updateDimensions); // NOTE: radius is obsolete + // these affect TerseUpdate properties + 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(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); From acb3163f4306c59d5f4aba74897149cea87ce1d6 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 2 Jun 2015 17:20:41 -0700 Subject: [PATCH 4/4] add comments for future work --- libraries/entities/src/EntityScriptingInterface.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index c1c13a832c..5a897f7fe7 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -143,15 +143,22 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, EntityItemProperties proper // make sure the properties has a type, so that the encode can know which properties to include properties.setType(entity->getType()); if (properties.hasTerseUpdateChanges()) { - // we make a bid for (or assert) our simulation ownership auto nodeList = DependencyManager::get(); const QUuid myNodeID = nodeList->getSessionUUID(); - properties.setSimulatorID(myNodeID); if (entity->getSimulatorID() == myNodeID) { // we think we already own the simulation, so make sure to send ALL TerseUpdate properties 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); } entity->setLastBroadcast(usecTimestampNow()); }