From d320f5b6f4e2f616501107c627cc3081300e035e Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 14:40:12 -0700 Subject: [PATCH 01/12] remember uuid of deleted actions for 20 seconds and don't re-add them --- libraries/entities/src/EntityItem.cpp | 20 +++++++++++++++++--- libraries/entities/src/EntityItem.h | 3 +++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index ce719ee976..161d2c9005 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -34,7 +34,7 @@ bool EntityItem::_sendPhysicsUpdates = true; int EntityItem::_maxActionsDataSize = 800; - +quint64 EntityItem::_rememberDeletedActionTime = 20; // seconds EntityItem::EntityItem(const EntityItemID& entityItemID) : _type(EntityTypes::Unknown), _id(entityItemID), @@ -1618,6 +1618,8 @@ void EntityItem::deserializeActions() { void EntityItem::deserializeActionsInternal() { assertWriteLocked(); + quint64 now = usecTimestampNow(); + if (!_element) { return; } @@ -1643,6 +1645,10 @@ void EntityItem::deserializeActionsInternal() { QUuid actionID; serializedActionStream >> actionType; serializedActionStream >> actionID; + if (_previouslyDeletedActions.contains(actionID)) { + continue; + } + updated << actionID; if (_objectActions.contains(actionID)) { @@ -1652,8 +1658,6 @@ void EntityItem::deserializeActionsInternal() { action->deserialize(serializedAction); } else { auto actionFactory = DependencyManager::get(); - - // EntityItemPointer entity = entityTree->findEntityByEntityItemID(_id, false); EntityItemPointer entity = shared_from_this(); EntityActionPointer action = actionFactory->factoryBA(entity, serializedAction); if (action) { @@ -1668,10 +1672,20 @@ void EntityItem::deserializeActionsInternal() { QUuid id = i.key(); if (!updated.contains(id)) { _actionsToRemove << id; + _previouslyDeletedActions.insert(id, now); } i++; } + // trim down _previouslyDeletedActions + QMutableHashIterator _previouslyDeletedIter(_previouslyDeletedActions); + while (_previouslyDeletedIter.hasNext()) { + _previouslyDeletedIter.next(); + if (_previouslyDeletedIter.value() - now > _rememberDeletedActionTime) { + _previouslyDeletedActions.remove(_previouslyDeletedIter.key()); + } + } + return; } diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 62b436b498..7f760bcf33 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -519,6 +519,9 @@ protected: void checkWaitingToRemove(EntitySimulation* simulation = nullptr); mutable QSet _actionsToRemove; mutable bool _actionDataDirty = false; + // _previouslyDeletedActions is used to avoid an action being re-added due to server round-trip lag + static quint64 _rememberDeletedActionTime; + mutable QHash _previouslyDeletedActions; }; #endif // hifi_EntityItem_h From 56b50b5fb302d5d889e6b4317b615c70e7ac9d51 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 14:45:10 -0700 Subject: [PATCH 02/12] also add action to _previouslyDeletedActions if the local interface deletes the action --- libraries/entities/src/EntityItem.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 161d2c9005..acd2e7d316 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1566,6 +1566,7 @@ bool EntityItem::removeAction(EntitySimulation* simulation, const QUuid& actionI bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* simulation) { assertWriteLocked(); + _previouslyDeletedActions.insert(actionID, usecTimestampNow()); if (_objectActions.contains(actionID)) { if (!simulation) { EntityTreePointer entityTree = _element ? _element->getTree() : nullptr; From fa7a5f1a615e599f6f7e7e8cbe9878a67b5c464c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 14:51:33 -0700 Subject: [PATCH 03/12] formatting, fix a bug --- libraries/entities/src/EntityItem.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index acd2e7d316..1ecaf238b2 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -35,6 +35,7 @@ bool EntityItem::_sendPhysicsUpdates = true; int EntityItem::_maxActionsDataSize = 800; quint64 EntityItem::_rememberDeletedActionTime = 20; // seconds + EntityItem::EntityItem(const EntityItemID& entityItemID) : _type(EntityTypes::Unknown), _id(entityItemID), @@ -1682,7 +1683,7 @@ void EntityItem::deserializeActionsInternal() { QMutableHashIterator _previouslyDeletedIter(_previouslyDeletedActions); while (_previouslyDeletedIter.hasNext()) { _previouslyDeletedIter.next(); - if (_previouslyDeletedIter.value() - now > _rememberDeletedActionTime) { + if (now - _previouslyDeletedIter.value() > _rememberDeletedActionTime) { _previouslyDeletedActions.remove(_previouslyDeletedIter.key()); } } From f2ebba9863a077cff9a008eb98082e470be41c79 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 16:16:23 -0700 Subject: [PATCH 04/12] separate out the inbound and outbound caches of serialized action data --- libraries/entities/src/EntityItem.cpp | 28 ++++++++++++++++----------- libraries/entities/src/EntityItem.h | 4 +++- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 1ecaf238b2..1bb70be205 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -34,7 +34,7 @@ bool EntityItem::_sendPhysicsUpdates = true; int EntityItem::_maxActionsDataSize = 800; -quint64 EntityItem::_rememberDeletedActionTime = 20; // seconds +quint64 EntityItem::_rememberDeletedActionTime = 20 * USECS_PER_SECOND; EntityItem::EntityItem(const EntityItemID& entityItemID) : _type(EntityTypes::Unknown), @@ -1528,7 +1528,7 @@ bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPoi bool success; QByteArray newDataCache = serializeActions(success); if (success) { - _allActionsDataCache = newDataCache; + _allActionsDataCacheOut = newDataCache; _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; } return success; @@ -1547,7 +1547,7 @@ bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionI success = action->updateArguments(arguments); if (success) { - _allActionsDataCache = serializeActions(success); + _allActionsDataCacheOut = serializeActions(success); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; } else { qDebug() << "EntityItem::updateAction failed"; @@ -1583,7 +1583,7 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* s } bool success = true; - _allActionsDataCache = serializeActions(success); + _allActionsDataCacheOut = serializeActions(success); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; return success; } @@ -1602,8 +1602,10 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { } // empty _serializedActions means no actions for the EntityItem _actionsToRemove.clear(); - _allActionsDataCache.clear(); + _allActionsDataCacheIn.clear(); + _allActionsDataCacheOut.clear(); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; + _actionDataDirty = true; }); return true; } @@ -1634,8 +1636,8 @@ void EntityItem::deserializeActionsInternal() { assert(simulation); QVector serializedActions; - if (_allActionsDataCache.size() > 0) { - QDataStream serializedActionsStream(_allActionsDataCache); + if (_allActionsDataCacheIn.size() > 0) { + QDataStream serializedActionsStream(_allActionsDataCacheIn); serializedActionsStream >> serializedActions; } @@ -1688,6 +1690,8 @@ void EntityItem::deserializeActionsInternal() { } } + _actionDataDirty = true; + return; } @@ -1709,8 +1713,10 @@ void EntityItem::setActionData(QByteArray actionData) { void EntityItem::setActionDataInternal(QByteArray actionData) { assertWriteLocked(); checkWaitingToRemove(); - _allActionsDataCache = actionData; - deserializeActionsInternal(); + if (_allActionsDataCacheIn != actionData) { + _allActionsDataCacheIn = actionData; + deserializeActionsInternal(); + } } QByteArray EntityItem::serializeActions(bool& success) const { @@ -1749,11 +1755,11 @@ const QByteArray EntityItem::getActionDataInternal() const { bool success; QByteArray newDataCache = serializeActions(success); if (success) { - _allActionsDataCache = newDataCache; + _allActionsDataCacheOut = newDataCache; } _actionDataDirty = false; } - return _allActionsDataCache; + return _allActionsDataCacheOut; } const QByteArray EntityItem::getActionData() const { diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 7f760bcf33..b48da7cdff 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -512,7 +512,9 @@ protected: QHash _objectActions; static int _maxActionsDataSize; - mutable QByteArray _allActionsDataCache; + mutable QByteArray _allActionsDataCacheIn; + mutable QByteArray _allActionsDataCacheOut; + // when an entity-server starts up, EntityItem::setActionData is called before the entity-tree is // ready. This means we can't find our EntityItemPointer or add the action to the simulation. These // are used to keep track of and work around this situation. From e90b156b7bd7c8456e6a6376cdf0db8f861a6c2f Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 17:02:02 -0700 Subject: [PATCH 05/12] avoid: add an action, immediately receive an update to that entities action, delete the just-added action because it wasn't in the packet --- libraries/entities/src/EntityActionInterface.h | 2 ++ libraries/entities/src/EntityItem.cpp | 11 +++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityActionInterface.h b/libraries/entities/src/EntityActionInterface.h index e61019c98c..4432b26a6a 100644 --- a/libraries/entities/src/EntityActionInterface.h +++ b/libraries/entities/src/EntityActionInterface.h @@ -48,6 +48,8 @@ public: virtual bool lifetimeIsOver() { return false; } + bool locallyAddedButNotYetReceived = false; + protected: virtual glm::vec3 getPosition() = 0; virtual void setPosition(glm::vec3 position) = 0; diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 1bb70be205..5bcb61963c 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1506,6 +1506,8 @@ bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer act result = addActionInternal(simulation, action); if (!result) { removeActionInternal(action->getID()); + } else { + action->locallyAddedButNotYetReceived = true; } }); @@ -1665,6 +1667,7 @@ void EntityItem::deserializeActionsInternal() { EntityItemPointer entity = shared_from_this(); EntityActionPointer action = actionFactory->factoryBA(entity, serializedAction); if (action) { + action->locallyAddedButNotYetReceived = false; entity->addActionInternal(simulation, action); } } @@ -1675,8 +1678,12 @@ void EntityItem::deserializeActionsInternal() { while (i != _objectActions.end()) { QUuid id = i.key(); if (!updated.contains(id)) { - _actionsToRemove << id; - _previouslyDeletedActions.insert(id, now); + EntityActionPointer action = i.value(); + // if we've just added this action, don't remove it due to lack of mention in an incoming packet. + if (! action->locallyAddedButNotYetReceived) { + _actionsToRemove << id; + _previouslyDeletedActions.insert(id, now); + } } i++; } From 40c5103d2fba7b3cbd2cc047cd002086651d6a1c Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 17:15:20 -0700 Subject: [PATCH 06/12] back out change where actino data caches were split into in/out --- libraries/entities/src/EntityItem.cpp | 21 ++++++++++----------- libraries/entities/src/EntityItem.h | 3 +-- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 5bcb61963c..2c18ac5cfd 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1530,7 +1530,7 @@ bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPoi bool success; QByteArray newDataCache = serializeActions(success); if (success) { - _allActionsDataCacheOut = newDataCache; + _allActionsDataCache = newDataCache; _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; } return success; @@ -1549,7 +1549,7 @@ bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionI success = action->updateArguments(arguments); if (success) { - _allActionsDataCacheOut = serializeActions(success); + _allActionsDataCache = serializeActions(success); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; } else { qDebug() << "EntityItem::updateAction failed"; @@ -1585,7 +1585,7 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* s } bool success = true; - _allActionsDataCacheOut = serializeActions(success); + _allActionsDataCache = serializeActions(success); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; return success; } @@ -1604,8 +1604,7 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { } // empty _serializedActions means no actions for the EntityItem _actionsToRemove.clear(); - _allActionsDataCacheIn.clear(); - _allActionsDataCacheOut.clear(); + _allActionsDataCache.clear(); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; _actionDataDirty = true; }); @@ -1638,8 +1637,8 @@ void EntityItem::deserializeActionsInternal() { assert(simulation); QVector serializedActions; - if (_allActionsDataCacheIn.size() > 0) { - QDataStream serializedActionsStream(_allActionsDataCacheIn); + if (_allActionsDataCache.size() > 0) { + QDataStream serializedActionsStream(_allActionsDataCache); serializedActionsStream >> serializedActions; } @@ -1720,8 +1719,8 @@ void EntityItem::setActionData(QByteArray actionData) { void EntityItem::setActionDataInternal(QByteArray actionData) { assertWriteLocked(); checkWaitingToRemove(); - if (_allActionsDataCacheIn != actionData) { - _allActionsDataCacheIn = actionData; + if (_allActionsDataCache != actionData) { + _allActionsDataCache = actionData; deserializeActionsInternal(); } } @@ -1762,11 +1761,11 @@ const QByteArray EntityItem::getActionDataInternal() const { bool success; QByteArray newDataCache = serializeActions(success); if (success) { - _allActionsDataCacheOut = newDataCache; + _allActionsDataCache = newDataCache; } _actionDataDirty = false; } - return _allActionsDataCacheOut; + return _allActionsDataCache; } const QByteArray EntityItem::getActionData() const { diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index b48da7cdff..c856794322 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -512,8 +512,7 @@ protected: QHash _objectActions; static int _maxActionsDataSize; - mutable QByteArray _allActionsDataCacheIn; - mutable QByteArray _allActionsDataCacheOut; + mutable QByteArray _allActionsDataCache; // when an entity-server starts up, EntityItem::setActionData is called before the entity-tree is // ready. This means we can't find our EntityItemPointer or add the action to the simulation. These From 9f27e5ff1fb260bc313fd8c9c08775ab8eae47f9 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Thu, 1 Oct 2015 18:21:53 -0700 Subject: [PATCH 07/12] back out a couple other unneeded changes --- libraries/entities/src/EntityItem.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 2c18ac5cfd..87967629dd 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1606,7 +1606,6 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { _actionsToRemove.clear(); _allActionsDataCache.clear(); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; - _actionDataDirty = true; }); return true; } @@ -1719,10 +1718,8 @@ void EntityItem::setActionData(QByteArray actionData) { void EntityItem::setActionDataInternal(QByteArray actionData) { assertWriteLocked(); checkWaitingToRemove(); - if (_allActionsDataCache != actionData) { - _allActionsDataCache = actionData; - deserializeActionsInternal(); - } + _allActionsDataCache = actionData; + deserializeActionsInternal(); } QByteArray EntityItem::serializeActions(bool& success) const { From 5b970e6b1af1a3d06efd73ce868e53d0d65ea96e Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 2 Oct 2015 11:43:19 -0700 Subject: [PATCH 08/12] try to fix double-distance-grabbing lockout --- examples/controllers/handControllerGrab.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/examples/controllers/handControllerGrab.js b/examples/controllers/handControllerGrab.js index 5705bd4498..9822f90670 100644 --- a/examples/controllers/handControllerGrab.js +++ b/examples/controllers/handControllerGrab.js @@ -87,15 +87,18 @@ function getTag() { } function entityIsGrabbedByOther(entityID) { + // by convention, a distance grab sets the tag of its action to be grab-*owner-session-id*. var actionIDs = Entities.getActionIDs(entityID); for (var actionIndex = 0; actionIndex < actionIDs.length; actionIndex++) { var actionID = actionIDs[actionIndex]; var actionArguments = Entities.getActionArguments(entityID, actionID); var tag = actionArguments["tag"]; if (tag == getTag()) { + // we see a grab-*uuid* shaped tag, but it's our tag, so that's okay. continue; } if (tag.slice(0, 5) == "grab-") { + // we see a grab-*uuid* shaped tag and it's not ours, so someone else is grabbing it. return true; } } @@ -259,6 +262,7 @@ function MyController(hand, triggerAction) { } else { if (entityIsGrabbedByOther(intersection.entityID)) { // don't allow two people to distance grab the same object + this.lineOn(pickRay.origin, Vec3.multiply(pickRay.direction, LINE_LENGTH), NO_INTERSECT_COLOR); return; } // the hand is far from the intersected object. go into distance-holding mode From 880d92ee64d6cd369731c9b2449df4ab6ee214fd Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 2 Oct 2015 11:46:48 -0700 Subject: [PATCH 09/12] try to fix double-distance-grabbing lockout --- examples/grab.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/examples/grab.js b/examples/grab.js index 03a227931d..1e0adf9f1b 100644 --- a/examples/grab.js +++ b/examples/grab.js @@ -45,26 +45,25 @@ var IDENTITY_QUAT = { z: 0, w: 0 }; -var ACTION_LIFETIME = 120; // 2 minutes +var ACTION_LIFETIME = 10; // seconds function getTag() { return "grab-" + MyAvatar.sessionUUID; } function entityIsGrabbedByOther(entityID) { + // by convention, a distance grab sets the tag of its action to be grab-*owner-session-id*. var actionIDs = Entities.getActionIDs(entityID); - var actionIndex; - var actionID; - var actionArguments; - var tag; - for (actionIndex = 0; actionIndex < actionIDs.length; actionIndex++) { - actionID = actionIDs[actionIndex]; - actionArguments = Entities.getActionArguments(entityID, actionID); - tag = actionArguments.tag; - if (tag === getTag()) { + for (var actionIndex = 0; actionIndex < actionIDs.length; actionIndex++) { + var actionID = actionIDs[actionIndex]; + var actionArguments = Entities.getActionArguments(entityID, actionID); + var tag = actionArguments["tag"]; + if (tag == getTag()) { + // we see a grab-*uuid* shaped tag, but it's our tag, so that's okay. continue; } - if (tag.slice(0, 5) === "grab-") { + if (tag.slice(0, 5) == "grab-") { + // we see a grab-*uuid* shaped tag and it's not ours, so someone else is grabbing it. return true; } } @@ -530,4 +529,4 @@ Controller.mousePressEvent.connect(pressEvent); Controller.mouseMoveEvent.connect(moveEvent); Controller.mouseReleaseEvent.connect(releaseEvent); Controller.keyPressEvent.connect(keyPressEvent); -Controller.keyReleaseEvent.connect(keyReleaseEvent); \ No newline at end of file +Controller.keyReleaseEvent.connect(keyReleaseEvent); From c0e8b02a2fddc5768c1b6bc97d225523f47ce44b Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 2 Oct 2015 12:25:34 -0700 Subject: [PATCH 10/12] fix what happens when a distance grab is blocked because someone else already has it --- examples/controllers/handControllerGrab.js | 54 ++++++++++++---------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/examples/controllers/handControllerGrab.js b/examples/controllers/handControllerGrab.js index 9822f90670..f4a6d1790d 100644 --- a/examples/controllers/handControllerGrab.js +++ b/examples/controllers/handControllerGrab.js @@ -167,6 +167,12 @@ function MyController(hand, triggerAction) { } }; + this.setState = function(newState) { + // print("STATE: " + this.state + " --> " + newState); + this.state = newState; + } + + this.lineOn = function(closePoint, farPoint, color) { // draw a line if (this.pointer === null) { @@ -220,14 +226,14 @@ function MyController(hand, triggerAction) { this.off = function() { if (this.triggerSmoothedSqueezed()) { - this.state = STATE_SEARCHING; + this.setState(STATE_SEARCHING); return; } } this.search = function() { if (this.triggerSmoothedReleased()) { - this.state = STATE_RELEASE; + this.setState(STATE_RELEASE); return; } @@ -238,6 +244,8 @@ function MyController(hand, triggerAction) { direction: Quat.getUp(this.getHandRotation()) }; + this.lineOn(pickRay.origin, Vec3.multiply(pickRay.direction, LINE_LENGTH), NO_INTERSECT_COLOR); + var defaultGrabbableData = { grabbable: true }; @@ -253,21 +261,20 @@ function MyController(hand, triggerAction) { var grabbableData = getEntityCustomData(GRABBABLE_DATA_KEY, intersection.entityID, defaultGrabbableData); if (grabbableData.grabbable === false) { + this.grabbedEntity = null; return; } if (intersectionDistance < NEAR_PICK_MAX_DISTANCE) { // the hand is very close to the intersected object. go into close-grabbing mode. - this.state = STATE_NEAR_GRABBING; - + this.setState(STATE_NEAR_GRABBING); } else { + // don't allow two people to distance grab the same object if (entityIsGrabbedByOther(intersection.entityID)) { - // don't allow two people to distance grab the same object - this.lineOn(pickRay.origin, Vec3.multiply(pickRay.direction, LINE_LENGTH), NO_INTERSECT_COLOR); - return; + this.grabbedEntity = null; + } else { + // the hand is far from the intersected object. go into distance-holding mode + this.setState(STATE_DISTANCE_HOLDING); } - // the hand is far from the intersected object. go into distance-holding mode - this.state = STATE_DISTANCE_HOLDING; - this.lineOn(pickRay.origin, Vec3.multiply(pickRay.direction, LINE_LENGTH), NO_INTERSECT_COLOR); } } else { // forward ray test failed, try sphere test. @@ -291,15 +298,14 @@ function MyController(hand, triggerAction) { } } if (this.grabbedEntity === null) { - this.lineOn(pickRay.origin, Vec3.multiply(pickRay.direction, LINE_LENGTH), NO_INTERSECT_COLOR); + // this.lineOn(pickRay.origin, Vec3.multiply(pickRay.direction, LINE_LENGTH), NO_INTERSECT_COLOR); } else if (props.locked === 0 && props.collisionsWillMove === 1) { - this.state = STATE_NEAR_GRABBING; + this.setState(STATE_NEAR_GRABBING); } else if (props.collisionsWillMove === 0) { // We have grabbed a non-physical object, so we want to trigger a non-colliding event as opposed to a grab event - this.state = STATE_NEAR_GRABBING_NON_COLLIDING; + this.setState(STATE_NEAR_GRABBING_NON_COLLIDING); } } - }; this.distanceHolding = function() { @@ -329,7 +335,7 @@ function MyController(hand, triggerAction) { } if (this.actionID !== null) { - this.state = STATE_CONTINUE_DISTANCE_HOLDING; + this.setState(STATE_CONTINUE_DISTANCE_HOLDING); this.activateEntity(this.grabbedEntity); if (this.hand === RIGHT_HAND) { Entities.callEntityMethod(this.grabbedEntity, "setRightHand"); @@ -346,7 +352,7 @@ function MyController(hand, triggerAction) { this.continueDistanceHolding = function() { if (this.triggerSmoothedReleased()) { - this.state = STATE_RELEASE; + this.setState(STATE_RELEASE); return; } @@ -404,7 +410,7 @@ function MyController(hand, triggerAction) { var now = Date.now(); var deltaTime = (now - this.currentObjectTime) / MSEC_PER_SEC; // convert to seconds this.computeReleaseVelocity(deltaPosition, deltaTime, false); - + this.currentObjectPosition = newObjectPosition; this.currentObjectTime = now; @@ -427,7 +433,7 @@ function MyController(hand, triggerAction) { this.nearGrabbing = function() { if (this.triggerSmoothedReleased()) { - this.state = STATE_RELEASE; + this.setState(STATE_RELEASE); return; } @@ -458,7 +464,7 @@ function MyController(hand, triggerAction) { if (this.actionID === NULL_ACTION_ID) { this.actionID = null; } else { - this.state = STATE_CONTINUE_NEAR_GRABBING; + this.setState(STATE_CONTINUE_NEAR_GRABBING); if (this.hand === RIGHT_HAND) { Entities.callEntityMethod(this.grabbedEntity, "setRightHand"); } else { @@ -475,7 +481,7 @@ function MyController(hand, triggerAction) { this.continueNearGrabbing = function() { if (this.triggerSmoothedReleased()) { - this.state = STATE_RELEASE; + this.setState(STATE_RELEASE); return; } @@ -502,7 +508,7 @@ function MyController(hand, triggerAction) { this.nearGrabbingNonColliding = function() { if (this.triggerSmoothedReleased()) { - this.state = STATE_RELEASE; + this.setState(STATE_RELEASE); return; } if (this.hand === RIGHT_HAND) { @@ -511,12 +517,12 @@ function MyController(hand, triggerAction) { Entities.callEntityMethod(this.grabbedEntity, "setLeftHand"); } Entities.callEntityMethod(this.grabbedEntity, "startNearGrabNonColliding"); - this.state = STATE_CONTINUE_NEAR_GRABBING_NON_COLLIDING; + this.setState(STATE_CONTINUE_NEAR_GRABBING_NON_COLLIDING); }; this.continueNearGrabbingNonColliding = function() { if (this.triggerSmoothedReleased()) { - this.state = STATE_RELEASE; + this.setState(STATE_RELEASE); return; } Entities.callEntityMethod(this.grabbedEntity, "continueNearGrabbingNonColliding"); @@ -627,7 +633,7 @@ function MyController(hand, triggerAction) { this.grabbedVelocity = ZERO_VEC; this.grabbedEntity = null; this.actionID = null; - this.state = STATE_OFF; + this.setState(STATE_OFF); }; this.cleanup = function() { From 67cc944afc27ce9182b716a9b7c4ab07547901e6 Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Fri, 2 Oct 2015 13:27:45 -0700 Subject: [PATCH 11/12] fix double free problem --- libraries/entities/src/EntityItem.cpp | 22 +++++++++++----------- libraries/entities/src/EntityItem.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 87967629dd..a031fbb196 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1528,7 +1528,8 @@ bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPoi simulation->addAction(action); bool success; - QByteArray newDataCache = serializeActions(success); + QByteArray newDataCache; + serializeActions(success, newDataCache); if (success) { _allActionsDataCache = newDataCache; _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; @@ -1549,7 +1550,7 @@ bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionI success = action->updateArguments(arguments); if (success) { - _allActionsDataCache = serializeActions(success); + serializeActions(success, _allActionsDataCache); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; } else { qDebug() << "EntityItem::updateAction failed"; @@ -1585,7 +1586,7 @@ bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* s } bool success = true; - _allActionsDataCache = serializeActions(success); + serializeActions(success, _allActionsDataCache); _dirtyFlags |= EntityItem::DIRTY_PHYSICS_ACTIVATION; return success; } @@ -1722,13 +1723,13 @@ void EntityItem::setActionDataInternal(QByteArray actionData) { deserializeActionsInternal(); } -QByteArray EntityItem::serializeActions(bool& success) const { +void EntityItem::serializeActions(bool& success, QByteArray& result) const { assertLocked(); - QByteArray result; if (_objectActions.size() == 0) { success = true; - return QByteArray(); + result.clear(); + return; } QVector serializedActions; @@ -1746,21 +1747,20 @@ QByteArray EntityItem::serializeActions(bool& success) const { if (result.size() >= _maxActionsDataSize) { success = false; - return result; + return; } success = true; - return result; + return; } const QByteArray EntityItem::getActionDataInternal() const { if (_actionDataDirty) { bool success; - QByteArray newDataCache = serializeActions(success); + serializeActions(success, _allActionsDataCache); if (success) { - _allActionsDataCache = newDataCache; + _actionDataDirty = false; } - _actionDataDirty = false; } return _allActionsDataCache; } diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index c856794322..7b1537bbc3 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -508,7 +508,7 @@ protected: bool addActionInternal(EntitySimulation* simulation, EntityActionPointer action); bool removeActionInternal(const QUuid& actionID, EntitySimulation* simulation = nullptr); void deserializeActionsInternal(); - QByteArray serializeActions(bool& success) const; + void serializeActions(bool& success, QByteArray& result) const; QHash _objectActions; static int _maxActionsDataSize; From 993111d22f6249b788c4ca37f51dce9ca85d63ee Mon Sep 17 00:00:00 2001 From: Seth Alves Date: Mon, 5 Oct 2015 11:28:59 -0700 Subject: [PATCH 12/12] code review --- interface/src/InterfaceActionFactory.cpp | 6 +++--- libraries/entities/src/EntityItem.cpp | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/interface/src/InterfaceActionFactory.cpp b/interface/src/InterfaceActionFactory.cpp index 2879c19eaa..cf5dad8fa5 100644 --- a/interface/src/InterfaceActionFactory.cpp +++ b/interface/src/InterfaceActionFactory.cpp @@ -65,9 +65,9 @@ EntityActionPointer InterfaceActionFactory::factoryBA(EntityItemPointer ownerEnt if (action) { action->deserialize(data); - } - if (action->lifetimeIsOver()) { - return nullptr; + if (action->lifetimeIsOver()) { + return nullptr; + } } return action; diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index a031fbb196..c4f5ad0061 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1661,13 +1661,14 @@ void EntityItem::deserializeActionsInternal() { // TODO: make sure types match? there isn't currently a way to // change the type of an existing action. action->deserialize(serializedAction); + action->locallyAddedButNotYetReceived = false; } else { auto actionFactory = DependencyManager::get(); EntityItemPointer entity = shared_from_this(); EntityActionPointer action = actionFactory->factoryBA(entity, serializedAction); if (action) { - action->locallyAddedButNotYetReceived = false; entity->addActionInternal(simulation, action); + action->locallyAddedButNotYetReceived = false; } } }