Merge pull request #7807 from AndrewMeadows/ignore-echo-of-stale-data

fix object position glitch of owned simulations on networks with long ping-time

libraries/entities/src/EntityItem.cpp

@@ -655,11 +655,32 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef
         if (wantTerseEditLogging() && _simulationOwner != newSimOwner) {
             qCDebug(entities) << "sim ownership for" << getDebugName() << "is now" << newSimOwner;
         }
-        if (_simulationOwner.set(newSimOwner)) {
+        if (weOwnSimulation) {
+            if (newSimOwner.getID().isNull() && !_simulationOwner.pendingRelease(lastEditedFromBufferAdjusted)) {
+                // entity-server is trying to clear our ownership (probably at our own request)
+                // but we actually want to own it, therefore we ignore this clear event
+                // and pretend that we own it (we assume we'll recover it soon)
+            } else if (_simulationOwner.set(newSimOwner)) {
+                _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+                somethingChanged = true;
+                // recompute weOwnSimulation for later
+                weOwnSimulation = _simulationOwner.matchesValidID(myNodeID);
+            }
+        } else if (newSimOwner.getID().isNull() && _simulationOwner.pendingTake(lastEditedFromBufferAdjusted)) {
+            // entity-server is trying to clear someone  else's ownership
+            // but we want to own it, therefore we ignore this clear event
+            // and pretend that we own it (we assume we'll get it soon)
+            weOwnSimulation = true;
+            if (!_simulationOwner.isNull()) {
+                // someone else really did own it
+                _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
+                somethingChanged = true;
+                _simulationOwner.clearCurrentOwner();
+            }
+        } else if (_simulationOwner.set(newSimOwner)) {
             _dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
             somethingChanged = true;
-            // recompute weOwnSimulation so that if this is the packet that tells use we are the owner,
-            // we ignore the physics changes from this packet.
+            // recompute weOwnSimulation for later
             weOwnSimulation = _simulationOwner.matchesValidID(myNodeID);
         }
     }
@@ -1112,6 +1133,30 @@ void EntityItem::getAllTerseUpdateProperties(EntityItemProperties& properties) c
     properties._accelerationChanged = true;
 }
 
