yield ownership outside workload R1

libraries/entities/src/EntityTree.cpp

@@ -360,7 +360,12 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti
                     // the sender is trying to take or continue ownership
                     if (entity->getSimulatorID().isNull()) {
                         // the sender is taking ownership
-                        properties.promoteSimulationPriority(RECRUIT_SIMULATION_PRIORITY);
+                        SimulationOwner owner = properties.getSimulationOwner();
+                        if (owner.getPriority() == VOLUNTEER_SIMULATION_PRIORITY) {
+                            // the entity-server always promotes VOLUNTEER to RECRUIT to avoid ownership thrash
+                            // when dynamic objects first activate and multiple participants bid simultaneously
+                            properties.promoteSimulationPriority(RECRUIT_SIMULATION_PRIORITY);
+                        }
                         simulationBlocked = false;
                     } else if (entity->getSimulatorID() == senderID) {
                         // the sender is asserting ownership, maybe changing priority
@@ -392,6 +397,7 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti
             }
             if (simulationBlocked) {
                 // squash ownership and physics-related changes.
+                // TODO? replace these eight calls with just one?
                 properties.setSimulationOwnerChanged(false);
                 properties.setPositionChanged(false);
                 properties.setRotationChanged(false);
@@ -1795,7 +1801,7 @@ void EntityTree::addToNeedsParentFixupList(EntityItemPointer entity) {
 
 void EntityTree::update(bool simulate) {
     PROFILE_RANGE(simulation_physics, "UpdateTree");
-    PerformanceTimer perfTimer("UpdateTreen");
+    PerformanceTimer perfTimer("updateTree");
     withWriteLock([&] {
         fixupNeedsParentFixups();
         if (simulate && _simulation) {

libraries/entities/src/SimulationOwner.h

@@ -76,11 +76,11 @@
 //    (12) When entityA, locally owned at priority = N, collides with an unowned entityB the owner will
 //         also bid for entityB at priority = N-1 (or VOLUNTEER, whichever is larger).
 //
-//    (13) When an entity comes to rest and is deactivated in the physics simulation the owner will
+//    (13) When an entity comes to rest and is deactivated in the physics simulation the owner will send
-//         send an update to: clear their ownerhsip, set priority to zero, and set the object's
+//         an update to: clear their ownerhsip, set priority to zero, and set the object's velocities to
-//         velocities to be zero.  As per a normal bid, the owner does NOT assume that its ownership
+//         zero.  As per a normal bid, the owner does NOT assume that its ownership has been cleared until
-//         has been cleared until it hears from the entity-server.  This, if the packet is lost the
+//         it hears from the entity-server.  Thus, if the packet is lost the owner will re-send after some
-//         owner will re-send after some period.
+//         period.
 //
 //    (14) When an entity's ownership priority drops below VOLUNTEER other participants may bid for it
 //         immediately at priority = VOLUNTEER.

libraries/physics/src/EntityMotionState.cpp

@@ -139,6 +139,7 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) {
         if (_entity->getSimulatorID().isNull()) {
             // simulation ownership has been removed
             if (glm::length2(_entity->getWorldVelocity()) == 0.0f) {
+            // TODO: also check angularVelocity
                 // this object is coming to rest --> clear the ACTIVATION flag and _bidPriority
                 flags &= ~Simulation::DIRTY_PHYSICS_ACTIVATION;
                 _body->setActivationState(WANTS_DEACTIVATION);
@@ -148,19 +149,21 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) {
             } else {
                 // disowned object is still moving --> start timer for ownership bid
                 // TODO? put a delay in here proportional to distance from object?
-                upgradeBidPriority(VOLUNTEER_SIMULATION_PRIORITY);
+                computeNewBidPriority(VOLUNTEER_SIMULATION_PRIORITY);
                 _nextBidExpiry = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS;
             }
             _loopsWithoutOwner = 0;
             _numInactiveUpdates = 0;
         } else if (isLocallyOwned()) {
-            // we just inherited ownership, make sure our desired priority matches what we have
+            // we just received ownership, or the priority of our existing ownership has changed
-            upgradeBidPriority(_entity->getSimulationPriority());
+            computeNewBidPriority(_entity->getSimulationPriority());
         } else {
-            // the entity is owned by someone else, so we clear _bidPriority here
+            // the entity is owned by someone else
-            // but _bidPriority may be updated to non-zero value if this object interacts with locally owned simulation
+            // we are always willing to volunteer for nearby objects
-            // in which case we may try to bid again
+            // otherwise we zero _bidPriority here
-            _bidPriority = 0;
+            // (it's possible _bidPriority will be promoted in subsequent frames
+            // when local scripts or owned simulation interact with it)
+            _bidPriority = (_region == workload::Region::R1) ? VOLUNTEER_SIMULATION_PRIORITY : 0;
             _nextBidExpiry = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS;
             _numInactiveUpdates = 0;
         }
@@ -170,7 +173,7 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) {
         // (1) we own it but may need to change the priority OR...
         // (2) we don't own it but should bid (because a local script has been changing physics properties)
         uint8_t newPriority = isLocallyOwned() ? _entity->getSimulationOwner().getPriority() : _entity->getSimulationOwner().getPendingPriority();
-        upgradeBidPriority(newPriority);
+        computeNewBidPriority(newPriority);
 
         // reset bid expiry so that we bid ASAP
         _nextBidExpiry = 0;
@@ -299,7 +302,7 @@ void EntityMotionState::setWorldTransform(const btTransform& worldTrans) {
     if (_entity->getSimulatorID().isNull()) {
         _loopsWithoutOwner++;
         if (_loopsWithoutOwner > LOOPS_FOR_SIMULATION_ORPHAN && usecTimestampNow() > _nextBidExpiry) {
-            upgradeBidPriority(VOLUNTEER_SIMULATION_PRIORITY);
+            computeNewBidPriority(VOLUNTEER_SIMULATION_PRIORITY);
         }
     }
 }
@@ -326,21 +329,26 @@ void EntityMotionState::setShape(const btCollisionShape* shape) {
 }
 
 bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) {
-    // NOTE: we only get here if we think we own the simulation
+    // NOTE: this method is only ever called when the entity simulation is locally owned
     DETAILED_PROFILE_RANGE(simulation_physics, "CheckOutOfSync");
 
     // Since we own the simulation: make sure _bidPriority is not less than current owned priority
-    // because: an _bidPriority of zero indicates that we should drop ownership when we have it.
+    // because: a _bidPriority of zero indicates that we should drop ownership in the send.
-    upgradeBidPriority(_entity->getSimulationPriority());
+    // TODO: need to be able to detect when logic dictates we *decrease* priority
+    // WIP: print info whenever _bidPriority mismatches what is known to the entity
+
+    if (_entity->dynamicDataNeedsTransmit()) {
+        uint8_t priority = _entity->hasActions() ? SCRIPT_GRAB_SIMULATION_PRIORITY : SCRIPT_POKE_SIMULATION_PRIORITY;
+        computeNewBidPriority(priority);
+        return true;
+    }
+    computeNewBidPriority(_entity->getSimulationPriority());
 
     bool parentTransformSuccess;
     Transform localToWorld = _entity->getParentTransform(parentTransformSuccess);
     Transform worldToLocal;
-    Transform worldVelocityToLocal;
     if (parentTransformSuccess) {
         localToWorld.evalInverse(worldToLocal);
-        worldVelocityToLocal = worldToLocal;
-        worldVelocityToLocal.setTranslation(glm::vec3(0.0f));
     }
 
     int numSteps = simulationStep - _lastStep;
@@ -389,12 +397,6 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) {
         }
     }
 
-    if (_entity->dynamicDataNeedsTransmit()) {
-        uint8_t priority = _entity->hasActions() ? SCRIPT_GRAB_SIMULATION_PRIORITY : SCRIPT_POKE_SIMULATION_PRIORITY;
-        upgradeBidPriority(priority);
-        return true;
-    }
-
     // Else we measure the error between current and extrapolated transform (according to expected behavior
     // of remote EntitySimulation) and return true if the error is significant.
 
@@ -439,6 +441,7 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) {
 }
 
 bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) {
+    // NOTE: this method is only ever called when the entity simulation is locally owned
     DETAILED_PROFILE_RANGE(simulation_physics, "ShouldSend");
     // NOTE: we expect _entity and _body to be valid in this context, since shouldSendUpdate() is only called
     // after doesNotNeedToSendUpdate() returns false and that call should return 'true' if _entity or _body are NULL.
@@ -447,7 +450,7 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) {
     // this case is prevented by setting _ownershipState to UNOWNABLE in EntityMotionState::ctor
     assert(!(_entity->getClientOnly() && _entity->getOwningAvatarID() != Physics::getSessionUUID()));
 
-    if (_entity->dynamicDataNeedsTransmit() || _entity->queryAACubeNeedsUpdate()) {
+    if (_entity->dynamicDataNeedsTransmit() || (!_entity->getDynamic() && _entity->queryAACubeNeedsUpdate())) {
         return true;
     }
 
@@ -548,10 +551,10 @@ void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t s
     _lastStep = step;
     _nextBidExpiry = now + USECS_BETWEEN_OWNERSHIP_BIDS;
 
-    // finally: clear _bidPriority
+    // after sending the bid we try to clear _bidPriority
-    // which will may get promoted before next bid
+    // which might get promoted again next frame (after local script or simulation interaction)
-    // or maybe we'll win simulation ownership
+    // or we might win the bid
-    _bidPriority = 0;
+    computeNewBidPriority(0);
 }
 
 void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step) {
@@ -686,7 +689,7 @@ uint8_t EntityMotionState::getSimulationPriority() const {
 }
 
 void EntityMotionState::slaveBidPriority() {
-    upgradeBidPriority(_entity->getSimulationPriority());
+    computeNewBidPriority(_entity->getSimulationPriority());
 }
 
 // virtual
@@ -697,7 +700,7 @@ QUuid EntityMotionState::getSimulatorID() const {
 
 void EntityMotionState::bump(uint8_t priority) {
     assert(priority != 0);
-    upgradeBidPriority(glm::max(VOLUNTEER_SIMULATION_PRIORITY, --priority));
+    computeNewBidPriority(glm::max(VOLUNTEER_SIMULATION_PRIORITY, --priority));
 }
 
 void EntityMotionState::resetMeasuredBodyAcceleration() {
@@ -776,11 +779,12 @@ void EntityMotionState::computeCollisionGroupAndMask(int16_t& group, int16_t& ma
 }
 
 bool EntityMotionState::shouldSendBid() {
-    if (_bidPriority >= glm::max(_entity->getSimulationPriority(), VOLUNTEER_SIMULATION_PRIORITY)) {
+    // NOTE: this method is only ever called when the entity simulation is NOT locally owned
+    if (_bidPriority > glm::max(_entity->getSimulationPriority(), YIELD_SIMULATION_PRIORITY)) {
         return true;
     } else {
         // NOTE: this 'else' case has a side-effect: it clears _bidPriority
-        // which may be updated next simulation step (via collision or script event)
+        // which might get promoted again next frame (after local script or simulation interaction)
         _bidPriority = 0;
         return false;
     }
@@ -791,10 +795,19 @@ bool EntityMotionState::isLocallyOwned() const {
 }
 
 bool EntityMotionState::isLocallyOwnedOrShouldBe() const {
-    return (_bidPriority > VOLUNTEER_SIMULATION_PRIORITY && _bidPriority > _entity->getSimulationPriority()) ||
+    // this method could also be called "shouldGenerateCollisionEventForLocalScripts()"
+    // because that is the only reason it's used
+    return (_bidPriority > VOLUNTEER_SIMULATION_PRIORITY && _bidPriority >= _entity->getSimulationPriority()) ||
         _entity->getSimulatorID() == Physics::getSessionUUID();
 }
 
+void EntityMotionState::setRegion(uint8_t region) {
+    _region = region;
+    if (_region == workload::Region::R1 && _bidPriority < VOLUNTEER_SIMULATION_PRIORITY && !isLocallyOwned()) {
+        _bidPriority = VOLUNTEER_SIMULATION_PRIORITY;
+    }
+}
+
 void EntityMotionState::initForBid() {
     assert(_ownershipState != EntityMotionState::OwnershipState::Unownable);
     _ownershipState = EntityMotionState::OwnershipState::PendingBid;
@@ -805,8 +818,13 @@ void EntityMotionState::initForOwned() {
     _ownershipState = EntityMotionState::OwnershipState::LocallyOwned;
 }
 
-void EntityMotionState::upgradeBidPriority(uint8_t priority) {
+void EntityMotionState::computeNewBidPriority(uint8_t newPriority) {
-    _bidPriority = glm::max<uint8_t>(_bidPriority, priority);
+    uint8_t oldPriority = _bidPriority;
+    if (_region == workload::Region::R1) {
+        _bidPriority = glm::max<uint8_t>(VOLUNTEER_SIMULATION_PRIORITY, glm::max<uint8_t>(_bidPriority, newPriority));
+    } else {
+        _bidPriority = glm::min<uint8_t>(YIELD_SIMULATION_PRIORITY, newPriority);
+    }
 }
 
 void EntityMotionState::clearObjectVelocities() const {

libraries/physics/src/EntityMotionState.h

@@ -14,6 +14,7 @@
 
 #include <EntityTypes.h>
 #include <AACube.h>
+#include <workload/Region.h>
 
 #include "ObjectMotionState.h"
 
@@ -93,6 +94,8 @@ public:
     friend class PhysicalEntitySimulation;
     OwnershipState getOwnershipState() const { return _ownershipState; }
 
+    void setRegion(uint8_t region);
+
 protected:
     void updateSendVelocities();
     uint64_t getNextBidExpiry() const { return _nextBidExpiry; }
@@ -102,11 +105,10 @@ protected:
     void updateServerPhysicsVariables();
     bool remoteSimulationOutOfSync(uint32_t simulationStep);
 
-    // changes _bidPriority only if priority is larger
+    // computes _bidPriority using newPriority and special case rules
-    void upgradeBidPriority(uint8_t priority);
+    void computeNewBidPriority(uint8_t newPriority);
 
-    // upgradeBidPriority to value stored in _entity
+    void slaveBidPriority(); // computeNewBidPriority() with value stored in _entity
-    void slaveBidPriority();
 
     void clearObjectVelocities() const;
 
@@ -155,6 +157,7 @@ protected:
     mutable uint8_t _accelerationNearlyGravityCount;
     uint8_t _numInactiveUpdates { 1 };
     uint8_t _bidPriority { 0 };
+    uint8_t _region { workload::Region::INVALID };
     bool _serverVariablesSet { false };
 };
 

libraries/physics/src/PhysicalEntitySimulation.cpp

@@ -53,7 +53,9 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) {
     bool canBeKinematic = region <= workload::Region::R3;
     if (shouldBePhysical) {
         EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
-        if (!motionState) {
+        if (motionState) {
+            motionState->setRegion(region);
+        } else {
             _entitiesToAddToPhysics.insert(entity);
         }
     } else if (canBeKinematic && entity->isMovingRelativeToParent()) {
@@ -139,6 +141,7 @@ void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) {
         } else {
             _incomingChanges.insert(motionState);
         }
+        motionState->setRegion(region);
     } else if (shouldBePhysical) {
         // The intent is for this object to be in the PhysicsEngine, but it has no MotionState yet.
         // Perhaps it's shape has changed and it can now be added?