diff --git a/assignment-client/src/AssignmentAction.h b/assignment-client/src/AssignmentAction.h index 7df4acde1d..91c41c3d52 100644 --- a/assignment-client/src/AssignmentAction.h +++ b/assignment-client/src/AssignmentAction.h @@ -27,7 +27,7 @@ public: const QUuid& getID() const { return _id; } virtual EntityActionType getType() { return _type; } virtual void removeFromSimulation(EntitySimulation* simulation) const; - virtual const EntityItemPointer& getOwnerEntity() const { return _ownerEntity; } + virtual const EntityItemWeakPointer getOwnerEntity() const { return _ownerEntity; } virtual void setOwnerEntity(const EntityItemPointer ownerEntity) { _ownerEntity = ownerEntity; } virtual bool updateArguments(QVariantMap arguments) { assert(false); return false; } @@ -50,7 +50,7 @@ protected: virtual void setAngularVelocity(glm::vec3 angularVelocity) { assert(false); } bool _active; - EntityItemPointer _ownerEntity; + EntityItemWeakPointer _ownerEntity; }; #endif // hifi_AssignmentAction_h diff --git a/libraries/entities/src/EntityActionInterface.h b/libraries/entities/src/EntityActionInterface.h index 7ee9c19bdb..d71e070121 100644 --- a/libraries/entities/src/EntityActionInterface.h +++ b/libraries/entities/src/EntityActionInterface.h @@ -35,7 +35,7 @@ public: virtual EntityActionType getType() { assert(false); return ACTION_TYPE_NONE; } virtual void removeFromSimulation(EntitySimulation* simulation) const = 0; - virtual const EntityItemPointer& getOwnerEntity() const = 0; + virtual const EntityItemWeakPointer getOwnerEntity() const = 0; virtual void setOwnerEntity(const EntityItemPointer ownerEntity) = 0; virtual bool updateArguments(QVariantMap arguments) = 0; diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 998cd5f0c7..f6c185d124 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1371,13 +1371,13 @@ void EntityItem::updateSimulatorID(const QUuid& value) { bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer action) { assert(action); + auto actionOwnerEntity = action->getOwnerEntity().lock(); + assert(actionOwnerEntity); + assert(actionOwnerEntity.get() == this); + const QUuid& actionID = action->getID(); - _objectActionsLock.lockForWrite(); assert(!_objectActions.contains(actionID) || _objectActions[actionID] == action); _objectActions[actionID] = action; - _objectActionsLock.unlock(); - - assert(action->getOwnerEntity().get() == this); simulation->addAction(action); bool success = serializeActionData(); @@ -1388,13 +1388,10 @@ bool EntityItem::addAction(EntitySimulation* simulation, EntityActionPointer act } bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionID, const QVariantMap& arguments) { - _objectActionsLock.lockForRead(); if (!_objectActions.contains(actionID)) { - _objectActionsLock.unlock(); return false; } EntityActionPointer action = _objectActions[actionID]; - _objectActionsLock.unlock(); bool success = action->updateArguments(arguments); if (success) { success = serializeActionData(); @@ -1403,21 +1400,17 @@ bool EntityItem::updateAction(EntitySimulation* simulation, const QUuid& actionI } bool EntityItem::removeAction(EntitySimulation* simulation, const QUuid& actionID) { - _objectActionsLock.lockForWrite(); if (_objectActions.contains(actionID)) { EntityActionPointer action = _objectActions[actionID]; _objectActions.remove(actionID); - _objectActionsLock.unlock(); action->setOwnerEntity(nullptr); action->removeFromSimulation(simulation); return serializeActionData(); } - _objectActionsLock.unlock(); return false; } bool EntityItem::clearActions(EntitySimulation* simulation) { - _objectActionsLock.lockForWrite(); QHash::iterator i = _objectActions.begin(); while (i != _objectActions.end()) { const QUuid id = i.key(); @@ -1427,7 +1420,6 @@ bool EntityItem::clearActions(EntitySimulation* simulation) { action->removeFromSimulation(simulation); } _actionData = QByteArray(); - _objectActionsLock.unlock(); return true; } @@ -1456,14 +1448,11 @@ void EntityItem::setActionData(QByteArray actionData) { dsForAction >> actionID; updated << actionID; - _objectActionsLock.lockForRead(); if (_objectActions.contains(actionID)) { EntityActionPointer action = _objectActions[actionID]; - _objectActionsLock.unlock(); // XXX make sure types match? action->deserialize(serializedAction); } else { - _objectActionsLock.unlock(); auto actionFactory = DependencyManager::get(); EntityTree* entityTree = _element ? _element->getTree() : nullptr; @@ -1480,7 +1469,6 @@ void EntityItem::setActionData(QByteArray actionData) { EntityTree* entityTree = _element ? _element->getTree() : nullptr; EntitySimulation* simulation = entityTree ? entityTree->getSimulation() : nullptr; if (simulation) { - _objectActionsLock.lockForWrite(); QHash::iterator i = _objectActions.begin(); while (i != _objectActions.end()) { const QUuid id = i.key(); @@ -1493,18 +1481,13 @@ void EntityItem::setActionData(QByteArray actionData) { action->setOwnerEntity(nullptr); action->removeFromSimulation(simulation); } - _objectActionsLock.unlock(); } } bool EntityItem::serializeActionData() { - _objectActionsLock.lockForRead(); if (_objectActions.size() == 0) { - _objectActionsLock.unlock(); - _objectActionsLock.lockForWrite(); _actionData = QByteArray(); - _objectActionsLock.unlock(); return true; } @@ -1517,7 +1500,6 @@ bool EntityItem::serializeActionData() { serializedActions << bytesForAction; i++; } - _objectActionsLock.unlock(); QByteArray result; QDataStream ds(&result, QIODevice::WriteOnly); diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index b4c8f400e3..b7f816ecf9 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -461,7 +461,6 @@ protected: bool _simulated; // set by EntitySimulation bool serializeActionData(); - QReadWriteLock _objectActionsLock; QHash _objectActions; static int _maxActionDataSize; QByteArray _actionData; diff --git a/libraries/entities/src/EntityTypes.h b/libraries/entities/src/EntityTypes.h index 323a4eb92b..013f1064bd 100644 --- a/libraries/entities/src/EntityTypes.h +++ b/libraries/entities/src/EntityTypes.h @@ -21,6 +21,7 @@ class EntityItem; typedef std::shared_ptr EntityItemPointer; +typedef std::weak_ptr EntityItemWeakPointer; inline uint qHash(const EntityItemPointer& a, uint seed) { return qHash(a.get(), seed); diff --git a/libraries/physics/src/ObjectAction.cpp b/libraries/physics/src/ObjectAction.cpp index 274a79a814..0dc5eceaa5 100644 --- a/libraries/physics/src/ObjectAction.cpp +++ b/libraries/physics/src/ObjectAction.cpp @@ -27,10 +27,6 @@ void ObjectAction::updateAction(btCollisionWorld* collisionWorld, btScalar delta if (!_active) { return; } - if (!_ownerEntity) { - qDebug() << "ObjectAction::updateAction no owner entity"; - return; - } updateActionWorker(deltaTimeStep); } @@ -42,10 +38,11 @@ void ObjectAction::removeFromSimulation(EntitySimulation* simulation) const { } btRigidBody* ObjectAction::getRigidBody() { - if (!_ownerEntity) { + auto ownerEntity = _ownerEntity.lock(); + if (!ownerEntity) { return nullptr; } - void* physicsInfo = _ownerEntity->getPhysicsInfo(); + void* physicsInfo = ownerEntity->getPhysicsInfo(); if (!physicsInfo) { return nullptr; } diff --git a/libraries/physics/src/ObjectAction.h b/libraries/physics/src/ObjectAction.h index 2b7adacf78..320ae9e38f 100644 --- a/libraries/physics/src/ObjectAction.h +++ b/libraries/physics/src/ObjectAction.h @@ -32,7 +32,7 @@ public: const QUuid& getID() const { return _id; } virtual EntityActionType getType() { assert(false); return ACTION_TYPE_NONE; } virtual void removeFromSimulation(EntitySimulation* simulation) const; - virtual const EntityItemPointer& getOwnerEntity() const { return _ownerEntity; } + virtual const EntityItemWeakPointer getOwnerEntity() const { return _ownerEntity; } virtual void setOwnerEntity(const EntityItemPointer ownerEntity) { _ownerEntity = ownerEntity; } virtual bool updateArguments(QVariantMap arguments) { return false; } @@ -68,7 +68,7 @@ protected: void unlock() { _lock.unlock(); } bool _active; - EntityItemPointer _ownerEntity; + EntityItemWeakPointer _ownerEntity; }; #endif // hifi_ObjectAction_h diff --git a/libraries/physics/src/ObjectActionOffset.cpp b/libraries/physics/src/ObjectActionOffset.cpp index 54e9a151e8..9ceecea42b 100644 --- a/libraries/physics/src/ObjectActionOffset.cpp +++ b/libraries/physics/src/ObjectActionOffset.cpp @@ -32,7 +32,12 @@ void ObjectActionOffset::updateActionWorker(btScalar deltaTimeStep) { return; } - void* physicsInfo = _ownerEntity->getPhysicsInfo(); + auto ownerEntity = _ownerEntity.lock(); + if (!ownerEntity) { + return; + } + + void* physicsInfo = ownerEntity->getPhysicsInfo(); if (!physicsInfo) { unlock(); return; diff --git a/libraries/physics/src/ObjectActionSpring.cpp b/libraries/physics/src/ObjectActionSpring.cpp index 19aeb3265d..e85f5076b7 100644 --- a/libraries/physics/src/ObjectActionSpring.cpp +++ b/libraries/physics/src/ObjectActionSpring.cpp @@ -33,7 +33,12 @@ void ObjectActionSpring::updateActionWorker(btScalar deltaTimeStep) { return; } - void* physicsInfo = _ownerEntity->getPhysicsInfo(); + auto ownerEntity = _ownerEntity.lock(); + if (!ownerEntity) { + return; + } + + void* physicsInfo = ownerEntity->getPhysicsInfo(); if (!physicsInfo) { unlock(); return;