+void EntityItem::pokeSimulationOwnership() {
+    _dirtyFlags |= Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_POKE;
+    auto nodeList = DependencyManager::get<NodeList>();
+    if (_simulationOwner.matchesValidID(nodeList->getSessionUUID())) {
+        // we already own it
+        _simulationOwner.promotePriority(SCRIPT_POKE_SIMULATION_PRIORITY);
+    } else {
+        // we don't own it yet
+        _simulationOwner.setPendingPriority(SCRIPT_POKE_SIMULATION_PRIORITY, usecTimestampNow());
+    }
+}
+
+void EntityItem::grabSimulationOwnership() {
+    _dirtyFlags |= Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_GRAB;
+    auto nodeList = DependencyManager::get<NodeList>();
+    if (_simulationOwner.matchesValidID(nodeList->getSessionUUID())) {
+        // we already own it
+        _simulationOwner.promotePriority(SCRIPT_POKE_SIMULATION_PRIORITY);
+    } else {
+        // we don't own it yet
+        _simulationOwner.setPendingPriority(SCRIPT_GRAB_SIMULATION_PRIORITY, usecTimestampNow());
+    }
+}
+
 bool EntityItem::setProperties(const EntityItemProperties& properties) {
     bool somethingChanged = false;
 
@@ -1646,11 +1691,14 @@ void EntityItem::clearSimulationOwnership() {
 
     _simulationOwner.clear();
     // don't bother setting the DIRTY_SIMULATOR_ID flag because clearSimulationOwnership()
-    // is only ever called entity-server-side and the flags are only used client-side
+    // is only ever called on the entity-server and the flags are only used client-side
     //_dirtyFlags |= Simulation::DIRTY_SIMULATOR_ID;
 
 }
 
+void EntityItem::setPendingOwnershipPriority(quint8 priority, const quint64& timestamp) {
+    _simulationOwner.setPendingPriority(priority, timestamp);
+}
 
 bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer action) {
     bool result;

libraries/entities/src/EntityItem.h

@@ -302,6 +302,7 @@ public:
     QUuid getSimulatorID() const { return _simulationOwner.getID(); }
     void updateSimulationOwner(const SimulationOwner& owner);
     void clearSimulationOwnership();
+    void setPendingOwnershipPriority(quint8 priority, const quint64& timestamp);
 
     const QString& getMarketplaceID() const { return _marketplaceID; }
     void setMarketplaceID(const QString& value) { _marketplaceID = value; }
@@ -373,8 +374,8 @@ public:
 
     void getAllTerseUpdateProperties(EntityItemProperties& properties) const;
 
-    void pokeSimulationOwnership() { _dirtyFlags |= Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_POKE; }
-    void grabSimulationOwnership() { _dirtyFlags |= Simulation::DIRTY_SIMULATION_OWNERSHIP_FOR_GRAB; }
+    void pokeSimulationOwnership();
+    void grabSimulationOwnership();
     void flagForMotionStateChange() { _dirtyFlags |= Simulation::DIRTY_MOTION_TYPE; }
 
     bool addAction(EntitySimulation* simulation, EntityActionPointer action);

libraries/entities/src/SimulationOwner.cpp

@@ -16,12 +16,30 @@
 
 #include <NumericalConstants.h>
 
+const quint8 PENDING_STATE_NOTHING = 0;
+const quint8 PENDING_STATE_TAKE = 1;
+const quint8 PENDING_STATE_RELEASE = 2;
+
 // static
 const int SimulationOwner::NUM_BYTES_ENCODED = NUM_BYTES_RFC4122_UUID + 1;
 
+SimulationOwner::SimulationOwner() :
+        _id(),
+        _expiry(0),
+        _pendingTimestamp(0),
+        _priority(0),
+        _pendingPriority(0),
+        _pendingState(PENDING_STATE_NOTHING)
+{
+}
 
-SimulationOwner::SimulationOwner(const SimulationOwner& other)
-    : _id(other._id), _priority(other._priority), _expiry(other._expiry) {
+SimulationOwner::SimulationOwner(const QUuid& id, quint8 priority) :
+        _id(id),
+        _expiry(0),
+        _pendingTimestamp(0),
+        _priority(priority),
+        _pendingPriority(0)
+{
 }
 
 QByteArray SimulationOwner::toByteArray() const {
@@ -42,8 +60,11 @@ bool SimulationOwner::fromByteArray(const QByteArray& data) {
 
 void SimulationOwner::clear() {
     _id = QUuid();
-    _priority = 0;
     _expiry = 0;
+    _pendingTimestamp = 0;
+    _priority = 0;
+    _pendingPriority = 0;
+    _pendingState = PENDING_STATE_NOTHING;
 }
 
 void SimulationOwner::setPriority(quint8 priority) {
@@ -53,7 +74,6 @@ void SimulationOwner::setPriority(quint8 priority) {
 void SimulationOwner::promotePriority(quint8 priority) {
     if (priority > _priority) {
         _priority = priority;
-        updateExpiry();
     }
 }
 
@@ -81,11 +101,31 @@ bool SimulationOwner::set(const SimulationOwner& owner) {
     return setID(owner._id) || oldPriority != _priority;
 }
 
+void SimulationOwner::setPendingPriority(quint8 priority, const quint64& timestamp) {
+    _pendingPriority = priority;
+    _pendingTimestamp = timestamp;
+    _pendingState = (_pendingPriority == 0) ? PENDING_STATE_RELEASE : PENDING_STATE_TAKE;
+}
+
 void SimulationOwner::updateExpiry() {
     const quint64 OWNERSHIP_LOCKOUT_EXPIRY = USECS_PER_SECOND / 5;
     _expiry = usecTimestampNow() + OWNERSHIP_LOCKOUT_EXPIRY;
 }
 
+bool SimulationOwner::pendingRelease(const quint64& timestamp) {
+    return _pendingPriority == 0 && _pendingState == PENDING_STATE_RELEASE && _pendingTimestamp > timestamp;
+}
+
+bool SimulationOwner::pendingTake(const quint64& timestamp) {
+    return _pendingPriority > 0 && _pendingState == PENDING_STATE_TAKE && _pendingTimestamp > timestamp;
+}
+
+void SimulationOwner::clearCurrentOwner() {
+    _id = QUuid();
+    _expiry = 0;
+    _priority = 0;
+}
+
 // NOTE: eventually this code will be moved into unit tests
 // static debug
 void SimulationOwner::test() {

libraries/entities/src/SimulationOwner.h

@@ -37,13 +37,12 @@ class SimulationOwner {
 public:
     static const int NUM_BYTES_ENCODED;
 
-    SimulationOwner() : _id(), _priority(0), _expiry(0) {}
-    SimulationOwner(const QUuid& id, quint8 priority) : _id(id), _priority(priority), _expiry(0) {}
-    SimulationOwner(const SimulationOwner& other);
+    SimulationOwner();
+    SimulationOwner(const QUuid& id, quint8 priority);
 
     const QUuid& getID() const { return _id; }
-    quint8 getPriority() const { return _priority; }
     const quint64& getExpiry() const { return _expiry; }
+    quint8 getPriority() const { return _priority; }
 
     QByteArray toByteArray() const;
     bool fromByteArray(const QByteArray& data);
@@ -57,6 +56,7 @@ public:
     bool setID(const QUuid& id);
     bool set(const QUuid& id, quint8 priority);
     bool set(const SimulationOwner& owner);
+    void setPendingPriority(quint8 priority, const quint64& timestamp);
 
     bool isNull() const { return _id.isNull(); }
     bool matchesValidID(const QUuid& id) const { return _id == id && !_id.isNull(); }
@@ -65,6 +65,10 @@ public:
 
     bool hasExpired() const { return usecTimestampNow() > _expiry; }
 
+    bool pendingRelease(const quint64& timestamp); // return true if valid pending RELEASE
+    bool pendingTake(const quint64& timestamp); // return true if valid pending TAKE
+    void clearCurrentOwner();
+
     bool operator>=(quint8 priority) const { return _priority >= priority; }
     bool operator==(const SimulationOwner& other) { return (_id == other._id && _priority == other._priority); }
 
@@ -77,9 +81,12 @@ public:
     static void test();
 
 private:
-    QUuid _id;
-    quint8 _priority;
-    quint64 _expiry;
+    QUuid _id; // owner
+    quint64 _expiry; // time when ownership can transition at equal priority
+    quint64 _pendingTimestamp; // time when pending update was set
+    quint8 _priority; // priority of current owner
+    quint8 _pendingPriority; // priority of pendingTake
+    quint8 _pendingState; // NOTHING, TAKE, or RELEASE
 };
 
 

libraries/physics/src/EntityMotionState.cpp

@@ -519,12 +519,16 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_
         // but we remember we do still own it...  and rely on the server to tell us we don't
         properties.clearSimulationOwner();
         _outgoingPriority = 0;
+        _entity->setPendingOwnershipPriority(_outgoingPriority, now);
     } else if (Physics::getSessionUUID() != _entity->getSimulatorID()) {
         // we don't own the simulation for this entity yet, but we're sending a bid for it
-        properties.setSimulationOwner(Physics::getSessionUUID(),
-                glm::max<uint8_t>(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY));
+        quint8 bidPriority = glm::max<uint8_t>(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY);
+        properties.setSimulationOwner(Physics::getSessionUUID(), bidPriority);
         _nextOwnershipBid = now + USECS_BETWEEN_OWNERSHIP_BIDS;
-        _outgoingPriority = 0; // reset outgoing priority whenever we bid
+        // copy _outgoingPriority into pendingPriority...
+        _entity->setPendingOwnershipPriority(_outgoingPriority, now);
+        // ...then reset _outgoingPriority in preparation for the next frame
+        _outgoingPriority = 0;
     } else if (_outgoingPriority != _entity->getSimulationPriority()) {
         // we own the simulation but our desired priority has changed
         if (_outgoingPriority == 0) {
@@ -534,6 +538,7 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_
             // we just need to change the priority
             properties.setSimulationOwner(Physics::getSessionUUID(), _outgoingPriority);
         }
+        _entity->setPendingOwnershipPriority(_outgoingPriority, now);
     }
 
     EntityItemID id(_entity->getID());