diff --git a/assignment-client/src/AssignmentActionFactory.cpp b/assignment-client/src/AssignmentActionFactory.cpp index f15ad6d006..ba2692c611 100644 --- a/assignment-client/src/AssignmentActionFactory.cpp +++ b/assignment-client/src/AssignmentActionFactory.cpp @@ -26,11 +26,10 @@ EntityActionPointer AssignmentActionFactory::factory(EntitySimulation* simulatio if (action) { bool ok = action->updateArguments(arguments); if (ok) { - ownerEntity->addAction(simulation, action); return action; } } - return action; + return nullptr; } @@ -46,7 +45,8 @@ EntityActionPointer AssignmentActionFactory::factoryBA(EntitySimulation* simulat EntityActionPointer action = assignmentActionFactory(type, id, ownerEntity); - action->deserialize(data); - ownerEntity->addAction(simulation, action); + if (action) { + action->deserialize(data); + } return action; } diff --git a/examples/stick.js b/examples/stick.js index 5ae80bcaf3..f581591957 100644 --- a/examples/stick.js +++ b/examples/stick.js @@ -42,6 +42,9 @@ function makeNewStick() { actionID = Entities.addAction("hold", stickID, {relativePosition: {x: 0.0, y: 0.0, z: -0.9}, hand: hand, timeScale: 0.15}); + if (actionID == nullActionID) { + cleanUp(); + } makingNewStick = false; }, 3000); } diff --git a/interface/src/InterfaceActionFactory.cpp b/interface/src/InterfaceActionFactory.cpp index 02f8c8728d..ccff5b4dc6 100644 --- a/interface/src/InterfaceActionFactory.cpp +++ b/interface/src/InterfaceActionFactory.cpp @@ -44,11 +44,10 @@ EntityActionPointer InterfaceActionFactory::factory(EntitySimulation* simulation if (action) { bool ok = action->updateArguments(arguments); if (ok) { - ownerEntity->addAction(simulation, action); return action; } } - return action; + return nullptr; } @@ -64,7 +63,8 @@ EntityActionPointer InterfaceActionFactory::factoryBA(EntitySimulation* simulati EntityActionPointer action = interfaceActionFactory(type, id, ownerEntity); - action->deserialize(data); - ownerEntity->addAction(simulation, action); + if (action) { + action->deserialize(data); + } return action; } diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 705303fe7a..59e909eab0 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1404,20 +1404,26 @@ void EntityItem::updateSimulatorID(const QUuid& value) { void EntityItem::clearSimulationOwnership() { _simulationOwner.clear(); - // don't bother setting the DIRTY_SIMULATOR_ID flag because clearSimulationOwnership() + // 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 //_dirtyFlags |= EntityItem::DIRTY_SIMULATOR_ID; } bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer action) { - if (!_serializedActionsProcessed) { - setActionData(_serializedActions); - if (!_serializedActionsProcessed) { - return false; - } + checkWaitingToRemove(simulation); + if (!checkWaitingActionData(simulation)) { + return false; } + bool result = addActionInternal(simulation, action); + if (!result) { + removeAction(simulation, action->getID()); + } + return result; +} + +bool EntityItem::addActionInternal(EntitySimulation* simulation, EntityActionPointer action) { assert(action); assert(simulation); auto actionOwnerEntity = action->getOwnerEntity().lock(); @@ -1427,21 +1433,20 @@ bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer act const QUuid& actionID = action->getID(); assert(!_objectActions.contains(actionID) || _objectActions[actionID] == action); _objectActions[actionID] = action; - simulation->addAction(action); - bool success = serializeActions(); - if (!success) { - removeAction(simulation, actionID); + + bool success; + QByteArray newDataCache = serializeActions(success); + if (success) { + _allActionsDataCache = newDataCache; } return success; } bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionID, const QVariantMap& arguments) { - if (!_serializedActionsProcessed) { - setActionData(_serializedActions); - if (!_serializedActionsProcessed) { - return false; - } + checkWaitingToRemove(simulation); + if (!checkWaitingActionData(simulation)) { + return false; } if (!_objectActions.contains(actionID)) { @@ -1449,31 +1454,49 @@ bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionI } EntityActionPointer action = _objectActions[actionID]; bool success = action->updateArguments(arguments); + if (success) { - success = serializeActions(); + _allActionsDataCache = serializeActions(success); + } else { + qDebug() << "EntityItem::updateAction failed"; } + return success; } bool EntityItem::removeAction(EntitySimulation* simulation, const QUuid& actionID) { - if (!_serializedActionsProcessed) { - setActionData(_serializedActions); - if (!_serializedActionsProcessed) { - return false; - } + checkWaitingToRemove(simulation); + if (!checkWaitingActionData(simulation)) { + return false;; } + return removeActionInternal(actionID); +} + +bool EntityItem::removeActionInternal(const QUuid& actionID, EntitySimulation* simulation) { if (_objectActions.contains(actionID)) { + if (!simulation) { + EntityTree* entityTree = _element ? _element->getTree() : nullptr; + simulation = entityTree ? entityTree->getSimulation() : nullptr; + } + EntityActionPointer action = _objectActions[actionID]; - _objectActions.remove(actionID); action->setOwnerEntity(nullptr); - action->removeFromSimulation(simulation); - return serializeActions(); + _objectActions.remove(actionID); + + if (simulation) { + action->removeFromSimulation(simulation); + } + + bool success = true; + _allActionsDataCache = serializeActions(success); + return success; } return false; } bool EntityItem::clearActions(EntitySimulation* simulation) { + _waitingActionData.clear(); QHash::iterator i = _objectActions.begin(); while (i != _objectActions.end()) { const QUuid id = i.key(); @@ -1482,83 +1505,112 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { action->setOwnerEntity(nullptr); action->removeFromSimulation(simulation); } - // empty _serializedActions means no actions for the EntityItem - _serializedActions = QByteArray(); + _actionsToRemove.clear(); + _allActionsDataCache.clear(); return true; } -void EntityItem::setActionData(QByteArray actionData) { - // it would be nice to take this shortcut, but a previous add may have failed - // if (_serializedActions == actionData) { - // return; - // } - _serializedActions = actionData; - +bool EntityItem::deserializeActions(QByteArray allActionsData, EntitySimulation* simulation) const { + bool success = true; QVector serializedActions; - if (_serializedActions.size() > 0) { - QDataStream serializedActionsStream(actionData); + if (allActionsData.size() > 0) { + QDataStream serializedActionsStream(allActionsData); serializedActionsStream >> serializedActions; } // Keep track of which actions got added or updated by the new actionData QSet updated; - _serializedActionsProcessed = true; - foreach(QByteArray serializedAction, serializedActions) { - QDataStream serializedActionStream(serializedAction); - EntityActionType actionType; - QUuid actionID; - serializedActionStream >> actionType; - serializedActionStream >> actionID; - updated << actionID; - - if (_objectActions.contains(actionID)) { - EntityActionPointer action = _objectActions[actionID]; - // TODO: make sure types match? there isn't currently a way to - // change the type of an existing action. - action->deserialize(serializedAction); - } else { - auto actionFactory = DependencyManager::get(); - - EntityTree* entityTree = _element ? _element->getTree() : nullptr; - EntitySimulation* simulation = entityTree ? entityTree->getSimulation() : nullptr; - if (simulation) { - EntityItemPointer entity = entityTree->findEntityByEntityItemID(_id); - actionFactory->factoryBA(simulation, entity, serializedAction); - } else { - // we can't yet add the action. This method will be called later. - _serializedActionsProcessed = false; - } - } + EntityTree* entityTree = _element ? _element->getTree() : nullptr; + if (!simulation) { + simulation = entityTree ? entityTree->getSimulation() : nullptr; } - // remove any actions that weren't included in the new data. - EntityTree* entityTree = _element ? _element->getTree() : nullptr; - EntitySimulation* simulation = entityTree ? entityTree->getSimulation() : nullptr; - if (simulation) { - QHash::iterator i = _objectActions.begin(); + if (simulation && entityTree) { + foreach(QByteArray serializedAction, serializedActions) { + QDataStream serializedActionStream(serializedAction); + EntityActionType actionType; + QUuid actionID; + serializedActionStream >> actionType; + serializedActionStream >> actionID; + updated << actionID; + + if (_objectActions.contains(actionID)) { + EntityActionPointer action = _objectActions[actionID]; + // TODO: make sure types match? there isn't currently a way to + // change the type of an existing action. + action->deserialize(serializedAction); + } else { + auto actionFactory = DependencyManager::get(); + if (simulation) { + EntityItemPointer entity = entityTree->findEntityByEntityItemID(_id); + EntityActionPointer action = actionFactory->factoryBA(simulation, entity, serializedAction); + if (action) { + entity->addActionInternal(simulation, action); + } + } else { + // we can't yet add the action. This method will be called later. + qDebug() << "\nCANT ADD ACTION YET"; + success = false; + } + } + } + + // remove any actions that weren't included in the new data. + QHash::const_iterator i = _objectActions.begin(); while (i != _objectActions.end()) { const QUuid id = i.key(); - if (updated.contains(id)) { - i++; - continue; + if (!updated.contains(id)) { + _actionsToRemove << id; } - EntityActionPointer action = _objectActions[id]; - i = _objectActions.erase(i); - action->setOwnerEntity(nullptr); - action->removeFromSimulation(simulation); + i++; } + } else { + // no simulation + success = false; + } + + return success; +} + +bool EntityItem::checkWaitingActionData(EntitySimulation* simulation) const { + if (_waitingActionData.size() == 0) { + return true; + } + bool success = deserializeActions(_waitingActionData, simulation); + if (success) { + _waitingActionData.clear(); + } + return success; +} + +void EntityItem::checkWaitingToRemove(EntitySimulation* simulation) { + foreach(QUuid actionID, _actionsToRemove) { + removeActionInternal(actionID, simulation); + } + _actionsToRemove.clear(); +} + +void EntityItem::setActionData(QByteArray actionData) { + checkWaitingToRemove(); + bool success = deserializeActions(actionData); + _allActionsDataCache = actionData; + if (success) { + _waitingActionData.clear(); + } else { + _waitingActionData = actionData; } } -bool EntityItem::serializeActions() const { - if (!_serializedActionsProcessed) { - return false; +QByteArray EntityItem::serializeActions(bool& success) const { + QByteArray result; + if (!checkWaitingActionData()) { + return _waitingActionData; } if (_objectActions.size() == 0) { - _serializedActions = QByteArray(); - return true; + success = true; + return QByteArray(); } QVector serializedActions; @@ -1571,31 +1623,27 @@ bool EntityItem::serializeActions() const { i++; } - QByteArray result; QDataStream serializedActionsStream(&result, QIODevice::WriteOnly); serializedActionsStream << serializedActions; if (result.size() >= _maxActionsDataSize) { - return false; + success = false; + return result; } - _serializedActions = result; - return true; + success = true; + return result; } const QByteArray EntityItem::getActionData() const { - serializeActions(); - return _serializedActions; + return _allActionsDataCache; } -QVariantMap EntityItem::getActionArguments(const QUuid& actionID) { +QVariantMap EntityItem::getActionArguments(const QUuid& actionID) const { QVariantMap result; - if (!_serializedActionsProcessed) { - setActionData(_serializedActions); - if (!_serializedActionsProcessed) { - return result; - } + if (!checkWaitingActionData()) { + return result; } if (_objectActions.contains(actionID)) { diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index c0b689b3de..bc8901c6b1 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -394,7 +394,7 @@ public: const QByteArray getActionData() const; bool hasActions() { return !_objectActions.empty(); } QList getActionIDs() { return _objectActions.keys(); } - QVariantMap getActionArguments(const QUuid& actionID); + QVariantMap getActionArguments(const QUuid& actionID) const; protected: @@ -468,14 +468,20 @@ protected: void* _physicsInfo = nullptr; // set by EntitySimulation bool _simulated; // set by EntitySimulation - bool serializeActions() const; + bool addActionInternal(EntitySimulation* simulation, EntityActionPointer action); + bool removeActionInternal(const QUuid& actionID, EntitySimulation* simulation = nullptr); + bool deserializeActions(QByteArray allActionsData, EntitySimulation* simulation = nullptr) const; + QByteArray serializeActions(bool& success) const; QHash _objectActions; static int _maxActionsDataSize; - mutable QByteArray _serializedActions; + 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. This - // flag is used to keep track of and work around this situation. - bool _serializedActionsProcessed = false; + // 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. + bool checkWaitingActionData(EntitySimulation* simulation = nullptr) const; + void checkWaitingToRemove(EntitySimulation* simulation = nullptr); + mutable QByteArray _waitingActionData; + mutable QSet _actionsToRemove; }; #endif // hifi_EntityItem_h diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 51c698e7e8..7cc2c03dfc 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -574,7 +574,9 @@ QUuid EntityScriptingInterface::addAction(const QString& actionTypeString, if (actionType == ACTION_TYPE_NONE) { return false; } - if (actionFactory->factory(simulation, actionType, actionID, entity, arguments)) { + EntityActionPointer action = actionFactory->factory(simulation, actionType, actionID, entity, arguments); + if (action) { + entity->addAction(simulation, action); auto nodeList = DependencyManager::get(); const QUuid myNodeID = nodeList->getSessionUUID(); if (entity->getSimulatorID() != myNodeID) {