From fb7daa185d63aa7445bf20959ff9ac1df63dee51 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 27 Mar 2019 09:53:55 -0700 Subject: [PATCH] improved physics for grabbed AvatarEntities --- interface/src/avatar/OtherAvatar.cpp | 12 +++++ .../src/avatars-renderer/Avatar.cpp | 26 ----------- .../src/avatars-renderer/Avatar.h | 2 - libraries/entities/src/EntityItem.cpp | 45 ++++++++++--------- libraries/entities/src/EntityItem.h | 8 ++-- 5 files changed, 41 insertions(+), 52 deletions(-) diff --git a/interface/src/avatar/OtherAvatar.cpp b/interface/src/avatar/OtherAvatar.cpp index 2058408596..b100b33dc8 100755 --- a/interface/src/avatar/OtherAvatar.cpp +++ b/interface/src/avatar/OtherAvatar.cpp @@ -495,6 +495,18 @@ void OtherAvatar::handleChangedAvatarEntityData() { const QUuid NULL_ID = QUuid("{00000000-0000-0000-0000-000000000005}"); entity->setParentID(NULL_ID); entity->setParentID(oldParentID); + + if (entity->stillHasMyGrabAction()) { + // For this case: we want to ignore transform+velocities coming from authoritative OtherAvatar + // because the MyAvatar is grabbing and we expect the local grab state + // to have enough information to prevent simulation drift. + // + // Clever readers might realize this could cause problems. For example, + // if an ignored OtherAvagtar were to simultanously grab the object then there would be + // a noticeable discrepancy between participants in the distributed physics simulation, + // however the difference would be stable and would not drift. + properties.clearTransformOrVelocityChanges(); + } if (entityTree->updateEntity(entityID, properties)) { entity->updateLastEditedFromRemote(); } else { diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index b0a8875cbc..839c4ed1d9 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -372,23 +372,6 @@ bool Avatar::applyGrabChanges() { target->removeGrab(grab); _avatarGrabs.erase(itr); grabAddedOrRemoved = true; - const EntityItemPointer& entity = std::dynamic_pointer_cast(target); - if (entity && entity->getEntityHostType() == entity::HostType::AVATAR) { - // grabs are able to move avatar-entities which belong ot other avatars (assuming - // the entities are grabbable, unlocked, etc). Regardless of who released the grab - // on this entity, the entity's owner needs to send off an update. - QUuid entityOwnerID = entity->getOwningAvatarID(); - if (entityOwnerID == getMyAvatarID() || entityOwnerID == AVATAR_SELF_ID) { - bool success; - SpatiallyNestablePointer myAvatarSN = SpatiallyNestable::findByID(entityOwnerID, success); - if (success) { - std::shared_ptr myAvatar = std::dynamic_pointer_cast(myAvatarSN); - if (myAvatar) { - myAvatar->sendPacket(entity->getID()); - } - } - } - } } else { undeleted.push_back(id); } @@ -2113,12 +2096,3 @@ void Avatar::updateDescendantRenderIDs() { } }); } - -QUuid Avatar::getMyAvatarID() const { - auto nodeList = DependencyManager::get(); - if (nodeList) { - return nodeList->getSessionUUID(); - } else { - return QUuid(); - } -} diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h index 2bed13cb12..974fae2034 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.h +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.h @@ -180,8 +180,6 @@ public: /// Returns the distance to use as a LOD parameter. float getLODDistance() const; - QUuid getMyAvatarID() const; - virtual void createOrb() { } enum class LoadingStatus { diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index f0bf13891b..bd4c6e5c71 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -789,8 +789,10 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef auto lastEdited = lastEditedFromBufferAdjusted; bool otherOverwrites = overwriteLocalData && !weOwnSimulation; - auto shouldUpdate = [this, lastEdited, otherOverwrites, filterRejection](quint64 updatedTimestamp, bool valueChanged) { - if (stillHasGrabActions()) { + // calculate hasGrab once outside the lambda rather than calling it every time inside + bool hasGrab = stillHasGrabAction(); + auto shouldUpdate = [this, lastEdited, otherOverwrites, filterRejection, hasGrab](quint64 updatedTimestamp, bool valueChanged) { + if (hasGrab) { return false; } bool simulationChanged = lastEdited > updatedTimestamp; @@ -957,12 +959,18 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef // by doing this parsing here... but it's not likely going to fully recover the content. // - if (overwriteLocalData && (getDirtyFlags() & (Simulation::DIRTY_TRANSFORM | Simulation::DIRTY_VELOCITIES))) { + if (overwriteLocalData && + !hasGrab && + (getDirtyFlags() & (Simulation::DIRTY_TRANSFORM | Simulation::DIRTY_VELOCITIES))) { // NOTE: This code is attempting to "repair" the old data we just got from the server to make it more // closely match where the entities should be if they'd stepped forward in time to "now". The server // is sending us data with a known "last simulated" time. That time is likely in the past, and therefore // this "new" data is actually slightly out of date. We calculate the time we need to skip forward and // use our simulation helper routine to get a best estimate of where the entity should be. + // + // NOTE: We don't want to do this in the hasGrab case because grabs "know best" + // (e.g. grabs will prevent drift between distributed physics simulations). + // float skipTimeForward = (float)(now - lastSimulatedFromBufferAdjusted) / (float)(USECS_PER_SECOND); // we want to extrapolate the motion forward to compensate for packet travel time, but @@ -1426,7 +1434,7 @@ void EntityItem::getTransformAndVelocityProperties(EntityItemProperties& propert void EntityItem::upgradeScriptSimulationPriority(uint8_t priority) { uint8_t newPriority = glm::max(priority, _scriptSimulationPriority); - if (newPriority < SCRIPT_GRAB_SIMULATION_PRIORITY && stillHasGrabActions()) { + if (newPriority < SCRIPT_GRAB_SIMULATION_PRIORITY && stillHasMyGrabAction()) { newPriority = SCRIPT_GRAB_SIMULATION_PRIORITY; } if (newPriority != _scriptSimulationPriority) { @@ -1439,7 +1447,7 @@ void EntityItem::upgradeScriptSimulationPriority(uint8_t priority) { void EntityItem::clearScriptSimulationPriority() { // DO NOT markDirtyFlags(Simulation::DIRTY_SIMULATION_OWNERSHIP_PRIORITY) here, because this // is only ever called from the code that actually handles the dirty flags, and it knows best. - _scriptSimulationPriority = stillHasGrabActions() ? SCRIPT_GRAB_SIMULATION_PRIORITY : 0; + _scriptSimulationPriority = stillHasMyGrabAction() ? SCRIPT_GRAB_SIMULATION_PRIORITY : 0; } void EntityItem::setPendingOwnershipPriority(uint8_t priority) { @@ -2186,7 +2194,7 @@ void EntityItem::enableNoBootstrap() { } void EntityItem::disableNoBootstrap() { - if (!stillHasGrabActions()) { + if (!stillHasMyGrabAction()) { _flags &= ~Simulation::SPECIAL_FLAGS_NO_BOOTSTRAPPING; _flags |= Simulation::DIRTY_COLLISION_GROUP; // may need to not collide with own avatar @@ -2272,7 +2280,13 @@ bool EntityItem::removeAction(EntitySimulationPointer simulation, const QUuid& a return success; } -bool EntityItem::stillHasGrabActions() const { +bool EntityItem::stillHasGrabAction() const { + return !_grabActions.empty(); +} + +// retutrns 'true' if there exists an action that returns 'true' for EntityActionInterface::isMine() +// (e.g. the action belongs to the MyAvatar instance) +bool EntityItem::stillHasMyGrabAction() const { QList holdActions = getActionsOfType(DYNAMIC_TYPE_HOLD); QList::const_iterator i = holdActions.begin(); while (i != holdActions.end()) { @@ -2700,20 +2714,6 @@ void EntityItem::setLastEdited(quint64 lastEdited) { }); } -quint64 EntityItem::getLastBroadcast() const { - quint64 result; - withReadLock([&] { - result = _lastBroadcast; - }); - return result; -} - -void EntityItem::setLastBroadcast(quint64 lastBroadcast) { - withWriteLock([&] { - _lastBroadcast = lastBroadcast; - }); -} - void EntityItem::markAsChangedOnServer() { withWriteLock([&] { _changedOnServer = usecTimestampNow(); @@ -3479,6 +3479,9 @@ void EntityItem::addGrab(GrabPointer grab) { simulation->addDynamic(action); markDirtyFlags(Simulation::DIRTY_MOTION_TYPE); simulation->changeEntity(getThisPointer()); + + // don't forget to set isMine() for locally-created grabs + action->setIsMine(grab->getOwnerID() == Physics::getSessionUUID()); } } diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index fae871a124..01ed949a0c 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -124,8 +124,8 @@ public: { return (float)(usecTimestampNow() - getLastEdited()) / (float)USECS_PER_SECOND; } /// Last time we sent out an edit packet for this entity - quint64 getLastBroadcast() const; - void setLastBroadcast(quint64 lastBroadcast); + quint64 getLastBroadcast() const { return _lastBroadcast; } + void setLastBroadcast(quint64 lastBroadcast) { _lastBroadcast = lastBroadcast; } void markAsChangedOnServer(); quint64 getLastChangedOnServer() const; @@ -562,6 +562,8 @@ public: static void setPrimaryViewFrustumPositionOperator(std::function getPrimaryViewFrustumPositionOperator) { _getPrimaryViewFrustumPositionOperator = getPrimaryViewFrustumPositionOperator; } static glm::vec3 getPrimaryViewFrustumPosition() { return _getPrimaryViewFrustumPositionOperator(); } + bool stillHasMyGrabAction() const; + signals: void requestRenderUpdate(); void spaceUpdate(std::pair data); @@ -574,7 +576,7 @@ protected: void setSimulated(bool simulated) { _simulated = simulated; } const QByteArray getDynamicDataInternal() const; - bool stillHasGrabActions() const; + bool stillHasGrabAction() const; void setDynamicDataInternal(QByteArray dynamicData); virtual void dimensionsChanged() override;