From 4d791211eef403095e8b6ea459d9a6d6c5cf5999 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 23 May 2017 10:11:16 -0700 Subject: [PATCH 1/7] Make the changes - big thanks to Andrew! --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 9 ++++++--- libraries/entities/src/EntityItemProperties.cpp | 7 ++++++- libraries/entities/src/EntityItemProperties.h | 1 + libraries/entities/src/EntityScriptingInterface.cpp | 10 ++++++---- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 09308baabb..d62181c651 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -1091,15 +1091,18 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons // trigger scripted collision sounds and events for locally owned objects EntityItemPointer entityA = entityTree->findEntityByEntityItemID(idA); - if ((bool)entityA && myNodeID == entityA->getSimulatorID()) { + EntityItemPointer entityB = entityTree->findEntityByEntityItemID(idB); + QUuid entityASimulatorID = entityA->getSimulatorID(); + QUuid entityBSimulatorID = entityB->getSimulatorID(); + if ((bool)entityA && (myNodeID == entityASimulatorID || ((bool)entityB && myNodeID == entityBSimulatorID && entityASimulatorID.isNull()))) { playEntityCollisionSound(entityA, collision); emit collisionWithEntity(idA, idB, collision); if (_entitiesScriptEngine) { _entitiesScriptEngine->callEntityScriptMethod(idA, "collisionWithEntity", idB, collision); } } - EntityItemPointer entityB = entityTree->findEntityByEntityItemID(idB); - if ((bool)entityB && myNodeID == entityB->getSimulatorID()) { + + if ((bool)entityB && (myNodeID == entityBSimulatorID || ((bool)entityA && myNodeID == entityASimulatorID && entityBSimulatorID.isNull()))) { playEntityCollisionSound(entityB, collision); // since we're swapping A and B we need to send the inverted collision Collision invertedCollision(collision); diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 1ed020e592..4595ebf70d 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -1824,10 +1824,15 @@ bool EntityItemProperties::hasTerseUpdateChanges() const { return _positionChanged || _velocityChanged || _rotationChanged || _angularVelocityChanged || _accelerationChanged; } +bool EntityItemProperties::hasDynamicPhysicsChanges() const { + return _dynamicChanged || _velocityChanged || _angularVelocityChanged || _accelerationChanged; +} + bool EntityItemProperties::hasMiscPhysicsChanges() const { return _gravityChanged || _dimensionsChanged || _densityChanged || _frictionChanged || _restitutionChanged || _dampingChanged || _angularDampingChanged || _registrationPointChanged || - _compoundShapeURLChanged || _dynamicChanged || _collisionlessChanged || _collisionMaskChanged; + _compoundShapeURLChanged || _collisionlessChanged || _collisionMaskChanged || + _rotationChanged || _positionChanged; } void EntityItemProperties::clearSimulationOwner() { diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 590298e102..697c634c67 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -274,6 +274,7 @@ public: void setCreated(QDateTime& v); bool hasTerseUpdateChanges() const; + bool hasDynamicPhysicsChanges() const; bool hasMiscPhysicsChanges() const; void clearSimulationOwner(); diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index b184d648da..367343cb60 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -414,15 +414,17 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties& entityFound = true; // make sure the properties has a type, so that the encode can know which properties to include properties.setType(entity->getType()); - bool hasTerseUpdateChanges = properties.hasTerseUpdateChanges(); - bool hasPhysicsChanges = properties.hasMiscPhysicsChanges() || hasTerseUpdateChanges; - if (_bidOnSimulationOwnership && hasPhysicsChanges) { + bool hasMiscPhysicsChanges = properties.hasMiscPhysicsChanges(); + bool hasDynamicsChanges = properties.hasDynamicPhysicsChanges(); + // _bidOnSimulationOwnership is set per-instance of the scripting interface. + // It essentially corresponds to "Am I an AC or an Interface client?" - ACs will never bid. + if ((_bidOnSimulationOwnership && ((hasMiscPhysicsChanges && entity->isMoving()) || hasDynamicsChanges))) { auto nodeList = DependencyManager::get(); const QUuid myNodeID = nodeList->getSessionUUID(); if (entity->getSimulatorID() == myNodeID) { // we think we already own the simulation, so make sure to send ALL TerseUpdate properties - if (hasTerseUpdateChanges) { + if (properties.hasTerseUpdateChanges()) { entity->getAllTerseUpdateProperties(properties); } // TODO: if we knew that ONLY TerseUpdate properties have changed in properties AND the object From acd85b379f67d3ab12836cc814c9d2374a422755 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 23 May 2017 12:50:43 -0700 Subject: [PATCH 2/7] Make this PR a protocol change --- libraries/networking/src/udt/PacketHeaders.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index f88015a4e4..0dbbcd771f 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -114,7 +114,8 @@ public: EntityServerScriptLog, AdjustAvatarSorting, OctreeFileReplacement, - LAST_PACKET_TYPE = OctreeFileReplacement + SimulationBiddingChanges, + LAST_PACKET_TYPE = SimulationBiddingChanges }; }; From 0ef507c03791c399e5aebc4430275e060243b2e5 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 23 May 2017 14:41:02 -0700 Subject: [PATCH 3/7] Revert 'dynamicChanged' to be a 'misc' physics property --- libraries/entities/src/EntityItemProperties.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 4595ebf70d..fa4df72387 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -1825,13 +1825,13 @@ bool EntityItemProperties::hasTerseUpdateChanges() const { } bool EntityItemProperties::hasDynamicPhysicsChanges() const { - return _dynamicChanged || _velocityChanged || _angularVelocityChanged || _accelerationChanged; + return _velocityChanged || _angularVelocityChanged || _accelerationChanged; } bool EntityItemProperties::hasMiscPhysicsChanges() const { return _gravityChanged || _dimensionsChanged || _densityChanged || _frictionChanged || _restitutionChanged || _dampingChanged || _angularDampingChanged || _registrationPointChanged || - _compoundShapeURLChanged || _collisionlessChanged || _collisionMaskChanged || + _compoundShapeURLChanged || _dynamicChanged || _collisionlessChanged || _collisionMaskChanged || _rotationChanged || _positionChanged; } From f6195cdb1eb712c61c6965abdc99c05b95146eec Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 24 May 2017 10:16:51 -0700 Subject: [PATCH 4/7] Committing checkpoint changes, then testing --- .../src/EntityTreeRenderer.cpp | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index d62181c651..7b2333ade9 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -1092,24 +1092,29 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons // trigger scripted collision sounds and events for locally owned objects EntityItemPointer entityA = entityTree->findEntityByEntityItemID(idA); EntityItemPointer entityB = entityTree->findEntityByEntityItemID(idB); - QUuid entityASimulatorID = entityA->getSimulatorID(); - QUuid entityBSimulatorID = entityB->getSimulatorID(); - if ((bool)entityA && (myNodeID == entityASimulatorID || ((bool)entityB && myNodeID == entityBSimulatorID && entityASimulatorID.isNull()))) { - playEntityCollisionSound(entityA, collision); - emit collisionWithEntity(idA, idB, collision); - if (_entitiesScriptEngine) { - _entitiesScriptEngine->callEntityScriptMethod(idA, "collisionWithEntity", idB, collision); - } - } + if ((bool)entityA && (bool)entityB) { + QUuid entityASimulatorID = entityA->getSimulatorID(); + QUuid entityBSimulatorID = entityB->getSimulatorID(); + bool entityAIsStatic = !entityA->getDynamic() && !entityA->isMoving(); + bool entityBIsStatic = !entityB->getDynamic() && !entityB->isMoving(); - if ((bool)entityB && (myNodeID == entityBSimulatorID || ((bool)entityA && myNodeID == entityASimulatorID && entityBSimulatorID.isNull()))) { - playEntityCollisionSound(entityB, collision); - // since we're swapping A and B we need to send the inverted collision - Collision invertedCollision(collision); - invertedCollision.invert(); - emit collisionWithEntity(idB, idA, invertedCollision); - if (_entitiesScriptEngine) { - _entitiesScriptEngine->callEntityScriptMethod(idB, "collisionWithEntity", idA, invertedCollision); + if (myNodeID == entityASimulatorID || (myNodeID == entityBSimulatorID && entityAIsStatic)) { + playEntityCollisionSound(entityA, collision); + emit collisionWithEntity(idA, idB, collision); + if (_entitiesScriptEngine) { + _entitiesScriptEngine->callEntityScriptMethod(idA, "collisionWithEntity", idB, collision); + } + } + + if (myNodeID == entityBSimulatorID || (myNodeID == entityASimulatorID && entityBIsStatic)) { + playEntityCollisionSound(entityB, collision); + // since we're swapping A and B we need to send the inverted collision + Collision invertedCollision(collision); + invertedCollision.invert(); + emit collisionWithEntity(idB, idA, invertedCollision); + if (_entitiesScriptEngine) { + _entitiesScriptEngine->callEntityScriptMethod(idB, "collisionWithEntity", idA, invertedCollision); + } } } } From be398749997c78c8acf3e4de35afe03e866244ce Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 24 May 2017 11:17:03 -0700 Subject: [PATCH 5/7] Changes after discussion with Brad --- .../src/EntityTreeRenderer.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 7b2333ade9..fccc6f512f 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -1095,10 +1095,20 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons if ((bool)entityA && (bool)entityB) { QUuid entityASimulatorID = entityA->getSimulatorID(); QUuid entityBSimulatorID = entityB->getSimulatorID(); - bool entityAIsStatic = !entityA->getDynamic() && !entityA->isMoving(); - bool entityBIsStatic = !entityB->getDynamic() && !entityB->isMoving(); + bool entityAIsDynamic = entityA->getDynamic(); + bool entityBIsDynamic = entityB->getDynamic(); - if (myNodeID == entityASimulatorID || (myNodeID == entityBSimulatorID && entityAIsStatic)) { +#ifdef WANT_DEBUG + bool bothEntitiesStatic = !entityAIsDynamic && !entityBIsDynamic; + if (bothEntitiesStatic) { + qCDebug(entities) << "A collision has occurred between two static entities!"; + qCDebug(entities) << "Entity A ID:" << entityA->getID(); + qCDebug(entities) << "Entity B ID:" << entityB->getID(); + } + assert(!bothEntitiesStatic); +#endif + + if ((myNodeID == entityASimulatorID && entityAIsDynamic) || (myNodeID == entityBSimulatorID && !entityAIsDynamic)) { playEntityCollisionSound(entityA, collision); emit collisionWithEntity(idA, idB, collision); if (_entitiesScriptEngine) { @@ -1106,7 +1116,7 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons } } - if (myNodeID == entityBSimulatorID || (myNodeID == entityASimulatorID && entityBIsStatic)) { + if ((myNodeID == entityBSimulatorID && entityBIsDynamic) || (myNodeID == entityASimulatorID && !entityBIsDynamic)) { playEntityCollisionSound(entityB, collision); // since we're swapping A and B we need to send the inverted collision Collision invertedCollision(collision); From b86b07c08f03b8c11b0f593d97a6f68d92845269 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 24 May 2017 14:15:59 -0700 Subject: [PATCH 6/7] Pull out ownership bidding changes --- libraries/entities/src/EntityItemProperties.cpp | 7 +------ libraries/entities/src/EntityItemProperties.h | 1 - libraries/entities/src/EntityScriptingInterface.cpp | 10 ++++------ libraries/networking/src/udt/PacketHeaders.h | 4 ++-- 4 files changed, 7 insertions(+), 15 deletions(-) diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index fa4df72387..1ed020e592 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -1824,15 +1824,10 @@ bool EntityItemProperties::hasTerseUpdateChanges() const { return _positionChanged || _velocityChanged || _rotationChanged || _angularVelocityChanged || _accelerationChanged; } -bool EntityItemProperties::hasDynamicPhysicsChanges() const { - return _velocityChanged || _angularVelocityChanged || _accelerationChanged; -} - bool EntityItemProperties::hasMiscPhysicsChanges() const { return _gravityChanged || _dimensionsChanged || _densityChanged || _frictionChanged || _restitutionChanged || _dampingChanged || _angularDampingChanged || _registrationPointChanged || - _compoundShapeURLChanged || _dynamicChanged || _collisionlessChanged || _collisionMaskChanged || - _rotationChanged || _positionChanged; + _compoundShapeURLChanged || _dynamicChanged || _collisionlessChanged || _collisionMaskChanged; } void EntityItemProperties::clearSimulationOwner() { diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 697c634c67..590298e102 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -274,7 +274,6 @@ public: void setCreated(QDateTime& v); bool hasTerseUpdateChanges() const; - bool hasDynamicPhysicsChanges() const; bool hasMiscPhysicsChanges() const; void clearSimulationOwner(); diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 367343cb60..b184d648da 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -414,17 +414,15 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties& entityFound = true; // make sure the properties has a type, so that the encode can know which properties to include properties.setType(entity->getType()); - bool hasMiscPhysicsChanges = properties.hasMiscPhysicsChanges(); - bool hasDynamicsChanges = properties.hasDynamicPhysicsChanges(); - // _bidOnSimulationOwnership is set per-instance of the scripting interface. - // It essentially corresponds to "Am I an AC or an Interface client?" - ACs will never bid. - if ((_bidOnSimulationOwnership && ((hasMiscPhysicsChanges && entity->isMoving()) || hasDynamicsChanges))) { + bool hasTerseUpdateChanges = properties.hasTerseUpdateChanges(); + bool hasPhysicsChanges = properties.hasMiscPhysicsChanges() || hasTerseUpdateChanges; + if (_bidOnSimulationOwnership && hasPhysicsChanges) { auto nodeList = DependencyManager::get(); const QUuid myNodeID = nodeList->getSessionUUID(); if (entity->getSimulatorID() == myNodeID) { // we think we already own the simulation, so make sure to send ALL TerseUpdate properties - if (properties.hasTerseUpdateChanges()) { + if (hasTerseUpdateChanges) { entity->getAllTerseUpdateProperties(properties); } // TODO: if we knew that ONLY TerseUpdate properties have changed in properties AND the object diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 0dbbcd771f..2cc3a2c42e 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -114,8 +114,8 @@ public: EntityServerScriptLog, AdjustAvatarSorting, OctreeFileReplacement, - SimulationBiddingChanges, - LAST_PACKET_TYPE = SimulationBiddingChanges + CollisionEventChanges, + LAST_PACKET_TYPE = CollisionEventChanges }; }; From f2fab5718735e5ffce04dc8fca8a9cf6612514da Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Thu, 25 May 2017 09:28:11 -0700 Subject: [PATCH 7/7] Update conditional to handle 'other unowned' case --- libraries/entities-renderer/src/EntityTreeRenderer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index fccc6f512f..e029ca6ada 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -1108,7 +1108,7 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons assert(!bothEntitiesStatic); #endif - if ((myNodeID == entityASimulatorID && entityAIsDynamic) || (myNodeID == entityBSimulatorID && !entityAIsDynamic)) { + if ((myNodeID == entityASimulatorID && entityAIsDynamic) || (myNodeID == entityBSimulatorID && (!entityAIsDynamic || entityASimulatorID.isNull()))) { playEntityCollisionSound(entityA, collision); emit collisionWithEntity(idA, idB, collision); if (_entitiesScriptEngine) { @@ -1116,7 +1116,7 @@ void EntityTreeRenderer::entityCollisionWithEntity(const EntityItemID& idA, cons } } - if ((myNodeID == entityBSimulatorID && entityBIsDynamic) || (myNodeID == entityASimulatorID && !entityBIsDynamic)) { + if ((myNodeID == entityBSimulatorID && entityBIsDynamic) || (myNodeID == entityASimulatorID && (!entityBIsDynamic || entityBSimulatorID.isNull()))) { playEntityCollisionSound(entityB, collision); // since we're swapping A and B we need to send the inverted collision Collision invertedCollision(collision);